Opened 4 months ago

Last modified 4 months ago

#30479 new defect

Move away from using signed git tags to avoid rollback attacks?

Reported by: gk Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-rbm
Cc: kevun, arma, boklm Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description (last modified by gk)

When using git repos we try to build from signed git tags whenever possible and make sure we check the signature being valid. It turns out, though, that keys might expire at some point making the signature verification fail from that point on. This is unfortunate for some reasons: it might break building current Tor Browser versions (see #19737) and it might break verifying older releases which would be nice to have to be able to verify later on that everything went right.

In #19737 we chose to ignore expired keys in the sense that we treat a signature as valid as long as the verification succeeds (modulo the expiration problem). Now, it has been argued that this could lead to rollback attacks in the sense that an attacker could modify a git repo in a way that the tag we want to use points to older code while still satisfying all of the git tag signing constraints we have.

However, thinking a bit more the expiration problem is actually orthogonal as this could even happen with a properly signed tag, which does not suffer from a signature done with a key that is expired now, but which is still not the current version. That means: assuming you have three tags t1, t2, and t3 and t1 has a signature which is expired while t2 and t3 don't, but only t3 contains the critical fix, then with a git attacker in question it does not make a difference whether we fix the expiration date problem as they could easily make us use t1 *or* t2.

For fixing the expiration date problem there are some proposals with different trade-offs:

1) We could use signing keys that do not expire (see: https://blog.invisiblethings.org/keys/). If at all this is only helpful for our own keys but we use other project's git tags as well which might use expired signing keys.

2) We could use Peter Todd's OpenTimestamps tool (https://petertodd.org/2016/opentimestamps-git-integration). That again might seem useful but we have the problem again that we use other project's keys which might not have those timestamps.

3) Even if we solved 1) and 2) magically we'd still have the problem above that an attacker can point to either t1 or t2. So a different approach would be to point to (signed) commits directly to avoid the problem that (signed) tags could point to anything.

So, should we switch to pinning commits from now on? Or is there even a better solution available?

(Thanks to kevun and arma for input)

Child Tickets

Change History (7)

comment:1 Changed 4 months ago by gk

Description: modified (diff)

comment:2 Changed 4 months ago by boklm

However, thinking a bit more the expiration problem is actually orthogonal as this could even happen with a properly signed tag, which does not suffer from a signature done with a key that is expired now, but which is still not the current version. That means: assuming you have three tags t1, t2, and t3 and t1 has a signature which is expired while t2 and t3 don't, but only t3 contains the critical fix, then with a git attacker in question it does not make a difference whether we fix the expiration date problem as they could easily make us use t1 *or* t2.

I don't think signed tags allow rollback attacks. The data that is signed by gpg includes the tag itself, so if an attackers returns t2 when we want t3, the signature will be valid but the content of the tag will say t2 so git should reject it.

For example, we can see that the signed data for the tag tbb-8.5a12-build3 includes the line tag tbb-8.5a12-build3:

$ git cat-file -p  tbb-8.5a12-build3 
object a28a0c3b1b52a1ef87cb58bd6b62d888fcf313a0
type commit
tag tbb-8.5a12-build3
tagger Georg Koppen <gk@torproject.org> 1557175973 +0000

Tagging build3 for 8.5a12
-----BEGIN PGP SIGNATURE-----

iQJGBAABCgAwFiEEqhpZkioIc40jQrwFTZKn5Ktz7FQFAlzQnqUSHGdrQHRvcnBy
b2plY3Qub3JnAAoJEE2Sp+Src+xUZoYQAKfhS5VQxkhfU3HkNDNJXVXD4yeeTKz2
QlhM4KT742Gxa2u3mM4BcsjMhAR0PRyqZqt+7rBAHC4IKQvck1lDmRBswGinI+44
qc19hpIS5brZfzhmzTEAvqkM4KpAINNBBk6xeNjJvtfjY17DwGmFzeFG0UUzjQEM
PIW4sFkU1OVFe9E1sOWTNhhdEC0EhASa0hSgp9iZPbp7kdseUC0ebAZ/i2y9ITsF
ypoH12WlqqptJ8nkVmJg6+7IoAJXFeygP8BPmOqH7OeK6lwXNVoXe5NrlzqqtviN
3hwbmzextwRLPRLq5v1YELM6n9NHGs4uYDJUrMFo66cLszn+nALwWRYsGMh42PmH
KewX0AT/om+LhIhL0fW8Wr8HQ8hgUB3zH8cr8yhhR3puorYVm8fuQVyRAKKAAOzl
NijKqy/UchYOGDM//DRatl7UNhIIl2RD3miusFSk2Ssv7WK3m7BRmjUWyAzhIKBK
rKK0UEriQtp55bpa6q7pgsY7rqMGm/+NhiG5UcdsJIey9tJMt0VaWrpQlwGyWn4J
XDM1ezsdvePQRT/zNrZ57zWQIZ2ygF9w1Jw939x8ACUBp8c0pC1rdetaHr68Sb5k
WGU0Ofel+CQYAneuLTRnRpPr8VOZcdyoLY/Nr7MQs7bnCpx1M1IMOUUw4Qdmw/0K
+CjxsU9kYB1F
=TqMm
-----END PGP SIGNATURE-----

This is however true for the signed tarballs. The filename of the tarball is not part of the signed data, so an attacker could give us an older version of a tarball with a valid signature. So maybe we should stop using gpg to verify donloaded files, and use hashes instead (or in addition)?

