April 2017 - tor-commits - lists.torproject.org

[tor/master] Remove some now-needless openssl includes from src/test.
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit 706c44a6ce0bbeee51c800521a3199d76e1dcd96 Author: Nick Mathewson <nickm(a)torproject.org> Date: Sat Mar 25 12:19:14 2017 +0100 Remove some now-needless openssl includes from src/test. It would appear that these includes weren't actually used. --- src/test/test_crypto_slow.c | 3 --- src/test/test_microdesc.c | 6 ------ src/test/test_rendcache.c | 1 - 3 files changed, 10 deletions(-) diff --git a/src/test/test_crypto_slow.c b/src/test/test_crypto_slow.c index 6d676ff..d6b0a43 100644 --- a/src/test/test_crypto_slow.c +++ b/src/test/test_crypto_slow.c @@ -12,11 +12,8 @@ #if defined(HAVE_LIBSCRYPT_H) && defined(HAVE_LIBSCRYPT_SCRYPT) #define HAVE_LIBSCRYPT -#include <libscrypt.h> #endif -#include <openssl/evp.h> - /** Run unit tests for our secret-to-key passphrase hashing functionality. */ static void test_crypto_s2k_rfc2440(void *arg) diff --git a/src/test/test_microdesc.c b/src/test/test_microdesc.c index b8d9649..c78fda3 100644 --- a/src/test/test_microdesc.c +++ b/src/test/test_microdesc.c @@ -14,12 +14,6 @@ #include "test.h" -DISABLE_GCC_WARNING(redundant-decls) -#include <openssl/rsa.h> -#include <openssl/bn.h> -#include <openssl/pem.h> -ENABLE_GCC_WARNING(redundant-decls) - #ifdef _WIN32 /* For mkdir() */ #include <direct.h> diff --git a/src/test/test_rendcache.c b/src/test/test_rendcache.c index 59d3be9..feba8f6 100644 --- a/src/test/test_rendcache.c +++ b/src/test/test_rendcache.c @@ -11,7 +11,6 @@ #include "routerlist.h" #include "config.h" #include "hs_common.h" -#include <openssl/rsa.h> #include "rend_test_helpers.h" #include "log_test_helpers.h"

1 0

[tor/master] Merge branch 'isolate_openssl'
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit 61f1838cdc3dc11c6154c137b39817081dc0849f Merge: 1e54bdd f5cc8da Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 7 09:58:21 2017 -0400 Merge branch 'isolate_openssl' changes/ticket21841 | 4 ++ src/common/crypto.c | 12 ++++ src/common/crypto.h | 4 ++ src/common/crypto_ed25519.c | 16 ++--- src/common/tortls.c | 19 +++++ src/common/tortls.h | 46 ++++++++---- src/ext/ed25519/donna/ed25519-hash-custom.h | 31 ++++++++ src/ext/ed25519/ref10/crypto_hash_sha512.h | 30 ++++---- src/ext/include.am | 1 + src/or/main.c | 5 +- src/test/include.am | 1 + src/test/test.c | 1 + src/test/test.h | 1 + src/test/test_crypto.c | 72 ------------------- src/test/test_crypto_openssl.c | 107 ++++++++++++++++++++++++++++ src/test/test_crypto_slow.c | 3 - src/test/test_link_handshake.c | 24 ++----- src/test/test_microdesc.c | 6 -- src/test/test_rendcache.c | 1 - src/test/test_tortls.c | 1 + src/test/testing_common.c | 5 +- 21 files changed, 249 insertions(+), 141 deletions(-)

1 0

[tor/master] Changes file for openssl header isolation.
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit f3e399c6cefa846d8512cc90f11214ca582ff8d1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Mar 31 10:12:25 2017 -0400 Changes file for openssl header isolation. --- changes/ticket21841 | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/changes/ticket21841 b/changes/ticket21841 new file mode 100644 index 0000000..08c7406 --- /dev/null +++ b/changes/ticket21841 @@ -0,0 +1,4 @@ + o Code simplification and refactoring: + - Isolate our usage of the openssl headers so that they are only + used from our crypto wrapper modules, and from tests that examing those + modules' internals. Closes ticket 21841.

1 0

