Merge branch 'bug26116_029' into maint-0.2.9

3 files changed, 54 insertions, 1 deletions

diff --git a/changes/bug26116 b/changes/bug26116
new file mode 100644
index 0000000000..3bfde74f77
--- /dev/null
+++ b/changes/bug26116
@@ -0,0 +1,7 @@
+  o Minor bugfixes (compatibility, openssl):
+    - Work around a change in OpenSSL 1.1.1 where
+      return values that would previously indicate "no password" now
+      indicate an empty password. Without this workaround, Tor instances
+      running with OpenSSL 1.1.1 would accept descriptors that other Tor
+      instances would reject. Fixes bug 26116; bugfix on 0.2.5.16.
+      
diff --git a/src/common/crypto.c b/src/common/crypto.c
index 39c8cc2b0a..f8495bb107 100644
--- a/src/common/crypto.c
+++ b/src/common/crypto.c
@@ -653,7 +653,12 @@ pem_no_password_cb(char *buf, int size, int rwflag, void *u)
   (void)size;
   (void)rwflag;
   (void)u;
-  return 0;
+  /* The openssl documentation says that a callback "must" return 0 if an
+   * error occurred.  But during the 1.1.1 series (commit c82c3462267afdbbaa5
+   * they changed the interpretation so that 0 indicates an empty password and
+   * -1 indicates an error. We want to reject any encrypted PEM buffers, so we
+   * return -1.  This will work on older OpenSSL versions and LibreSSL too. */
+  return -1;
 }
 
 /** Read a PEM-encoded private key from the <b>len</b>-byte string <b>s</b>
diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c
index 64a46f7914..8fd9ca7671 100644
--- a/src/test/test_crypto.c
+++ b/src/test/test_crypto.c
@@ -1349,6 +1349,46 @@ test_crypto_pk_base64(void *arg)
   tor_free(encoded);
 }
 
+static void
+test_crypto_pk_pem_encrypted(void *arg)
+{
+  crypto_pk_t *pk = NULL;
+  (void)arg;
+
+  pk = crypto_pk_new();
+  /* we need to make sure that we won't stall if somebody gives us a key
+     that's encrypted with a password. */
+  {
+    const char *s =
+      "-----BEGIN RSA PRIVATE KEY-----\n"
+      "Proc-Type: 4,ENCRYPTED\n"
+      "DEK-Info: AES-128-CBC,EFA86BB9D2AB11E80B4E3DCD97782B16\n"
+      "\n"
+      "Z2Je4m0cFepc6coQkVbGcvNCHxTf941N2XYEVE6kn0CqWqoUH4tlwV6for5D91np\n"
+      "5NiEFTkWj31EhrvrYcuiJtQ/iEbABxZULFWFeJ058rb+1izBz5rScqnEacIS/3Go\n"
+      "YntnROBDwiKmUnue6PJVYg==\n"
+      "-----END RSA PRIVATE KEY-----\n";
+    tt_int_op(-1, OP_EQ,
+              crypto_pk_read_private_key_from_string(pk, s, strlen(s)));
+  }
+  /* For fun, make sure we aren't hit by OpenSSL issue
+     https://github.com/openssl/openssl/issues/6347 , where we get in trouble
+     if a cipher doesn't use an IV.
+  */
+  {
+    const char *s =
+      "-----BEGIN RSA PUBLIC KEY-----\n"
+      "Proc-Type:4,ENCRYPTED\n"
+      "DEK-Info:des-ede -\n"
+      "\n"
+      "iRqK\n"
+      "-----END RSA PUBLIC KEY-----\n";
+    tt_int_op(-1, OP_EQ,
+              crypto_pk_read_public_key_from_string(pk, s, strlen(s)));
+  }
+ done:
+  crypto_pk_free(pk);
+}
 #ifdef HAVE_TRUNCATE
 #define do_truncate truncate
 #else
@@ -2914,6 +2954,7 @@ struct testcase_t crypto_tests[] = {
   CRYPTO_LEGACY(pk),
   { "pk_fingerprints", test_crypto_pk_fingerprints, TT_FORK, NULL, NULL },
   { "pk_base64", test_crypto_pk_base64, TT_FORK, NULL, NULL },
+  { "pk_pem_encrypted", test_crypto_pk_pem_encrypted, TT_FORK, NULL, NULL },
   CRYPTO_LEGACY(digests),
   { "digest_names", test_crypto_digest_names, 0, NULL, NULL },
   { "sha3", test_crypto_sha3, TT_FORK, NULL, NULL},