HSv3: Faulty cross-certs in introduction point keys (allows naive onionbalance for v3s)
During a discussion with Nick and David, we figured out that we messed up the cross certificates of the introduction point keys in the HSv3 descriptors.
In particular the spec asks to cross-certificate the descriptor signing key using the introduction point auth key, but it seems like we are doing it the other way around in the code:
desc_ip->auth_key_cert = tor_cert_create(signing_kp,
CERT_TYPE_AUTH_HS_IP_KEY,
&ip->auth_key_kp.pubkey,
nearest_hour,
HS_DESC_CERT_LIFETIME,
CERT_FLAG_INCLUDE_SIGNING_KEY);same goes for the intro point encryption key. However, it seems that the legacy encryption key cross-cert was made properly.
This is very related to the onionbalance for v3 ticket (parent of this) because these two cross-certs certs was exactly what was preventing us from doing the naive onionbalance for v3s.
So here are some steps forward:
a) Figure out if there are any other cases where we have messed up cross certs in the code. Figure out the precise dangers of the cross certs we messed up.
With David we scanned the HSv3 code and it seems like these two are the only certs we messed up. If we are right, this means that this bug allows some fairly low-severity attacks like FairPretender from legacy/trac#15951 (moved). We should make sure we know exactly what attacks this opens us up to.
b) We should decide if we want to fix this or not. Fixing this would likely involve making a new descriptor version with the right cross-certs, so that clients can verify both the old version and the new one. It will require some engineering but not too much. If we care about the attacks, we shoul fix it. However, if we don't care about these attacks, we can "not fix it", which will also probably allow us to do naive onionbalance for v3s.
As a side-task that can be done paralelly we should revise and improve the spec when it comes to cross-certs because it's currently quite confusing. We should properly define cross-certs and what they do, and we should also improve the wording in places (e.g. there is bad wording even in descriptor-signing-key-cert even tho the code is right), and we should update the spec based on what we decide in this ticket.
I have CCed twim who reported the FairPretender attack in this ticket, and we should think about this more!
Activity
changed milestone to %Tor: unspecified in legacy/trac
added 035-backport in Legacy / Trac 040-backport in Legacy / Trac 041-deferred-20190530 in Legacy / Trac 041-longterm in Legacy / Trac component::core tor/tor in Legacy / Trac milestone::Tor: unspecified in Legacy / Trac needs-proposal in Legacy / Trac onionbalance in Legacy / Trac points::4 in Legacy / Trac priority::high in Legacy / Trac scaling in Legacy / Trac security in Legacy / Trac severity::normal in Legacy / Trac status::new in Legacy / Trac tor-hs in Legacy / Trac type::defect in Legacy / Trac labels
Trac:
Description: During a discussion with Nick and David, we figured out that we messed up the cross certificates of the introduction point keys in the HSv3 descriptors.In particular the spec asks to cross-certificate the descriptor signing key using the introduction point auth key, but it seems like we are doing it the other way around in the code:
desc_ip->auth_key_cert = tor_cert_create(signing_kp, CERT_TYPE_AUTH_HS_IP_KEY, &ip->auth_key_kp.pubkey, nearest_hour, HS_DESC_CERT_LIFETIME, CERT_FLAG_INCLUDE_SIGNING_KEY);same goes for the intro point encryption key. However, it seems that the legacy encryption key cross-cert was made properly.
This is very related to the onionbalance for v3 ticket (parent of this) because these two cross-certs certs was exactly what was preventing us from doing the naive onionbalance for v3s.
So here are some steps forward:
a) Figure out if there are any other points where we have messed up cross certs in the code. Figure out the precise dangers of the cross certs we messed up.
With David we scanned the HSv3 code and it seems like these two are the only certs we messed up. If we are right, this means that this bug allows some fairly low-severity attacks like
FairPretenderfrom legacy/trac#15951 (moved). We should make sure we know exactly what attacks this opens us up to.b) We should decide if we want to fix this or not. Fixing this would likely involve making a new descriptor version with the right cross-certs, so that clients can verify both the old version and the new one. It will require some engineering but not too much. If we care about the attacks, we shoul fix it. However, if we don't care about these attacks, we can "not fix it", which will also probably allow us to do naive onionbalance for v3s.
As a side-task that can be done paralelly we should revise and improve the spec when it comes to cross-certs because it's currently quite confusing. We should properly define cross-certs and what they do, and we should also improve the wording in places (e.g. there is bad wording even in
descriptor-signing-key-certeven tho the code is right), and we should update the spec based on what we decide in this ticket.I have CCed twim who reported the FairPretender attack in this ticket, and we should think about this more!
to
During a discussion with Nick and David, we figured out that we messed up the cross certificates of the introduction point keys in the HSv3 descriptors.
In particular the spec asks to cross-certificate the descriptor signing key using the introduction point auth key, but it seems like we are doing it the other way around in the code:
desc_ip->auth_key_cert = tor_cert_create(signing_kp, CERT_TYPE_AUTH_HS_IP_KEY, &ip->auth_key_kp.pubkey, nearest_hour, HS_DESC_CERT_LIFETIME, CERT_FLAG_INCLUDE_SIGNING_KEY);same goes for the intro point encryption key. However, it seems that the legacy encryption key cross-cert was made properly.
This is very related to the onionbalance for v3 ticket (parent of this) because these two cross-certs certs was exactly what was preventing us from doing the naive onionbalance for v3s.
So here are some steps forward:
a) Figure out if there are any other cases where we have messed up cross certs in the code. Figure out the precise dangers of the cross certs we messed up.
With David we scanned the HSv3 code and it seems like these two are the only certs we messed up. If we are right, this means that this bug allows some fairly low-severity attacks like
FairPretenderfrom legacy/trac#15951 (moved). We should make sure we know exactly what attacks this opens us up to.b) We should decide if we want to fix this or not. Fixing this would likely involve making a new descriptor version with the right cross-certs, so that clients can verify both the old version and the new one. It will require some engineering but not too much. If we care about the attacks, we shoul fix it. However, if we don't care about these attacks, we can "not fix it", which will also probably allow us to do naive onionbalance for v3s.
As a side-task that can be done paralelly we should revise and improve the spec when it comes to cross-certs because it's currently quite confusing. We should properly define cross-certs and what they do, and we should also improve the wording in places (e.g. there is bad wording even in
descriptor-signing-key-certeven tho the code is right), and we should update the spec based on what we decide in this ticket.I have CCed twim who reported the FairPretender attack in this ticket, and we should think about this more!
-
Trac:
Description: During a discussion with Nick and David, we figured out that we messed up the cross certificates of the introduction point keys in the HSv3 descriptors.In particular the spec asks to cross-certificate the descriptor signing key using the introduction point auth key, but it seems like we are doing it the other way around in the code:
desc_ip->auth_key_cert = tor_cert_create(signing_kp, CERT_TYPE_AUTH_HS_IP_KEY, &ip->auth_key_kp.pubkey, nearest_hour, HS_DESC_CERT_LIFETIME, CERT_FLAG_INCLUDE_SIGNING_KEY);same goes for the intro point encryption key. However, it seems that the legacy encryption key cross-cert was made properly.
This is very related to the onionbalance for v3 ticket (parent of this) because these two cross-certs certs was exactly what was preventing us from doing the naive onionbalance for v3s.
So here are some steps forward:
a) Figure out if there are any other cases where we have messed up cross certs in the code. Figure out the precise dangers of the cross certs we messed up.
With David we scanned the HSv3 code and it seems like these two are the only certs we messed up. If we are right, this means that this bug allows some fairly low-severity attacks like
FairPretenderfrom legacy/trac#15951 (moved). We should make sure we know exactly what attacks this opens us up to.b) We should decide if we want to fix this or not. Fixing this would likely involve making a new descriptor version with the right cross-certs, so that clients can verify both the old version and the new one. It will require some engineering but not too much. If we care about the attacks, we shoul fix it. However, if we don't care about these attacks, we can "not fix it", which will also probably allow us to do naive onionbalance for v3s.
As a side-task that can be done paralelly we should revise and improve the spec when it comes to cross-certs because it's currently quite confusing. We should properly define cross-certs and what they do, and we should also improve the wording in places (e.g. there is bad wording even in
descriptor-signing-key-certeven tho the code is right), and we should update the spec based on what we decide in this ticket.I have CCed twim who reported the FairPretender attack in this ticket, and we should think about this more!
to
During a discussion with Nick and David, we figured out that we messed up the cross certificates of the introduction point keys in the HSv3 descriptors.
In particular the spec asks to cross-certificate the descriptor signing key using the introduction point auth key, but it seems like we are doing it the other way around in the code:
desc_ip->auth_key_cert = tor_cert_create(signing_kp, CERT_TYPE_AUTH_HS_IP_KEY, &ip->auth_key_kp.pubkey, nearest_hour, HS_DESC_CERT_LIFETIME, CERT_FLAG_INCLUDE_SIGNING_KEY);same goes for the intro point encryption key. However, it seems that the legacy encryption key cross-cert was made properly.
This is very related to the onionbalance for v3 ticket (parent of this) because these two cross-certs certs was exactly what was preventing us from doing the naive onionbalance for v3s.
So here are some steps forward:
a) Figure out if there are any other cases where we have messed up cross certs in the code. Figure out the precise dangers of the cross certs we messed up.
With David we scanned the HSv3 code and it seems like these two are the only certs we messed up. If we are right, this means that this bug allows some fairly low-severity attacks like
FairPretenderfrom legacy/trac#15951 (moved). We should make sure we know exactly what attacks this opens us up to.b) We should decide if we want to fix this or not. Fixing this would likely involve making a new descriptor version with the right cross-certs, so that clients can verify both the old version and the new one. It will require some engineering but not too much. If we care about the attacks, we shoul fix it. However, if we don't care about these attacks, we can "not fix it", which will also probably allow us to do naive onionbalance for v3s.
As a side-task that can be done paralelly we should revise and improve the spec when it comes to cross-certs because it's currently quite confusing. We should properly define cross-certs and what they do, and we should also improve the wording in places (e.g. there is bad wording even in
descriptor-signing-key-certeven tho the code is right), and we should update the spec based on what we decide in this ticket.I have CCed twim who reported the FairPretender attack in this ticket, and we should think about this more!
If we do decide to fix this (and I think we should), I think we'll need a multistep process. Something like this:
- Begin including the correct versions of these certificates. Continue including the current (incorrect) versions so as not to break existing clients, but mark them with an extension to indicate that you should only accept them when the correct certificates are present too.
- Check the new (correct certificates) when they are present.
- Stop including the old (incorrect) certificates.
For step 1 and step 2, we'll probably want to use a consensus-triggered feature to avoid fingerprinting. We can't do step 3 until 2022, when support for 0.3.5.x ends, unless we decide to backport this or something, which would be ... questionable.
Replying to nickm:
If we do decide to fix this (and I think we should), I think we'll need a multistep process. Something like this:
This means basically that we end up being right now doing OnionBalance v3 easily but then by 2022 when we start removing the cert from the descriptor, we won't be able anymore...
In other words, fixing this probably means not doing the OnionBalance naively..... hmmmm
I want to add some opinion here. I think the main reason that we made this mistake is because we didn't document the reason why we need to cross-certify the desc signing key with the intro auth key in the spec.
And honestly, it's quite counterintuitive. It sounds like we want to sign the "signing" key with "something" key?? (intro auth key) and the reader will think that it's a typo in the spec because the "signing" key should be the signing key not the signed key :P
-
From the Stockholm 2019 tor meeting discussions:
-
As a first step and considering our sponsor27 timeline, we'll not fix this for now but still consider it a security problem and thus needs to be fixed at some point.
-
By not fixing this, we can proceed with the easy implementation for OnionBalance v3 (legacy/trac#26768 (moved)).
-
Proposal 306 is being proposed for OnionBalance v3 support which assumes that one day this bug is fixed.
I'm moving this one out of Sponsor 27 because again the decision is not to fix it for now. I'm hoping for us to provide a reasoning soon on tor-dev@ on why.
Moving this out of 042 Milestone and unparenting. Prop306 does link to this ticket so we don't forget about it.
Trac:
Sponsor: Sponsor27-can to N/A
Parent: legacy/trac#26768 (moved) to N/A
Milestone: Tor: 0.4.2.x-final to Tor: unspecified -
-
Sorry for the late reply here gaba. Yes this means we are removing it from the roadmap for September and in general from S27 scope. We are focusing on legacy/trac#26768 (moved) head on.
