Refactor crypto_rsa to use pem module.

This cleans up a lot of junk from crypto_rsa_openssl, and will
save us duplicated code in crypto_rsa_nss (when it exists).

(Actually, it already exists, but I am going to use git rebase so
that this commit precedes the creation of crypto_rsa_nss.)

src/lib/crypt_ops/crypto_rsa.c

@@ -23,8 +23,12 @@
 
 #include "lib/log/log.h"
 #include "lib/encoding/binascii.h"
+#include "lib/encoding/pem.h"
 
 #include <string.h>
+#ifdef HAVE_SYS_STAT_H
+#include <sys/stat.h>
+#endif
 
 /** Return the number of bytes added by padding method <b>padding</b>.
  */
@@ -388,3 +392,252 @@ crypto_pk_get_common_digests(crypto_pk_t *pk, common_digests_t *digests_out)
   tor_free(buf);
   return rv;
 }
+
+static const char RSA_PUBLIC_TAG[] = "RSA PUBLIC KEY";
+static const char RSA_PRIVATE_TAG[] = "RSA PRIVATE KEY";
+
+/** PEM-encode the public key portion of <b>env</b> and write it to a
+ * newly allocated string.  On success, set *<b>dest</b> to the new
+ * string, *<b>len</b> to the string's length, and return 0.  On
+ * failure, return -1.
+ */
+int
+crypto_pk_write_public_key_to_string(crypto_pk_t *env,
+                                     char **dest, size_t *len)
+{
+  size_t buflen = crypto_pk_keysize(env) * 3;
+  char *buf = tor_malloc(buflen);
+  char *result = NULL;
+  size_t resultlen = 0;
+  int rv = -1;
+
+  int n = crypto_pk_asn1_encode(env, buf, buflen);
+  if (n < 0)
+    goto done;
+
+  resultlen = pem_encoded_size(n, RSA_PUBLIC_TAG);
+  result = tor_malloc(resultlen);
+  if (pem_encode(result, resultlen,
+                 (const unsigned char *)buf, n, RSA_PUBLIC_TAG) < 0) {
+    goto done;
+  }
+
+  *dest = result;
+  *len = resultlen;
+  rv = 0;
+
+ done:
+  if (rv < 0 && result) {
+    memwipe(result, 0, resultlen);
+    tor_free(result);
+  }
+  memwipe(buf, 0, buflen);
+  tor_free(buf);
+  return rv;
+}
+
+/** PEM-encode the private key portion of <b>env</b> and write it to a
+ * newly allocated string.  On success, set *<b>dest</b> to the new
+ * string, *<b>len</b> to the string's length, and return 0.  On
+ * failure, return -1.
+ */
+int
+crypto_pk_write_private_key_to_string(crypto_pk_t *env,
+                                          char **dest, size_t *len)
+{
+  size_t buflen = crypto_pk_keysize(env) * 16;
+  char *buf = tor_malloc(buflen);
+  char *result = NULL;
+  size_t resultlen = 0;
+  int rv = -1;
+
+  int n = crypto_pk_asn1_encode_private(env, buf, buflen);
+  if (n < 0)
+    goto done;
+
+  resultlen = pem_encoded_size(n, RSA_PRIVATE_TAG);
+  result = tor_malloc(resultlen);
+  if (pem_encode(result, resultlen,
+                 (const unsigned char *)buf, n, RSA_PRIVATE_TAG) < 0)
+    goto done;
+
+  *dest = result;
+  *len = resultlen;
+  rv = 0;
+ done:
+  if (rv < 0 && result) {
+    memwipe(result, 0, resultlen);
+    tor_free(result);
+  }
+  memwipe(buf, 0, buflen);
+  tor_free(buf);
+  return rv;
+}
+
+/** Read a PEM-encoded public key from the first <b>len</b> characters of
+ * <b>src</b>, and store the result in <b>env</b>.  Return 0 on success, -1 on
+ * failure.
+ */
+int
+crypto_pk_read_public_key_from_string(crypto_pk_t *env,
+                                      const char *src, size_t len)
+{
+  if (len == (size_t)-1)
+    len = strlen(src);
+
+  size_t buflen = len;
+  uint8_t *buf = tor_malloc(buflen);
+  int rv = -1;
+
+  int n = pem_decode(buf, buflen, src, len, RSA_PUBLIC_TAG);
+  if (n < 0)
+    goto done;
+
+  crypto_pk_t *pk = crypto_pk_asn1_decode((const char*)buf, n);
+  if (! pk)
+    goto done;
+
+  crypto_pk_assign_public(env, pk);
+  crypto_pk_free(pk);
+  rv = 0;
+
+ done:
+  memwipe(buf, 0, buflen);
+  tor_free(buf);
+  return rv;
+}
+
+/** Read a PEM-encoded private key from the <b>len</b>-byte string <b>s</b>
+ * into <b>env</b>.  Return 0 on success, -1 on failure.  If len is -1,
+ * the string is nul-terminated.
+ */
+int
+crypto_pk_read_private_key_from_string(crypto_pk_t *env,
+                                       const char *s, ssize_t len)
+{
+  if (len == -1)
+    len = strlen(s);
+
+  size_t buflen = len;
+  uint8_t *buf = tor_malloc(buflen);
+  int rv = -1;
+
+  int n = pem_decode(buf, buflen, s, len, RSA_PRIVATE_TAG);
+  if (n < 0) {
+    goto done;
+  }
+
+  crypto_pk_t *pk = crypto_pk_asn1_decode_private((const char *)buf, n);
+  if (! pk)
+    goto done;
+
+  crypto_pk_assign_private(env, pk);
+  crypto_pk_free(pk);
+  rv = 0;
+
+ done:
+  memwipe(buf, 0, buflen);
+  tor_free(buf);
+  return rv;
+}
+
+/** Read a PEM-encoded private key from the file named by
+ * <b>keyfile</b> into <b>env</b>.  Return 0 on success, -1 on failure.
+ */
+int
+crypto_pk_read_private_key_from_filename(crypto_pk_t *env,
+                                         const char *keyfile)
+{
+  struct stat st;
+  char *buf = read_file_to_str(keyfile, 0, &st);
+  if (!buf)
+    return -1;
+
+  int rv = crypto_pk_read_private_key_from_string(env, buf, st.st_size);
+  memwipe(buf, 0, st.st_size);
+  tor_free(buf);
+  return rv;
+}
+
+/** Write the private key from <b>env</b> into the file named by <b>fname</b>,
+ * PEM-encoded.  Return 0 on success, -1 on failure.
+ */
+int
+crypto_pk_write_private_key_to_filename(crypto_pk_t *env,
+                                        const char *fname)
+{
+  char *s = NULL;
+  size_t n = 0;
+
+  if (crypto_pk_write_private_key_to_string(env, &s, &n) < 0)
+    return -1;
+
+  int rv = write_bytes_to_file(fname, s, n, 0);
+  memwipe(s, 0, n);
+  tor_free(s);
+  return rv;
+}
+
+/** Given a crypto_pk_t <b>pk</b>, allocate a new buffer containing the
+ * Base64 encoding of the DER representation of the private key as a NUL
+ * terminated string, and return it via <b>priv_out</b>.  Return 0 on
+ * success, -1 on failure.
+ *
+ * It is the caller's responsibility to sanitize and free the resulting buffer.
+ */
+int
+crypto_pk_base64_encode_private(const crypto_pk_t *pk, char **priv_out)
+{
+  size_t buflen = crypto_pk_keysize(pk)*16;
+  char *buf = tor_malloc(buflen);
+  char *result = NULL;
+  size_t reslen = 0;
+  bool ok = false;
+
+  int n = crypto_pk_asn1_encode_private(pk, buf, buflen);
+
+  if (n < 0)
+    goto done;
+
+  reslen = base64_encode_size(n, 0)+1;
+  result = tor_malloc(reslen);
+  if (base64_encode(result, reslen, buf, n, 0) < 0)
+    goto done;
+
+  ok = true;
+
+ done:
+  memwipe(buf, 0, buflen);
+  tor_free(buf);
+  if (result && ! ok) {
+    memwipe(result, 0, reslen);
+    tor_free(result);
+  }
+  *priv_out = result;
+  return ok ? 0 : -1;
+}
+
+/** Given a string containing the Base64 encoded DER representation of the
+ * private key <b>str</b>, decode and return the result on success, or NULL
+ * on failure.
+ */
+crypto_pk_t *
+crypto_pk_base64_decode_private(const char *str, size_t len)
+{
+  crypto_pk_t *pk = NULL;
+
+  char *der = tor_malloc_zero(len + 1);
+  int der_len = base64_decode(der, len, str, len);
+  if (der_len <= 0) {
+    log_warn(LD_CRYPTO, "Stored RSA private key seems corrupted (base64).");
+    goto out;
+  }
+
+  pk = crypto_pk_asn1_decode_private(der, der_len);
+
+ out:
+  memwipe(der, 0, len+1);
+  tor_free(der);
+
+  return pk;
+}

