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!