From 27315de590da24fe55353332c29a3a4454ee7da2 Mon Sep 17 00:00:00 2001 From: Daniel Pinto Date: Sat, 6 Jun 2020 10:44:28 +0100 Subject: [PATCH 1/3] Make curve25519_public_to_base64 output padding optional #7869 --- src/feature/hs/hs_descriptor.c | 7 +++--- src/lib/crypt_ops/crypto_curve25519.h | 4 ++- src/lib/crypt_ops/crypto_format.c | 35 +++++++++++++++++---------- src/lib/defs/x25519_sizes.h | 3 +++ src/test/test_crypto.c | 10 ++++---- 5 files changed, 37 insertions(+), 22 deletions(-) diff --git a/src/feature/hs/hs_descriptor.c b/src/feature/hs/hs_descriptor.c index 50a46fb40f..30a36030d1 100644 --- a/src/feature/hs/hs_descriptor.c +++ b/src/feature/hs/hs_descriptor.c @@ -55,6 +55,7 @@ /* For unit tests.*/ #define HS_DESCRIPTOR_PRIVATE +#include #include "core/or/or.h" #include "app/config/config.h" #include "trunnel/ed25519_cert.h" /* Trunnel interface. */ @@ -404,7 +405,7 @@ encode_enc_key(const hs_desc_intro_point_t *ip) tor_assert(ip); /* Base64 encode the encryption key for the "enc-key" field. */ - curve25519_public_to_base64(key_b64, &ip->enc_key); + curve25519_public_to_base64(key_b64, &ip->enc_key, true); if (tor_cert_encode_ed22519(ip->enc_key_cert, &encoded_cert) < 0) { goto done; } @@ -430,7 +431,7 @@ encode_onion_key(const hs_desc_intro_point_t *ip) tor_assert(ip); /* Base64 encode the encryption key for the "onion-key" field. */ - curve25519_public_to_base64(key_b64, &ip->onion_key); + curve25519_public_to_base64(key_b64, &ip->onion_key, true); tor_asprintf(&encoded, "%s ntor %s", str_ip_onion_key, key_b64); return encoded; @@ -813,7 +814,7 @@ get_outer_encrypted_layer_plaintext(const hs_descriptor_t *desc, tor_assert(!fast_mem_is_zero((char *) ephemeral_pubkey->public_key, CURVE25519_PUBKEY_LEN)); - curve25519_public_to_base64(ephemeral_key_base64, ephemeral_pubkey); + curve25519_public_to_base64(ephemeral_key_base64, ephemeral_pubkey, true); smartlist_add_asprintf(lines, "%s %s\n", str_desc_auth_key, ephemeral_key_base64); diff --git a/src/lib/crypt_ops/crypto_curve25519.h b/src/lib/crypt_ops/crypto_curve25519.h index 154a0b94bc..f1e5d1265d 100644 --- a/src/lib/crypt_ops/crypto_curve25519.h +++ b/src/lib/crypt_ops/crypto_curve25519.h @@ -9,6 +9,7 @@ #ifndef TOR_CRYPTO_CURVE25519_H #define TOR_CRYPTO_CURVE25519_H +#include #include "lib/testsupport/testsupport.h" #include "lib/cc/torint.h" #include "lib/crypt_ops/crypto_digest.h" @@ -77,7 +78,8 @@ STATIC int curve25519_basepoint_impl(uint8_t *output, const uint8_t *secret); int curve25519_public_from_base64(curve25519_public_key_t *pkey, const char *input); void curve25519_public_to_base64(char *output, - const curve25519_public_key_t *pkey); + const curve25519_public_key_t *pkey, + bool pad); void curve25519_set_impl_params(int use_ed); void curve25519_init(void); diff --git a/src/lib/crypt_ops/crypto_format.c b/src/lib/crypt_ops/crypto_format.c index 92b8b9372e..4483b7d2f5 100644 --- a/src/lib/crypt_ops/crypto_format.c +++ b/src/lib/crypt_ops/crypto_format.c @@ -131,9 +131,10 @@ crypto_read_tagged_contents_from_file(const char *fname, return r; } -/** Encode pkey as a base64-encoded string, including trailing "=" - * characters, in the buffer output, which must have at least - * CURVE25519_BASE64_PADDED_LEN+1 bytes available. +/** Encode pkey as a base64-encoded string in the buffer output. + * If pad is false do not include trailing "=" characters, otherwise + * include them. output must have at least + * CURVE25519_BASE64_PADDED_LEN+1 bytes available, even if pad is false. * Can not fail. * * Careful! CURVE25519_BASE64_PADDED_LEN is one byte longer than @@ -141,17 +142,25 @@ crypto_read_tagged_contents_from_file(const char *fname, */ void curve25519_public_to_base64(char *output, - const curve25519_public_key_t *pkey) + const curve25519_public_key_t *pkey, bool pad) { - char buf[128]; - int n = base64_encode(buf, sizeof(buf), - (const char*)pkey->public_key, - CURVE25519_PUBKEY_LEN, 0); + int n, expected_len; + if (pad) { + n = base64_encode(output, CURVE25519_BASE64_PADDED_LEN+1, + (const char*)pkey->public_key, + CURVE25519_PUBKEY_LEN, 0); + expected_len = CURVE25519_BASE64_PADDED_LEN; + } else { + n = base64_encode_nopad(output, CURVE25519_BASE64_PADDED_LEN+1, + (const uint8_t*)pkey->public_key, + CURVE25519_PUBKEY_LEN); + expected_len = CURVE25519_BASE64_LEN; + } + /* These asserts should always succeed, unless there is a bug in * base64_encode(). */ - tor_assert(n == CURVE25519_BASE64_PADDED_LEN); - tor_assert(buf[CURVE25519_BASE64_PADDED_LEN] == '\0'); - memcpy(output, buf, CURVE25519_BASE64_PADDED_LEN+1); + tor_assert(n == expected_len); + tor_assert(output[expected_len] == '\0'); } /** Try to decode a base64-encoded curve25519 public key from input @@ -162,11 +171,11 @@ curve25519_public_from_base64(curve25519_public_key_t *pkey, const char *input) { size_t len = strlen(input); - if (len == CURVE25519_BASE64_PADDED_LEN - 1) { + if (len == CURVE25519_BASE64_LEN) { /* not padded */ return digest256_from_base64((char*)pkey->public_key, input); } else if (len == CURVE25519_BASE64_PADDED_LEN) { - char buf[128]; + char buf[CURVE25519_BASE64_PADDED_LEN+1]; if (base64_decode(buf, sizeof(buf), input, len) != CURVE25519_PUBKEY_LEN) return -1; memcpy(pkey->public_key, buf, CURVE25519_PUBKEY_LEN); diff --git a/src/lib/defs/x25519_sizes.h b/src/lib/defs/x25519_sizes.h index acb08c5e6a..e650f5a350 100644 --- a/src/lib/defs/x25519_sizes.h +++ b/src/lib/defs/x25519_sizes.h @@ -36,6 +36,9 @@ /** Length of a Curve25519 key when encoded in base 64, with padding. */ #define CURVE25519_BASE64_PADDED_LEN 44 +/** Length of a Curve25519 key when encoded in base 64, without padding. */ +#define CURVE25519_BASE64_LEN 43 + /** Length of a Ed25519 key when encoded in base 64, without padding. */ #define ED25519_BASE64_LEN 43 /** Length of a Ed25519 signature when encoded in base 64, without padding. */ diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index 0d75a212e9..ffd6a25bd5 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -2107,21 +2107,21 @@ test_crypto_curve25519_encode(void *arg) { curve25519_secret_key_t seckey; curve25519_public_key_t key1, key2, key3; - char buf[64]; + char buf[64], buf_nopad[64]; (void)arg; curve25519_secret_key_generate(&seckey, 0); curve25519_public_key_generate(&key1, &seckey); - curve25519_public_to_base64(buf, &key1); + curve25519_public_to_base64(buf, &key1, true); tt_int_op(CURVE25519_BASE64_PADDED_LEN, OP_EQ, strlen(buf)); tt_int_op(0, OP_EQ, curve25519_public_from_base64(&key2, buf)); tt_mem_op(key1.public_key,OP_EQ, key2.public_key, CURVE25519_PUBKEY_LEN); - buf[CURVE25519_BASE64_PADDED_LEN - 1] = '\0'; - tt_int_op(CURVE25519_BASE64_PADDED_LEN-1, OP_EQ, strlen(buf)); - tt_int_op(0, OP_EQ, curve25519_public_from_base64(&key3, buf)); + curve25519_public_to_base64(buf_nopad, &key1, false); + tt_int_op(CURVE25519_BASE64_LEN, OP_EQ, strlen(buf_nopad)); + tt_int_op(0, OP_EQ, curve25519_public_from_base64(&key3, buf_nopad)); tt_mem_op(key1.public_key,OP_EQ, key3.public_key, CURVE25519_PUBKEY_LEN); /* Now try bogus parses. */ From d72618eb7f152c8f1633294fa30978c7ac0a48f3 Mon Sep 17 00:00:00 2001 From: Daniel Pinto Date: Sat, 6 Jun 2020 11:34:47 +0100 Subject: [PATCH 2/3] Remove padding from ntor-onion-key #7869 --- src/feature/dirauth/dirvote.c | 11 ++++++----- src/feature/dirauth/dirvote.h | 6 +++++- src/feature/relay/router.c | 8 +++----- src/test/test_dir.c | 8 ++------ 4 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/feature/dirauth/dirvote.c b/src/feature/dirauth/dirvote.c index 85a23a12f6..a1a530b7fa 100644 --- a/src/feature/dirauth/dirvote.c +++ b/src/feature/dirauth/dirvote.c @@ -3848,11 +3848,10 @@ dirvote_create_microdescriptor(const routerinfo_t *ri, int consensus_method) smartlist_add_asprintf(chunks, "onion-key\n%s", key); if (ri->onion_curve25519_pkey) { - char kbuf[128]; - base64_encode(kbuf, sizeof(kbuf), - (const char*)ri->onion_curve25519_pkey->public_key, - CURVE25519_PUBKEY_LEN, BASE64_ENCODE_MULTILINE); - smartlist_add_asprintf(chunks, "ntor-onion-key %s", kbuf); + char kbuf[CURVE25519_BASE64_PADDED_LEN + 1]; + bool add_padding = (consensus_method < MIN_METHOD_FOR_UNPADDED_NTOR_KEY); + curve25519_public_to_base64(kbuf, ri->onion_curve25519_pkey, add_padding); + smartlist_add_asprintf(chunks, "ntor-onion-key %s\n", kbuf); } if (family) { @@ -3963,6 +3962,8 @@ static const struct consensus_method_range_t { {MIN_SUPPORTED_CONSENSUS_METHOD, MIN_METHOD_FOR_CANONICAL_FAMILIES_IN_MICRODESCS - 1}, {MIN_METHOD_FOR_CANONICAL_FAMILIES_IN_MICRODESCS, + MIN_METHOD_FOR_UNPADDED_NTOR_KEY - 1}, + {MIN_METHOD_FOR_UNPADDED_NTOR_KEY, MAX_SUPPORTED_CONSENSUS_METHOD}, {-1, -1} }; diff --git a/src/feature/dirauth/dirvote.h b/src/feature/dirauth/dirvote.h index fa7b1da4ab..3ab40367ae 100644 --- a/src/feature/dirauth/dirvote.h +++ b/src/feature/dirauth/dirvote.h @@ -53,7 +53,7 @@ #define MIN_SUPPORTED_CONSENSUS_METHOD 28 /** The highest consensus method that we currently support. */ -#define MAX_SUPPORTED_CONSENSUS_METHOD 29 +#define MAX_SUPPORTED_CONSENSUS_METHOD 30 /** * Lowest consensus method where microdescriptor lines are put in canonical @@ -61,6 +61,10 @@ **/ #define MIN_METHOD_FOR_CANONICAL_FAMILIES_IN_MICRODESCS 29 +/** Lowest consensus method where an unpadded base64 onion-key-ntor is allowed + * See #7869 */ +#define MIN_METHOD_FOR_UNPADDED_NTOR_KEY 30 + /** Default bandwidth to clip unmeasured bandwidths to using method >= * MIN_METHOD_TO_CLIP_UNMEASURED_BW. (This is not a consensus method; do not * get confused with the above macros.) */ diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index 34d8163c36..ffaf7c3cc5 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -2854,11 +2854,9 @@ router_dump_router_to_string(routerinfo_t *router, } if (router->onion_curve25519_pkey) { - char kbuf[128]; - base64_encode(kbuf, sizeof(kbuf), - (const char *)router->onion_curve25519_pkey->public_key, - CURVE25519_PUBKEY_LEN, BASE64_ENCODE_MULTILINE); - smartlist_add_asprintf(chunks, "ntor-onion-key %s", kbuf); + char kbuf[CURVE25519_BASE64_PADDED_LEN + 1]; + curve25519_public_to_base64(kbuf, router->onion_curve25519_pkey, false); + smartlist_add_asprintf(chunks, "ntor-onion-key %s\n", kbuf); } else { /* Authorities will start rejecting relays without ntor keys in 0.2.9 */ log_err(LD_BUG, "A relay must have an ntor onion key"); diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 3a0b8237cb..f6a21c804e 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -397,18 +397,14 @@ get_new_ntor_onion_key_line(const curve25519_public_key_t *ntor_onion_pubkey) { char *line = NULL; char cert_buf[256]; - int rv = 0; tor_assert(ntor_onion_pubkey); - rv = base64_encode(cert_buf, sizeof(cert_buf), - (const char*)ntor_onion_pubkey->public_key, 32, - BASE64_ENCODE_MULTILINE); - tor_assert(rv > 0); + curve25519_public_to_base64(cert_buf, ntor_onion_pubkey, false); tor_assert(strlen(cert_buf) > 0); tor_asprintf(&line, - "ntor-onion-key %s", + "ntor-onion-key %s\n", cert_buf); tor_assert(line); From 8763d96d20f33ca7367effb77fb5d7ab30c39766 Mon Sep 17 00:00:00 2001 From: Daniel Pinto Date: Sun, 7 Jun 2020 17:26:02 +0100 Subject: [PATCH 3/3] Changes file for #7869 --- changes/ticket7869 | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 changes/ticket7869 diff --git a/changes/ticket7869 b/changes/ticket7869 new file mode 100644 index 0000000000..001b165ff5 --- /dev/null +++ b/changes/ticket7869 @@ -0,0 +1,3 @@ + o Minor feature (directory authorities): + - Create new consensus method that removes the unecessary = padding + from ntor-onion-key. Closes ticket 7869. Patch by Daniel Pinto.