[tor/master] Move "change cert expiration and re-sign" fn into tortls.c
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit 28f9b68e8763b2e5c4e7b0ed9170f41ead15feb1 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Mar 31 10:04:10 2017 -0400 Move "change cert expiration and re-sign" fn into tortls.c This lets test_link_handshake stop including openssl headers. --- src/common/tortls.c | 18 ++++++++++++++++++ src/common/tortls.h | 5 +++++ src/test/test_link_handshake.c | 23 ++++++----------------- 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/common/tortls.c b/src/common/tortls.c index aa19140..fadf52f 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -2264,6 +2264,24 @@ check_cert_lifetime_internal(int severity, const X509 *cert, return 0; } +#ifdef TOR_UNIT_TESTS +/* Testing only: return a new x509 cert with the same contents as inp, + but with the expiration time new_expiration_time, signed with + signing_key. */ +STATIC tor_x509_cert_t * +tor_x509_cert_replace_expiration(const tor_x509_cert_t *inp, + time_t new_expiration_time, + crypto_pk_t *signing_key) +{ + X509 *newc = X509_dup(inp->cert); + X509_time_adj(X509_get_notAfter(newc), 0, &new_expiration_time); + EVP_PKEY *pk = crypto_pk_get_evp_pkey_(signing_key, 1); + tor_assert(X509_sign(newc, pk, EVP_sha256())); + EVP_PKEY_free(pk); + return tor_x509_cert_new(newc); +} +#endif + /** Return the number of bytes available for reading from tls. */ int diff --git a/src/common/tortls.h b/src/common/tortls.h index a848039..fd0186c 100644 --- a/src/common/tortls.h +++ b/src/common/tortls.h @@ -187,6 +187,11 @@ extern tor_tls_context_t *client_tls_context; extern uint16_t v2_cipher_list[]; extern uint64_t total_bytes_written_over_tls; extern uint64_t total_bytes_written_by_tls; + +STATIC tor_x509_cert_t *tor_x509_cert_replace_expiration( + const tor_x509_cert_t *inp, + time_t new_expiration_time, + crypto_pk_t *signing_key); #endif #endif /* endif TORTLS_PRIVATE */ diff --git a/src/test/test_link_handshake.c b/src/test/test_link_handshake.c index 3f2f2b6..217088e 100644 --- a/src/test/test_link_handshake.c +++ b/src/test/test_link_handshake.c @@ -10,12 +10,6 @@ #include "compat.h" -/* Some versions of OpenSSL declare SSL_get_selected_srtp_profile twice in - * srtp.h. Suppress the GCC warning so we can build with -Wredundant-decl. */ -DISABLE_GCC_WARNING(redundant-decls) -#include <openssl/x509.h> -ENABLE_GCC_WARNING(redundant-decls) - #include "or.h" #include "config.h" #include "connection.h" @@ -784,19 +778,14 @@ CERTS_FAIL(expired_rsa_id, /* both */ certs_cell_cert_t *cert = certs_cell_get_certs(d->ccell, 1); const tor_x509_cert_t *idc; tor_tls_get_my_certs(1, NULL, &idc); - X509 *newc = X509_dup(idc->cert); + tor_x509_cert_t *newc; time_t new_end = time(NULL) - 86400 * 10; - X509_time_adj(X509_get_notAfter(newc), 0, &new_end); - EVP_PKEY *pk = crypto_pk_get_evp_pkey_(d->key2, 1); - tt_assert(X509_sign(newc, pk, EVP_sha1())); - int len = i2d_X509(newc, NULL); - certs_cell_cert_setlen_body(cert, len); - uint8_t *body = certs_cell_cert_getarray_body(cert); - int len2 = i2d_X509(newc, &body); - tt_int_op(len, ==, len2); + newc = tor_x509_cert_replace_expiration(idc, new_end, d->key2); + certs_cell_cert_setlen_body(cert, newc->encoded_len); + memcpy(certs_cell_cert_getarray_body(cert), + newc->encoded, newc->encoded_len); REENCODE(); - X509_free(newc); - EVP_PKEY_free(pk); + tor_x509_cert_free(newc); }) CERTS_FAIL(expired_ed_id, /* ed25519 */ {

1 0

[tor/master] Change many tortls.h declarations of private APIs to use structs
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit a8b9aba912c79e054dfdc5c6c7e64a42bc6ec541 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Mar 31 09:44:36 2017 -0400 Change many tortls.h declarations of private APIs to use structs This change makes it so those those APIs will not require prior inclusion of openssl headers. I've left some APIs alone-- those will change to be extra-private. --- src/common/tortls.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/common/tortls.c b/src/common/tortls.c index 3034fbc..aa19140 100644 --- a/src/common/tortls.c +++ b/src/common/tortls.c @@ -17,6 +17,7 @@ #include "orconfig.h" #define TORTLS_PRIVATE +#define TORTLS_OPENSSL_PRIVATE #include <assert.h> #ifdef _WIN32 /*wrkard for dtls1.h >= 0.9.8m of "#include <winsock.h>"*/

1 0

