Browse Source

Reject 128-byte keys that are not 1024-bit

When we added the check for key size, we required that the keys be
128 bytes.  But RSA_size (which defers to BN_num_bytes) will return
128 for keys of length 1017..1024.  This patch adds a new
crypto_pk_num_bits() that returns the actual number of significant
bits in the modulus, and uses that to enforce key sizes.

Also, credit the original bug3318 in the changes file.
Nick Mathewson 13 years ago
parent
commit
bbf2fee8ff
5 changed files with 21 additions and 3 deletions
  1. 5 1
      changes/bug3318
  2. 11 0
      src/common/crypto.c
  3. 1 0
      src/common/crypto.h
  4. 2 2
      src/or/routerparse.c
  5. 2 0
      src/test/test_crypto.c

+ 5 - 1
changes/bug3318

@@ -1,3 +1,7 @@
   o Minor bugfixes:
     - Fix a log message that said "bits" while displaying a value in
-      bytes. Fixes bug 3318; bugfix on 0.2.0.1-alpha.
+      bytes. Found by wanoskarnet. Fixes bug 3318; bugfix on
+      0.2.0.1-alpha.
+    - When checking for 1024-bit keys, check for 1024 bits, not 128
+      bytes. This allows Tor to correctly discard keys of length
+      1017 through 1023. Bugfix on 0.0.9pre5.

+ 11 - 0
src/common/crypto.c

@@ -777,6 +777,17 @@ crypto_pk_keysize(crypto_pk_env_t *env)
   return (size_t) RSA_size(env->key);
 }
 
+/** Return the size of the public key modulus of <b>env</b>, in bits. */
+int
+crypto_pk_num_bits(crypto_pk_env_t *env)
+{
+  tor_assert(env);
+  tor_assert(env->key);
+  tor_assert(env->key->n);
+
+  return BN_num_bits(env->key->n);
+}
+
 /** Increase the reference count of <b>env</b>, and return it.
  */
 crypto_pk_env_t *

+ 1 - 0
src/common/crypto.h

@@ -119,6 +119,7 @@ int crypto_pk_write_private_key_to_filename(crypto_pk_env_t *env,
 int crypto_pk_check_key(crypto_pk_env_t *env);
 int crypto_pk_cmp_keys(crypto_pk_env_t *a, crypto_pk_env_t *b);
 size_t crypto_pk_keysize(crypto_pk_env_t *env);
+int crypto_pk_num_bits(crypto_pk_env_t *env);
 crypto_pk_env_t *crypto_pk_dup_key(crypto_pk_env_t *orig);
 crypto_pk_env_t *crypto_pk_copy_full(crypto_pk_env_t *orig);
 int crypto_pk_key_is_private(const crypto_pk_env_t *key);

+ 2 - 2
src/or/routerparse.c

@@ -3765,9 +3765,9 @@ token_check_object(memarea_t *area, const char *kwd,
       break;
     case NEED_KEY_1024: /* There must be a 1024-bit public key. */
     case NEED_SKEY_1024: /* There must be a 1024-bit private key. */
-      if (tok->key && crypto_pk_keysize(tok->key) != PK_BYTES) {
+      if (tok->key && crypto_pk_num_bits(tok->key) != PK_BYTES*8) {
         tor_snprintf(ebuf, sizeof(ebuf), "Wrong size on key for %s: %d bits",
-                     kwd, (int)crypto_pk_keysize(tok->key)*8);
+                     kwd, crypto_pk_num_bits(tok->key));
         RET_ERR(ebuf);
       }
       /* fall through */

+ 2 - 0
src/test/test_crypto.c

@@ -343,7 +343,9 @@ test_crypto_pk(void)
   test_eq(0, crypto_pk_cmp_keys(pk1, pk2));
 
   test_eq(128, crypto_pk_keysize(pk1));
+  test_eq(1024, crypto_pk_num_bits(pk1));
   test_eq(128, crypto_pk_keysize(pk2));
+  test_eq(1024, crypto_pk_num_bits(pk2));
 
   test_eq(128, crypto_pk_public_encrypt(pk2, data1, sizeof(data1),
                                         "Hello whirled.", 15,