[tor-commits] [tor/master] Audit use of tor_tls_cert_get_key().

[nickm at torproject.org]

commit f156156d56ec61394eb814397c33557762870809
Author: Nick Mathewson <nickm at torproject.org>
Date:   Thu Sep 22 11:22:11 2016 -0400

    Audit use of tor_tls_cert_get_key().
    
    This function is allowed to return NULL if the certified key isn't
    RSA. But in a couple of places we were treating this as a bug or
    internal error, and in one other place we weren't checking for it at
    all!
    
    Caught by Isis during code review for #15055.  The serious bug was
    only on the 15055 branch, thank goodness.
---
 src/common/tortls.c |  2 +-
 src/or/channeltls.c | 10 +++-------
 src/or/torcert.c    |  3 +++
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/src/common/tortls.c b/src/common/tortls.c
index eb24411..e3550b2 100644
--- a/src/common/tortls.c
+++ b/src/common/tortls.c
@@ -814,7 +814,7 @@ tor_tls_get_my_client_auth_key(void)
 
 /**
  * Return a newly allocated copy of the public key that a certificate
- * certifies.  Return NULL if the cert's key is not RSA.
+ * certifies. Watch out! This returns NULL if the cert's key is not RSA.
  */
 crypto_pk_t *
 tor_tls_cert_get_key(tor_x509_cert_t *cert)
diff --git a/src/or/channeltls.c b/src/or/channeltls.c
index f5b81f0..a3e1f8c 100644
--- a/src/or/channeltls.c
+++ b/src/or/channeltls.c
@@ -1955,9 +1955,7 @@ channel_tls_process_certs_cell(var_cell_t *cell, channel_tls_t *chan)
 
       identity_rcvd = tor_tls_cert_get_key(id_cert);
       if (!identity_rcvd) {
-        //LCOV_EXCL_START
-        ERR("Internal error: Couldn't get RSA key from ID cert.");
-        //LCOV_EXCL_STOP
+        ERR("Couldn't get RSA key from ID cert.");
       }
       memcpy(chan->conn->handshake_state->authenticated_rsa_peer_id,
              id_digests->d[DIGEST_SHA1], DIGEST_LEN);
@@ -2242,10 +2240,8 @@ channel_tls_process_authenticate_cell(var_cell_t *cell, channel_tls_t *chan)
     size_t keysize;
     int signed_len;
 
-    if (BUG(!pk)) {
-      // LCOV_EXCL_START
-      ERR("Internal error: couldn't get RSA key from AUTH cert.");
-      // LCOV_EXCL_STOP
+    if (! pk) {
+      ERR("Couldn't get RSA key from AUTH cert.");
     }
     crypto_digest256(d, (char*)auth, V3_AUTH_BODY_LEN, DIGEST_SHA256);
 
diff --git a/src/or/torcert.c b/src/or/torcert.c
index d100298..69f50aa 100644
--- a/src/or/torcert.c
+++ b/src/or/torcert.c
@@ -559,6 +559,9 @@ or_handshake_certs_ed25519_ok(int severity,
     ERR("Missing RSA->Ed25519 crosscert");
   }
   crypto_pk_t *rsa_id_key = tor_tls_cert_get_key(rsa_id_cert);
+  if (!rsa_id_key) {
+    ERR("RSA ID cert had no RSA key");
+  }
 
   if (rsa_ed25519_crosscert_check(certs->ed_rsa_crosscert,
                                   certs->ed_rsa_crosscert_len,