src/lib/crypt_ops/crypto_rsa.h

@@ -93,6 +93,9 @@ int crypto_pk_private_sign(const crypto_pk_t *env, char *to, size_t tolen,
                            const char *from, size_t fromlen);
 int crypto_pk_asn1_encode(const crypto_pk_t *pk, char *dest, size_t dest_len);
 crypto_pk_t *crypto_pk_asn1_decode(const char *str, size_t len);
+int crypto_pk_asn1_encode_private(const crypto_pk_t *pk,
+                                  char *dest, size_t dest_len);
+crypto_pk_t *crypto_pk_asn1_decode_private(const char *str, size_t len);
 int crypto_pk_get_fingerprint(crypto_pk_t *pk, char *fp_out,int add_space);
 int crypto_pk_get_hashed_fingerprint(crypto_pk_t *pk, char *fp_out);
 void crypto_add_spaces_to_fp(char *out, size_t outlen, const char *in);
@@ -107,6 +110,14 @@ int crypto_pk_get_common_digests(crypto_pk_t *pk,
 int crypto_pk_base64_encode_private(const crypto_pk_t *pk, char **priv_out);
 crypto_pk_t *crypto_pk_base64_decode_private(const char *str, size_t len);
 
+#ifdef TOR_UNIT_TESTS
+#ifdef ENABLE_NSS
+struct SECItemStr;
+STATIC int secitem_uint_cmp(const struct SECItemStr *a,
+                            const struct SECItemStr *b);
+#endif
+#endif
+
 #ifdef ENABLE_OPENSSL
 /* Prototypes for private functions only used by tortls.c, crypto.c, and the
  * unit tests. */
@@ -118,8 +129,7 @@ MOCK_DECL(struct evp_pkey_st *, crypto_pk_get_openssl_evp_pkey_,(
                                  crypto_pk_t *env,int private));
 #endif
 
-#ifdef TOR_UNIT_TESTS
+void crypto_pk_assign_public(crypto_pk_t *dest, const crypto_pk_t *src);
-void crypto_pk_assign_(crypto_pk_t *dest, const crypto_pk_t *src);
+void crypto_pk_assign_private(crypto_pk_t *dest, const crypto_pk_t *src);
-#endif
 
 #endif

src/lib/crypt_ops/crypto_rsa_openssl.c

@@ -183,216 +183,6 @@ crypto_pk_generate_key_with_bits,(crypto_pk_t *env, int bits))
   return 0;
 }
 
