[tor-commits] [tor/master] Log more messages when failing to decode RSA keys

[nickm at torproject.org]

commit 0981ac4c59f04a9de76224abf75a94a6fcc0542c
Author: Nick Mathewson <nickm at torproject.org>
Date:   Wed Jan 16 14:33:40 2019 -0500

    Log more messages when failing to decode RSA keys
    
    We log these messages at INFO level, except when we are reading a
    private key from a file, in which case we log at WARN.
    
    This fixes a regression from when we re-wrote our PEM code to be
    generic between nss and openssl.
    
    Fixes bug 29042, bugfix on 0.3.5.1-alpha.
---
 changes/bug29042               |  5 +++++
 src/lib/crypt_ops/crypto_rsa.c | 34 ++++++++++++++++++++++++++--------
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/changes/bug29042 b/changes/bug29042
new file mode 100644
index 000000000..ecbcdeeec
--- /dev/null
+++ b/changes/bug29042
@@ -0,0 +1,5 @@
+  o Minor bugfixes (logging):
+    - Log more information at "warning" level when unable to read a private
+      key; log more information ad "info" level when unable to read a public
+      key. We had warnings here before, but they were lost during our
+      NSS work. Fixes bug 28981; bugfix on 0.3.5.1-alpha.
diff --git a/src/lib/crypt_ops/crypto_rsa.c b/src/lib/crypt_ops/crypto_rsa.c
index 8cc62bc18..2b977b0b9 100644
--- a/src/lib/crypt_ops/crypto_rsa.c
+++ b/src/lib/crypt_ops/crypto_rsa.c
@@ -21,6 +21,7 @@
 #include "lib/log/util_bug.h"
 #include "lib/fs/files.h"
 
+#include "lib/log/escape.h"
 #include "lib/log/log.h"
 #include "lib/encoding/binascii.h"
 #include "lib/encoding/pem.h"
@@ -484,11 +485,13 @@ crypto_pk_write_private_key_to_string(crypto_pk_t *env,
  */
 static int
 crypto_pk_read_from_string_generic(crypto_pk_t *env, const char *src,
-                                   size_t len, bool private_key)
+                                   size_t len, int severity,
+                                   bool private_key)
 {
   if (len == (size_t)-1) // "-1" indicates "use the length of the string."
     len = strlen(src);
 
+  const char *ktype = private_key ? "private key" : "public key";
   const char *tag =
     private_key ? RSA_PRIVATE_TAG : RSA_PUBLIC_TAG;
   size_t buflen = len;
@@ -496,14 +499,20 @@ crypto_pk_read_from_string_generic(crypto_pk_t *env, const char *src,
   int rv = -1;
 
   int n = pem_decode(buf, buflen, src, len, tag);
-  if (n < 0)
+  if (n < 0) {
+    log_fn(severity, LD_CRYPTO,
+           "Error decoding PEM wrapper while reading %s", ktype);
     goto done;
+  }
 
   crypto_pk_t *pk = private_key
     ? crypto_pk_asn1_decode_private((const char*)buf, n)
     : crypto_pk_asn1_decode((const char*)buf, n);
-  if (! pk)
+  if (! pk) {
+    log_fn(severity, LD_CRYPTO,
+           "Error decoding ASN.1 while reading %s", ktype);
     goto done;
+  }
 
   if (private_key)
     crypto_pk_assign_private(env, pk);
@@ -526,7 +535,7 @@ int
 crypto_pk_read_public_key_from_string(crypto_pk_t *env,
                                       const char *src, size_t len)
 {
-  return crypto_pk_read_from_string_generic(env, src, len, false);
+  return crypto_pk_read_from_string_generic(env, src, len, LOG_INFO, false);
 }
 
 /** Read a PEM-encoded private key from the <b>len</b>-byte string <b>src</b>
@@ -537,7 +546,7 @@ int
 crypto_pk_read_private_key_from_string(crypto_pk_t *env,
                                        const char *src, ssize_t len)
 {
-  return crypto_pk_read_from_string_generic(env, src, len, true);
+  return crypto_pk_read_from_string_generic(env, src, len, LOG_INFO, true);
 }
 
 /** If a file is longer than this, we won't try to decode its private key */
@@ -552,15 +561,24 @@ crypto_pk_read_private_key_from_filename(crypto_pk_t *env,
 {
   struct stat st;
   char *buf = read_file_to_str(keyfile, 0, &st);
-  if (!buf)
+  if (!buf) {
+    log_warn(LD_CRYPTO, "Unable to read file for private key in %s",
+             escaped(keyfile));
     return -1;
+  }
   if (st.st_size > MAX_PRIVKEY_FILE_LEN) {
+    log_warn(LD_CRYPTO, "Private key file %s was far too large.",
+             escaped(keyfile));
     tor_free(buf);
     return -1;
   }
 
-  int rv = crypto_pk_read_private_key_from_string(env, buf,
-                                                  (ssize_t)st.st_size);
+  int rv = crypto_pk_read_from_string_generic(env, buf, (ssize_t)st.st_size,
+                                              LOG_WARN, true);
+  if (rv < 0) {
+    log_warn(LD_CRYPTO, "Unable to decode private key from file %s",
+             escaped(keyfile));
+  }
   memwipe(buf, 0, (size_t)st.st_size);
   tor_free(buf);
   return rv;