May 2018 - tor-commits - lists.torproject.org

[tor/release-0.3.1] Return -1 from our PEM password callback
by nickm＠torproject.org 24 May '18

24 May '18

commit 881f7157f648eb8a39e5dfd3efb95951ee7ac215 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed May 16 11:39:42 2018 -0400 Return -1 from our PEM password callback Apparently, contrary to its documentation, this is how OpenSSL now wants us to report an error. Fixes bug 26116; bugfix on 0.2.5.16. --- changes/bug26116 | 7 +++++++ src/common/crypto.c | 7 ++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/changes/bug26116 b/changes/bug26116 new file mode 100644 index 000000000..3bfde74f7 --- /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 39c8cc2b0..f8495bb10 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 len-byte string s

1 0

[tor/release-0.3.1] Add a unit test for PEM-encrypted documents.
by nickm＠torproject.org 24 May '18

24 May '18

commit aeb4be1d5a17f8ff836e370f8942c09c66b31e1d Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 24 09:36:33 2018 -0400 Add a unit test for PEM-encrypted documents. --- src/test/test_crypto.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index 64a46f791..8fd9ca767 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},

1 0

[tor/release-0.3.1] Merge branch 'maint-0.3.1' into release-0.3.1
by nickm＠torproject.org 24 May '18

24 May '18

commit 3f8fe441cfd77ceea20cb6a3f5eb76f463e6d88e Merge: 12e465344 f48fb8a72 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 24 09:40:06 2018 -0400 Merge branch 'maint-0.3.1' into release-0.3.1 changes/bug26116 | 7 +++++++ src/common/crypto.c | 7 ++++++- src/test/test_crypto.c | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-)

1 0

[tor/release-0.2.9] Add a unit test for PEM-encrypted documents.
by nickm＠torproject.org 24 May '18

24 May '18

commit aeb4be1d5a17f8ff836e370f8942c09c66b31e1d Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 24 09:36:33 2018 -0400 Add a unit test for PEM-encrypted documents. --- src/test/test_crypto.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index 64a46f791..8fd9ca767 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},

1 0

[tor/release-0.3.1] Merge branch 'maint-0.2.9' into maint-0.3.1
by nickm＠torproject.org 24 May '18

24 May '18

commit f48fb8a720c7cca79249248f92e3bc99cee3414d Merge: d3a972561 c380562ae Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 24 09:40:06 2018 -0400 Merge branch 'maint-0.2.9' into maint-0.3.1 changes/bug26116 | 7 +++++++ src/common/crypto.c | 7 ++++++- src/test/test_crypto.c | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-)

1 0

[tor/release-0.2.9] Merge branch 'bug26116_029' into maint-0.2.9
by nickm＠torproject.org 24 May '18

24 May '18

commit c380562aed5242eab6449b054199d742f02833dd Merge: d1e4ffc71 aeb4be1d5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 24 09:39:46 2018 -0400 Merge branch 'bug26116_029' into maint-0.2.9 changes/bug26116 | 7 +++++++ src/common/crypto.c | 7 ++++++- src/test/test_crypto.c | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-)

1 0

[tor/release-0.3.2] Return -1 from our PEM password callback
by nickm＠torproject.org 24 May '18

24 May '18

commit 881f7157f648eb8a39e5dfd3efb95951ee7ac215 Author: Nick Mathewson <nickm(a)torproject.org> Date: Wed May 16 11:39:42 2018 -0400 Return -1 from our PEM password callback Apparently, contrary to its documentation, this is how OpenSSL now wants us to report an error. Fixes bug 26116; bugfix on 0.2.5.16. --- changes/bug26116 | 7 +++++++ src/common/crypto.c | 7 ++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/changes/bug26116 b/changes/bug26116 new file mode 100644 index 000000000..3bfde74f7 --- /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 39c8cc2b0..f8495bb10 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 len-byte string s

1 0

[tor/release-0.3.2] Add a unit test for PEM-encrypted documents.
by nickm＠torproject.org 24 May '18

24 May '18

commit aeb4be1d5a17f8ff836e370f8942c09c66b31e1d Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 24 09:36:33 2018 -0400 Add a unit test for PEM-encrypted documents. --- src/test/test_crypto.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index 64a46f791..8fd9ca767 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},

1 0

[tor/release-0.3.3] Merge branch 'bug26116_029' into maint-0.2.9
by nickm＠torproject.org 24 May '18

24 May '18

commit c380562aed5242eab6449b054199d742f02833dd Merge: d1e4ffc71 aeb4be1d5 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 24 09:39:46 2018 -0400 Merge branch 'bug26116_029' into maint-0.2.9 changes/bug26116 | 7 +++++++ src/common/crypto.c | 7 ++++++- src/test/test_crypto.c | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 1 deletion(-)

1 0

[tor/release-0.3.3] Add a unit test for PEM-encrypted documents.
by nickm＠torproject.org 24 May '18

24 May '18

commit aeb4be1d5a17f8ff836e370f8942c09c66b31e1d Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu May 24 09:36:33 2018 -0400 Add a unit test for PEM-encrypted documents. --- src/test/test_crypto.c | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index 64a46f791..8fd9ca767 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},

1 0