-/** A PEM callback that always reports a failure to get a password */
-static int
-pem_no_password_cb(char *buf, int size, int rwflag, void *u)
-{
-  (void)buf;
-  (void)size;
-  (void)rwflag;
-  (void)u;
-  return -1;
-}
-
-/** Read a PEM-encoded private key from the <b>len</b>-byte string <b>s</b>
- * into <b>env</b>.  Return 0 on success, -1 on failure.  If len is -1,
- * the string is nul-terminated.
- */
-int
-crypto_pk_read_private_key_from_string(crypto_pk_t *env,
-                                       const char *s, ssize_t len)
-{
-  BIO *b;
-
-  tor_assert(env);
-  tor_assert(s);
-  tor_assert(len < INT_MAX && len < SSIZE_T_CEILING);
-
-  /* Create a read-only memory BIO, backed by the string 's' */
-  b = BIO_new_mem_buf((char*)s, (int)len);
-  if (!b)
-    return -1;
-
-  if (env->key)
-    RSA_free(env->key);
-
-  env->key = PEM_read_bio_RSAPrivateKey(b,NULL,pem_no_password_cb,NULL);
-
-  BIO_free(b);
-
-  if (!env->key) {
-    crypto_openssl_log_errors(LOG_WARN, "Error parsing private key");
-    return -1;
-  }
-  return 0;
-}
-
-/** Read a PEM-encoded private key from the file named by
- * <b>keyfile</b> into <b>env</b>.  Return 0 on success, -1 on failure.
- */
-int
-crypto_pk_read_private_key_from_filename(crypto_pk_t *env,
-                                         const char *keyfile)
-{
-  char *contents;
-  int r;
-
-  /* Read the file into a string. */
-  contents = read_file_to_str(keyfile, 0, NULL);
-  if (!contents) {
-    log_warn(LD_CRYPTO, "Error reading private key from \"%s\"", keyfile);
-    return -1;
-  }
-
-  /* Try to parse it. */
-  r = crypto_pk_read_private_key_from_string(env, contents, -1);
-  memwipe(contents, 0, strlen(contents));
-  tor_free(contents);
-  if (r)
-    return -1; /* read_private_key_from_string already warned, so we don't.*/
-
-  /* Make sure it's valid. */
-  if (crypto_pk_check_key(env) <= 0)
-    return -1;
-
-  return 0;
-}
-
-/** Helper function to implement crypto_pk_write_*_key_to_string. Return 0 on
- * success, -1 on failure. */
-static int
-crypto_pk_write_key_to_string_impl(crypto_pk_t *env, char **dest,
-                                   size_t *len, int is_public)
-{
-  BUF_MEM *buf;
-  BIO *b;
-  int r;
-
-  tor_assert(env);
-  tor_assert(env->key);
-  tor_assert(dest);
-
-  b = BIO_new(BIO_s_mem()); /* Create a memory BIO */
-  if (!b)
-    return -1;
-
-  /* Now you can treat b as if it were a file.  Just use the
-   * PEM_*_bio_* functions instead of the non-bio variants.
-   */
-  if (is_public)
-    r = PEM_write_bio_RSAPublicKey(b, env->key);
-  else
-    r = PEM_write_bio_RSAPrivateKey(b, env->key, NULL,NULL,0,NULL,NULL);
-
-  if (!r) {
-    crypto_openssl_log_errors(LOG_WARN, "writing RSA key to string");
-    BIO_free(b);
-    return -1;
-  }
-
-  BIO_get_mem_ptr(b, &buf);
-
-  *dest = tor_malloc(buf->length+1);
-  memcpy(*dest, buf->data, buf->length);
-  (*dest)[buf->length] = 0; /* nul terminate it */
-  *len = buf->length;
-
-  BIO_free(b);
-
-  return 0;
-}
-
-/** PEM-encode the public key portion of <b>env</b> and write it to a
- * newly allocated string.  On success, set *<b>dest</b> to the new
- * string, *<b>len</b> to the string's length, and return 0.  On
- * failure, return -1.
- */
-int
-crypto_pk_write_public_key_to_string(crypto_pk_t *env, char **dest,
-                                     size_t *len)
-{
-  return crypto_pk_write_key_to_string_impl(env, dest, len, 1);
-}
-
-/** PEM-encode the private key portion of <b>env</b> and write it to a
- * newly allocated string.  On success, set *<b>dest</b> to the new
- * string, *<b>len</b> to the string's length, and return 0.  On
- * failure, return -1.
- */
-int
-crypto_pk_write_private_key_to_string(crypto_pk_t *env, char **dest,
-                                     size_t *len)
-{
-  return crypto_pk_write_key_to_string_impl(env, dest, len, 0);
-}
-
-/** Read a PEM-encoded public key from the first <b>len</b> characters of
- * <b>src</b>, and store the result in <b>env</b>.  Return 0 on success, -1 on
- * failure.
- */
-int
-crypto_pk_read_public_key_from_string(crypto_pk_t *env, const char *src,
-                                      size_t len)
-{
-  BIO *b;
-
-  tor_assert(env);
-  tor_assert(src);
-  tor_assert(len<INT_MAX);
-
-  b = BIO_new(BIO_s_mem()); /* Create a memory BIO */
-  if (!b)
-    return -1;
-
-  BIO_write(b, src, (int)len);
-
-  if (env->key)
-    RSA_free(env->key);
-  env->key = PEM_read_bio_RSAPublicKey(b, NULL, pem_no_password_cb, NULL);
-  BIO_free(b);
-  if (!env->key) {
-    crypto_openssl_log_errors(LOG_WARN, "reading public key from string");
-    return -1;
-  }
-
-  return 0;
-}
-
-/** Write the private key from <b>env</b> into the file named by <b>fname</b>,
- * PEM-encoded.  Return 0 on success, -1 on failure.
- */
-int
-crypto_pk_write_private_key_to_filename(crypto_pk_t *env,
-                                        const char *fname)
-{
-  BIO *bio;
-  char *cp;
-  long len;
-  char *s;
-  int r;
-
-  tor_assert(crypto_pk_key_is_private(env));
-
-  if (!(bio = BIO_new(BIO_s_mem())))
-    return -1;
-  if (PEM_write_bio_RSAPrivateKey(bio, env->key, NULL,NULL,0,NULL,NULL)
-      == 0) {
-    crypto_openssl_log_errors(LOG_WARN, "writing private key");
-    BIO_free(bio);
-    return -1;
-  }
-  len = BIO_get_mem_data(bio, &cp);
-  tor_assert(len >= 0);
-  s = tor_malloc(len+1);
-  memcpy(s, cp, len);
-  s[len]='\0';
-  r = write_str_to_file(fname, s, 0);
-  BIO_free(bio);
-  memwipe(s, 0, strlen(s));
-  tor_free(s);
-  return r;
-}
-
 /** Return true iff <b>env</b> has a valid key.
  */
 int