[tor/master] Use DIGEST512_LEN macro in crypto_hash_sha512.h in ref10
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit f5cc8da7e0dc5ba52a1a6a7744e749f4f46c35bc Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 7 09:58:04 2017 -0400 Use DIGEST512_LEN macro in crypto_hash_sha512.h in ref10 --- src/ext/ed25519/ref10/crypto_hash_sha512.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ext/ed25519/ref10/crypto_hash_sha512.h b/src/ext/ed25519/ref10/crypto_hash_sha512.h index e454c4e..5dad935 100644 --- a/src/ext/ed25519/ref10/crypto_hash_sha512.h +++ b/src/ext/ed25519/ref10/crypto_hash_sha512.h @@ -13,7 +13,7 @@ sha_ctx_ = crypto_digest512_new(DIGEST_SHA512); \ crypto_digest_add_bytes(sha_ctx_, (const char *)(inp1), (len1)); \ crypto_digest_add_bytes(sha_ctx_, (const char *)(inp2), (len2)); \ - crypto_digest_get_digest(sha_ctx_, (char *)out, 64); \ + crypto_digest_get_digest(sha_ctx_, (char *)out, DIGEST512_LEN); \ crypto_digest_free(sha_ctx_); \ } while (0) @@ -27,6 +27,6 @@ crypto_digest_add_bytes(sha_ctx_, (const char *)(inp1), (len1)); \ crypto_digest_add_bytes(sha_ctx_, (const char *)(inp2), (len2)); \ crypto_digest_add_bytes(sha_ctx_, (const char *)(inp3), (len3)); \ - crypto_digest_get_digest(sha_ctx_, (char *)out, 64); \ + crypto_digest_get_digest(sha_ctx_, (char *)out, DIGEST512_LEN); \ crypto_digest_free(sha_ctx_); \ } while(0)

1 0

[tor/master] Use baseXX length macros in baseXX_encode()
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit 86477f4e3fedb316e5708f4e7fd09ea61840993a Author: Taylor Yu <catalyst(a)torproject.org> Date: Thu Apr 6 15:17:28 2017 -0400 Use baseXX length macros in baseXX_encode() Use the new baseXX length macros to clean up the length checks in the baseXX_encode() functions. --- src/common/util_format.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/common/util_format.c b/src/common/util_format.c index 51b299b..80844af 100644 --- a/src/common/util_format.c +++ b/src/common/util_format.c @@ -28,7 +28,8 @@ size_t base32_encoded_size(size_t srclen) { size_t enclen; - enclen = CEIL_DIV(srclen*8, 5) + 1; + tor_assert(srclen < SIZE_T_CEILING / 8); + enclen = BASE32_NOPAD_BUFSIZE(srclen); tor_assert(enclen < INT_MAX && enclen > srclen); return enclen; } @@ -41,7 +42,6 @@ base32_encode(char *dest, size_t destlen, const char *src, size_t srclen) size_t nbits = srclen * 8; size_t bit; - tor_assert(srclen < SIZE_T_CEILING/8); /* We need enough space for the encoded data and the extra NUL byte. */ tor_assert(base32_encoded_size(srclen) <= destlen); tor_assert(destlen < SIZE_T_CEILING); @@ -133,6 +133,8 @@ base32_decode(char *dest, size_t destlen, const char *src, size_t srclen) /** Return the Base64 encoded size of srclen bytes of data in * bytes. * + * Does NOT count the terminating NUL. + * * If flags&BASE64_ENCODE_MULTILINE is true, return the size * of the encoded output as multiline output (64 character, `\n' terminated * lines). @@ -141,19 +143,16 @@ size_t base64_encode_size(size_t srclen, int flags) { size_t enclen; + + /* Use INT_MAX for overflow checking because base64_encode() returns int. */ tor_assert(srclen < INT_MAX); + tor_assert(CEIL_DIV(srclen, 3) < INT_MAX / 4); - if (srclen == 0) - return 0; + enclen = BASE64_LEN(srclen); + if (flags & BASE64_ENCODE_MULTILINE) + enclen += CEIL_DIV(enclen, BASE64_OPENSSL_LINELEN); - enclen = ((srclen - 1) / 3) * 4 + 4; - if (flags & BASE64_ENCODE_MULTILINE) { - size_t remainder = enclen % BASE64_OPENSSL_LINELEN; - enclen += enclen / BASE64_OPENSSL_LINELEN; - if (remainder) - enclen++; - } - tor_assert(enclen < INT_MAX && enclen > srclen); + tor_assert(enclen < INT_MAX && (enclen == 0 || enclen > srclen)); return enclen; } @@ -475,7 +474,8 @@ base16_encode(char *dest, size_t destlen, const char *src, size_t srclen) const char *end; char *cp; - tor_assert(destlen >= srclen*2+1); + tor_assert(srclen < SIZE_T_CEILING / 2 - 1); + tor_assert(destlen >= BASE16_BUFSIZE(srclen)); tor_assert(destlen < SIZE_T_CEILING); /* Make sure we leave no uninitialized data in the destination buffer. */