comment:3 Changed 4 months ago by boklm

Regarding expiration of keys (or sub-keys), I think whether we should accept signatures from expired keys depends on the meaning of this expiration:

  • if the expiration means "after this date, my key will very likely be compromised, so you should not trust anything signed by it anymore", then we should reject all signatures from expired keys.
  • if the expiration means "after this date I will use a new key, so anything new will be signed with a new key, but this key can still be used to verify things signed before this date", then I think we should accept signatures from expired keys, and also remove expired keys from our keyring when we don't need to use anything signed before the expiration date anymore.

I think the meaning of key expiration is usually the second one as it is hard to predict when a key will be compromised, but it is possible to plan when a key will be rotated, so I think it is fine to accept signatures from expired keys when we expect the signature to be made before it expired.

comment:4 in reply to:  2 ; Changed 4 months ago by gk

Replying to boklm:

However, thinking a bit more the expiration problem is actually orthogonal as this could even happen with a properly signed tag, which does not suffer from a signature done with a key that is expired now, but which is still not the current version. That means: assuming you have three tags t1, t2, and t3 and t1 has a signature which is expired while t2 and t3 don't, but only t3 contains the critical fix, then with a git attacker in question it does not make a difference whether we fix the expiration date problem as they could easily make us use t1 *or* t2.

I don't think signed tags allow rollback attacks. The data that is signed by gpg includes the tag itself, so if an attackers returns t2 when we want t3, the signature will be valid but the content of the tag will say t2 so git should reject it.

Here is a PoC that works for me:

1) Create a new tag for, say Torbutton, like 2.1.8 and push that to our git repo
2) An attacker replaces the contents of .git/refs/tags/2.1.8 with those of .git/refs/tags/2.1.7
3) We fetch the new tags to our build machines and start building
4) The verification of "2.1.8" succeeds and git is happily using the old and possibly outdated 2.1.7 as 2.1.8, although we wanted to have a different commit for 2.1.8 (i.e. the originally tagged one).
5) We ship 2.1.7 although our torbutton config shows 2.1.8

Version 0, edited 4 months ago by gk (next)

comment:5 in reply to:  4 ; Changed 4 months ago by boklm

Replying to gk:

Replying to boklm:

However, thinking a bit more the expiration problem is actually orthogonal as this could even happen with a properly signed tag, which does not suffer from a signature done with a key that is expired now, but which is still not the current version. That means: assuming you have three tags t1, t2, and t3 and t1 has a signature which is expired while t2 and t3 don't, but only t3 contains the critical fix, then with a git attacker in question it does not make a difference whether we fix the expiration date problem as they could easily make us use t1 *or* t2.

I don't think signed tags allow rollback attacks. The data that is signed by gpg includes the tag itself, so if an attackers returns t2 when we want t3, the signature will be valid but the content of the tag will say t2 so git should reject it.

Here is a PoC that works for me:

1) Create a new tag for, say Torbutton, like 2.1.8 and push that to our git repo
2) An attacker replaces the contents of .git/refs/tags/2.1.8 with those of .git/refs/tags/2.1.7
3) We fetch the new tag to our build machines and start building
4) The verification of "2.1.8" succeeds and git is happily using the old and possibly outdated 2.1.7 as 2.1.8, although we wanted to have a different commit for 2.1.8 (i.e. the originally tagged one).
5) We ship 2.1.7 although our torbutton config shows 2.1.8

Hmm, indeed it works. I was expecting git to check that the tag is really the tag we want, since the tag name is included in the signed data, but it seems that it does not check that. So it looks like a bug in git to me.

comment:6 in reply to:  5 Changed 4 months ago by boklm

Replying to boklm:

Replying to gk:

Replying to boklm:

However, thinking a bit more the expiration problem is actually orthogonal as this could even happen with a properly signed tag, which does not suffer from a signature done with a key that is expired now, but which is still not the current version. That means: assuming you have three tags t1, t2, and t3 and t1 has a signature which is expired while t2 and t3 don't, but only t3 contains the critical fix, then with a git attacker in question it does not make a difference whether we fix the expiration date problem as they could easily make us use t1 *or* t2.

I don't think signed tags allow rollback attacks. The data that is signed by gpg includes the tag itself, so if an attackers returns t2 when we want t3, the signature will be valid but the content of the tag will say t2 so git should reject it.

Here is a PoC that works for me:

1) Create a new tag for, say Torbutton, like 2.1.8 and push that to our git repo
2) An attacker replaces the contents of .git/refs/tags/2.1.8 with those of .git/refs/tags/2.1.7
3) We fetch the new tag to our build machines and start building
4) The verification of "2.1.8" succeeds and git is happily using the old and possibly outdated 2.1.7 as 2.1.8, although we wanted to have a different commit for 2.1.8 (i.e. the originally tagged one).
5) We ship 2.1.7 although our torbutton config shows 2.1.8

Hmm, indeed it works. I was expecting git to check that the tag is really the tag we want, since the tag name is included in the signed data, but it seems that it does not check that. So it looks like a bug in git to me.

I also see that git outputs the tag object when we run git tag -v [tag]. So we could check that the tag is the right tag in rbm. I opened #30480 to do that.

comment:7 Changed 4 months ago by boklm

Related to the issue of signatures made with expired keys, I opened #30548 to clean up our keyring files to remove any key that we don't need, and #30549 to make that easier to do.

Note: See TracTickets for help on using tickets.