@@ -512,11 +302,11 @@ crypto_pk_dup_key(crypto_pk_t *env)
   return env;
 }
 
-#ifdef TOR_UNIT_TESTS
+/** Replace dest with src (private key only).  (Dest must have a refcount
-/** For testing: replace dest with src.  (Dest must have a refcount
+ * of 1)
- * of 1) */
+ */
 void
-crypto_pk_assign_(crypto_pk_t *dest, const crypto_pk_t *src)
+crypto_pk_assign_private(crypto_pk_t *dest, const crypto_pk_t *src)
 {
   tor_assert(dest);
   tor_assert(dest->refs == 1);
@@ -524,7 +314,19 @@ crypto_pk_assign_(crypto_pk_t *dest, const crypto_pk_t *src)
   RSA_free(dest->key);
   dest->key = RSAPrivateKey_dup(src->key);
 }
-#endif /* defined(TOR_UNIT_TESTS) */
+
+/** Replace dest with src (public key only).  (Dest must have a refcount
+ * of 1)
+ */
+void
+crypto_pk_assign_public(crypto_pk_t *dest, const crypto_pk_t *src)
+{
+  tor_assert(dest);
+  tor_assert(dest->refs == 1);
+  tor_assert(src);
+  RSA_free(dest->key);
+  dest->key = RSAPublicKey_dup(src->key);
+}
 
 /** Make a real honest-to-goodness copy of <b>env</b>, and return it.
  * Returns NULL on failure. */