1 0

[tor/master] Merge remote-tracking branch 'argonblue/baseXXlen'
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit 2c86380bdec15272652607025a6fb46eb54aeef8 Merge: 30b13fd 681812e Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 7 09:48:18 2017 -0400 Merge remote-tracking branch 'argonblue/baseXXlen' src/common/util.h | 6 +++--- src/common/util_format.c | 26 +++++++++++++------------- src/common/util_format.h | 20 ++++++++++++++++++++ src/or/shared_random.h | 9 +++------ 4 files changed, 39 insertions(+), 22 deletions(-)

1 0

[tor/master] Add macros for baseXX encoding lengths
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit e7f40baade042ced7b7c211cee4b8e1cbf3e230f Author: Taylor Yu <catalyst(a)torproject.org> Date: Thu Apr 6 13:54:27 2017 -0400 Add macros for baseXX encoding lengths --- src/common/util_format.h | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/common/util_format.h b/src/common/util_format.h index 5fd82d6..c928052 100644 --- a/src/common/util_format.h +++ b/src/common/util_format.h @@ -10,6 +10,26 @@ #include "testsupport.h" #include "torint.h" +/** @{ */ +/** These macros don't check for overflow. Use them only for constant inputs + * (like array declarations). The *_LEN macros are the raw encoding lengths + * (without terminating NUL), while the *_BUFSIZE macros count the terminating + * NUL. */ +#define BASE64_LEN(n) (CEIL_DIV((n), 3) * 4) +#define BASE32_LEN(n) (CEIL_DIV((n), 5) * 8) +#define BASE16_LEN(n) ((n) * 2) + +#define BASE64_BUFSIZE(n) (BASE64_LEN(n) + 1) +#define BASE32_BUFSIZE(n) (BASE32_LEN(n) + 1) +#define BASE16_BUFSIZE(n) (BASE16_LEN(n) + 1) + +#define BASE64_NOPAD_LEN(n) (CEIL_DIV((n) * 4, 3) +#define BASE32_NOPAD_LEN(n) (CEIL_DIV((n) * 8, 5) + +#define BASE64_NOPAD_BUFSIZE(n) (BASE64_NOPAD_LEN(n) + 1)) +#define BASE32_NOPAD_BUFSIZE(n) (BASE32_NOPAD_LEN(n) + 1)) +/** @} */ + #define BASE64_ENCODE_MULTILINE 1 size_t base64_encode_size(size_t srclen, int flags); int base64_encode(char *dest, size_t destlen, const char *src, size_t srclen,

1 0

[tor/master] Make the warnings about terminating nuls a bit stronger
by nickm＠torproject.org 07 Apr '17

07 Apr '17

commit 1e54bdd48a5deec7c1c4a0a167fb5abe312c8458 Author: Nick Mathewson <nickm(a)torproject.org> Date: Fri Apr 7 09:51:08 2017 -0400 Make the warnings about terminating nuls a bit stronger It looks like 32_encoded_size/64_encode_size APIs are inconsistent not only in the number of "d"s they have, but also in whether they count the terminating NUL. Taylor noted this in 86477f4e3fedb316, but I think we should note the inconsistently more loudly in order to avoid trouble. (I ran into trouble with this when writing 30b13fd82e243713c6a0d.) --- src/common/util_format.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/common/util_format.c b/src/common/util_format.c index 80844af..f6788e9 100644 --- a/src/common/util_format.c +++ b/src/common/util_format.c @@ -22,8 +22,10 @@ #include <stdlib.h> /* Return the base32 encoded size in bytes using the source length srclen. - * The NUL terminated byte is added as well since every base32 encoding - * requires enough space for it. */ + * + * (WATCH OUT: This API counts the terminating NUL byte, but + * base64_encode_size does not.) + */ size_t base32_encoded_size(size_t srclen) { @@ -133,7 +135,8 @@ base32_decode(char *dest, size_t destlen, const char *src, size_t srclen) /** Return the Base64 encoded size of srclen bytes of data in * bytes. * - * Does NOT count the terminating NUL. + * (WATCH OUT: This API does not count the terminating NUL byte, + * but base64_encoded_size does.) * * If flags&BASE64_ENCODE_MULTILINE is true, return the size * of the encoded output as multiline output (64 character, `\n' terminated

1 0