@@ -733,74 +535,48 @@ crypto_pk_asn1_decode(const char *str, size_t len)
   return crypto_new_pk_from_openssl_rsa_(rsa);
 }
 
-/** Given a crypto_pk_t <b>pk</b>, allocate a new buffer containing the
+/** ASN.1-encode the private portion of <b>pk</b> into <b>dest</b>.
- * Base64 encoding of the DER representation of the private key as a NUL
+ * Return -1 on error, or the number of characters used on success.
- * terminated string, and return it via <b>priv_out</b>.  Return 0 on
- * success, -1 on failure.
- *
- * It is the caller's responsibility to sanitize and free the resulting buffer.
  */
 int
-crypto_pk_base64_encode_private(const crypto_pk_t *pk, char **priv_out)
+crypto_pk_asn1_encode_private(const crypto_pk_t *pk, char *dest,
+                              size_t dest_len)
 {
-  unsigned char *der = NULL;
+  int len;
-  int der_len;
+  unsigned char *buf = NULL;
-  int ret = -1;
 
-  *priv_out = NULL;
+  len = i2d_RSAPrivateKey(pk->key, &buf);
+  if (len < 0 || buf == NULL)
+    return -1;
 
-  der_len = i2d_RSAPrivateKey(pk->key, &der);
+  if ((size_t)len > dest_len || dest_len > SIZE_T_CEILING) {
-  if (der_len < 0 || der == NULL)
+    OPENSSL_free(buf);
-    return ret;
+    return -1;
-
-  size_t priv_len = base64_encode_size(der_len, 0) + 1;
-  char *priv = tor_malloc_zero(priv_len);
-  if (base64_encode(priv, priv_len, (char *)der, der_len, 0) >= 0) {
-    *priv_out = priv;
-    ret = 0;
-  } else {
-    tor_free(priv);
   }
-
+  /* We don't encode directly into 'dest', because that would be illegal
-  memwipe(der, 0, der_len);
+   * type-punning.  (C99 is smarter than me, C99 is smarter than me...)
-  OPENSSL_free(der);
+   */
-  return ret;
+  memcpy(dest,buf,len);
+  OPENSSL_free(buf);
+  return len;
 }
 
-/** Given a string containing the Base64 encoded DER representation of the
+/** Decode an ASN.1-encoded private key from <b>str</b>; return the result on
- * private key <b>str</b>, decode and return the result on success, or NULL
+ * success and NULL on failure.
- * on failure.
  */
 crypto_pk_t *
-crypto_pk_base64_decode_private(const char *str, size_t len)
+crypto_pk_asn1_decode_private(const char *str, size_t len)
 {
-  crypto_pk_t *pk = NULL;
+  RSA *rsa;
-
+  unsigned char *buf;
-  char *der = tor_malloc_zero(len + 1);
+  const unsigned char *cp;
-  int der_len = base64_decode(der, len, str, len);
+  cp = buf = tor_malloc(len);
-  if (der_len <= 0) {
+  memcpy(buf,str,len);
-    log_warn(LD_CRYPTO, "Stored RSA private key seems corrupted (base64).");
+  rsa = d2i_RSAPrivateKey(NULL, &cp, len);
-    goto out;
+  tor_free(buf);
-  }
-
-  const unsigned char *dp = (unsigned char*)der; /* Shut the compiler up. */
-  RSA *rsa = d2i_RSAPrivateKey(NULL, &dp, der_len);
   if (!rsa) {
-    crypto_openssl_log_errors(LOG_WARN, "decoding private key");
+    crypto_openssl_log_errors(LOG_WARN,"decoding public key");
-    goto out;
+    return NULL;
   }
-
+  return crypto_new_pk_from_openssl_rsa_(rsa);
-  pk = crypto_new_pk_from_openssl_rsa_(rsa);
-
-  /* Make sure it's valid. */
-  if (crypto_pk_check_key(pk) <= 0) {
-    crypto_pk_free(pk);
-    pk = NULL;
-    goto out;
-  }
-
- out:
-  memwipe(der, 0, len + 1);
-  tor_free(der);
-  return pk;
 }

src/test/test_crypto.c

@@ -1519,7 +1519,8 @@ test_crypto_digests(void *arg)
   (void)arg;
   k = crypto_pk_new();
   tt_assert(k);
-  r = crypto_pk_read_private_key_from_string(k, AUTHORITY_SIGNKEY_3, -1);
+  r = crypto_pk_read_private_key_from_string(k, AUTHORITY_SIGNKEY_3,
+                                             strlen(AUTHORITY_SIGNKEY_3));
   tt_assert(!r);
 
   r = crypto_pk_get_digest(k, digest);

src/test/testing_rsakeys.c

@@ -490,7 +490,7 @@ crypto_pk_generate_key_with_bits__get_cached(crypto_pk_t *env, int bits)
 {
   if (bits == 1024 || bits == 2048)  {
     crypto_pk_t *newkey = pk_generate_internal(bits);
-    crypto_pk_assign_(env, newkey);
+    crypto_pk_assign_private(env, newkey);
     crypto_pk_free(newkey);
   } else {
     return crypto_pk_generate_key_with_bits__real(env, bits);
@@ -544,4 +544,3 @@ init_pregenerated_keys(void)
        crypto_pk_generate_key_with_bits__get_cached);
 #endif /* defined(USE_PREGENERATED_RSA_KEYS) */
 }
-