From 7d513a5d5541d17c4e9622a9af76303042fd380b Mon Sep 17 00:00:00 2001 From: teor Date: Fri, 5 Apr 2019 15:02:43 +1000 Subject: [PATCH] crypto_format: Remove the return values from digest256_to_base64() ... and ed25519_public_to_base64(). Also remove all checks for the return values, which were redundant anyway, because the functions never failed. Part of 29960. --- src/feature/dirauth/dirvote.c | 3 +-- src/feature/hs/hs_client.c | 16 +++------------- src/feature/hs/hs_control.c | 23 +++++------------------ src/feature/relay/router.c | 7 ++----- src/lib/crypt_ops/crypto_format.c | 27 +++++++++++++++++---------- src/lib/crypt_ops/crypto_format.h | 6 +++--- src/test/test_crypto.c | 2 +- src/test/test_dir.c | 4 +--- src/test/test_hs_cache.c | 4 +--- src/test/test_hs_control.c | 3 +-- 10 files changed, 35 insertions(+), 60 deletions(-) diff --git a/src/feature/dirauth/dirvote.c b/src/feature/dirauth/dirvote.c index 29f5d04509..1f861d2417 100644 --- a/src/feature/dirauth/dirvote.c +++ b/src/feature/dirauth/dirvote.c @@ -3914,8 +3914,7 @@ dirvote_format_microdesc_vote_line(char *out_buf, size_t out_buf_len, ","); tor_assert(microdesc_consensus_methods); - if (digest256_to_base64(d64, md->digest)<0) - goto out; + digest256_to_base64(d64, md->digest); if (tor_snprintf(out_buf, out_buf_len, "m %s sha256=%s\n", microdesc_consensus_methods, d64)<0) diff --git a/src/feature/hs/hs_client.c b/src/feature/hs/hs_client.c index c34271efca..38b9646c17 100644 --- a/src/feature/hs/hs_client.c +++ b/src/feature/hs/hs_client.c @@ -165,9 +165,7 @@ purge_hid_serv_request(const ed25519_public_key_t *identity_pk) * some point and we don't care about those anymore. */ hs_build_blinded_pubkey(identity_pk, NULL, 0, hs_get_time_period_num(0), &blinded_pk); - if (BUG(ed25519_public_to_base64(base64_blinded_pk, &blinded_pk) < 0)) { - return; - } + ed25519_public_to_base64(base64_blinded_pk, &blinded_pk); /* Purge last hidden service request from cache for this blinded key. */ hs_purge_hid_serv_from_last_hid_serv_requests(base64_blinded_pk); } @@ -354,7 +352,6 @@ directory_launch_v3_desc_fetch(const ed25519_public_key_t *onion_identity_pk, ed25519_public_key_t blinded_pubkey; char base64_blinded_pubkey[ED25519_BASE64_LEN + 1]; hs_ident_dir_conn_t hs_conn_dir_ident; - int retval; tor_assert(hsdir); tor_assert(onion_identity_pk); @@ -363,10 +360,7 @@ directory_launch_v3_desc_fetch(const ed25519_public_key_t *onion_identity_pk, hs_build_blinded_pubkey(onion_identity_pk, NULL, 0, current_time_period, &blinded_pubkey); /* ...and base64 it. */ - retval = ed25519_public_to_base64(base64_blinded_pubkey, &blinded_pubkey); - if (BUG(retval < 0)) { - return HS_CLIENT_FETCH_ERROR; - } + ed25519_public_to_base64(base64_blinded_pubkey, &blinded_pubkey); /* Copy onion pk to a dir_ident so that we attach it to the dir conn */ hs_ident_dir_conn_init(onion_identity_pk, &blinded_pubkey, @@ -405,7 +399,6 @@ directory_launch_v3_desc_fetch(const ed25519_public_key_t *onion_identity_pk, STATIC routerstatus_t * pick_hsdir_v3(const ed25519_public_key_t *onion_identity_pk) { - int retval; char base64_blinded_pubkey[ED25519_BASE64_LEN + 1]; uint64_t current_time_period = hs_get_time_period_num(0); smartlist_t *responsible_hsdirs = NULL; @@ -418,10 +411,7 @@ pick_hsdir_v3(const ed25519_public_key_t *onion_identity_pk) hs_build_blinded_pubkey(onion_identity_pk, NULL, 0, current_time_period, &blinded_pubkey); /* ...and base64 it. */ - retval = ed25519_public_to_base64(base64_blinded_pubkey, &blinded_pubkey); - if (BUG(retval < 0)) { - return NULL; - } + ed25519_public_to_base64(base64_blinded_pubkey, &blinded_pubkey); /* Get responsible hsdirs of service for this time period */ responsible_hsdirs = smartlist_new(); diff --git a/src/feature/hs/hs_control.c b/src/feature/hs/hs_control.c index 20a1061609..abb421345c 100644 --- a/src/feature/hs/hs_control.c +++ b/src/feature/hs/hs_control.c @@ -74,10 +74,7 @@ hs_control_desc_event_failed(const hs_ident_dir_conn_t *ident, tor_assert(reason); /* Build onion address and encoded blinded key. */ - IF_BUG_ONCE(ed25519_public_to_base64(base64_blinded_pk, - &ident->blinded_pk) < 0) { - return; - } + ed25519_public_to_base64(base64_blinded_pk, &ident->blinded_pk); hs_build_address(&ident->identity_pk, HS_VERSION_THREE, onion_address); control_event_hsv3_descriptor_failed(onion_address, base64_blinded_pk, @@ -99,10 +96,7 @@ hs_control_desc_event_received(const hs_ident_dir_conn_t *ident, tor_assert(hsdir_id_digest); /* Build onion address and encoded blinded key. */ - IF_BUG_ONCE(ed25519_public_to_base64(base64_blinded_pk, - &ident->blinded_pk) < 0) { - return; - } + ed25519_public_to_base64(base64_blinded_pk, &ident->blinded_pk); hs_build_address(&ident->identity_pk, HS_VERSION_THREE, onion_address); control_event_hsv3_descriptor_received(onion_address, base64_blinded_pk, @@ -123,9 +117,7 @@ hs_control_desc_event_created(const char *onion_address, tor_assert(blinded_pk); /* Build base64 encoded blinded key. */ - IF_BUG_ONCE(ed25519_public_to_base64(base64_blinded_pk, blinded_pk) < 0) { - return; - } + ed25519_public_to_base64(base64_blinded_pk, blinded_pk); /* Version 3 doesn't use the replica number in its descriptor ID computation * so we pass negative value so the control port subsystem can ignore it. */ @@ -151,9 +143,7 @@ hs_control_desc_event_upload(const char *onion_address, tor_assert(hsdir_index); /* Build base64 encoded blinded key. */ - IF_BUG_ONCE(ed25519_public_to_base64(base64_blinded_pk, blinded_pk) < 0) { - return; - } + ed25519_public_to_base64(base64_blinded_pk, blinded_pk); control_event_hs_descriptor_upload(onion_address, hsdir_id_digest, base64_blinded_pk, @@ -196,10 +186,7 @@ hs_control_desc_event_content(const hs_ident_dir_conn_t *ident, tor_assert(hsdir_id_digest); /* Build onion address and encoded blinded key. */ - IF_BUG_ONCE(ed25519_public_to_base64(base64_blinded_pk, - &ident->blinded_pk) < 0) { - return; - } + ed25519_public_to_base64(base64_blinded_pk, &ident->blinded_pk); hs_build_address(&ident->identity_pk, HS_VERSION_THREE, onion_address); control_event_hs_descriptor_content(onion_address, base64_blinded_pk, diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c index e5cf72ad18..837465cfe9 100644 --- a/src/feature/relay/router.c +++ b/src/feature/relay/router.c @@ -2728,11 +2728,8 @@ router_dump_router_to_string(routerinfo_t *router, log_err(LD_BUG,"Couldn't base64-encode signing key certificate!"); goto err; } - if (ed25519_public_to_base64(ed_fp_base64, - &router->cache_info.signing_key_cert->signing_key)<0) { - log_err(LD_BUG,"Couldn't base64-encode identity key\n"); - goto err; - } + ed25519_public_to_base64(ed_fp_base64, + &router->cache_info.signing_key_cert->signing_key); tor_asprintf(&ed_cert_line, "identity-ed25519\n" "-----BEGIN ED25519 CERT-----\n" "%s" diff --git a/src/lib/crypt_ops/crypto_format.c b/src/lib/crypt_ops/crypto_format.c index 1467b3d0a6..217ac8959c 100644 --- a/src/lib/crypt_ops/crypto_format.c +++ b/src/lib/crypt_ops/crypto_format.c @@ -181,8 +181,7 @@ ed25519_fmt(const ed25519_public_key_t *pkey) if (ed25519_public_key_is_zero(pkey)) { strlcpy(formatted, "", sizeof(formatted)); } else { - int r = ed25519_public_to_base64(formatted, pkey); - tor_assert(!r); + ed25519_public_to_base64(formatted, pkey); } } else { strlcpy(formatted, "", sizeof(formatted)); @@ -202,13 +201,17 @@ ed25519_public_from_base64(ed25519_public_key_t *pkey, /** Encode the public key pkey into the buffer at output, * which must have space for ED25519_BASE64_LEN bytes of encoded key, - * plus one byte for a terminating NUL. Return 0 on success, -1 on failure. + * plus one byte for a terminating NUL. + * Can not fail. + * + * Careful! ED25519_BASE64_LEN is one byte shorter than + * CURVE25519_BASE64_PADDED_LEN. */ -int +void ed25519_public_to_base64(char *output, const ed25519_public_key_t *pkey) { - return digest256_to_base64(output, (const char *)pkey->pubkey); + digest256_to_base64(output, (const char *)pkey->pubkey); } /** Encode the signature sig into the buffer at output, @@ -281,15 +284,19 @@ digest_from_base64(char *digest, const char *d64) /** Base64 encode DIGEST256_LINE bytes from digest, remove the * trailing = characters, and store the nul-terminated result in the first - * BASE64_DIGEST256_LEN+1 bytes of d64. */ -int + * BASE64_DIGEST256_LEN+1 bytes of d64. + * Can not fail. */ +void digest256_to_base64(char *d64, const char *digest) { char buf[256]; - base64_encode(buf, sizeof(buf), digest, DIGEST256_LEN, 0); - buf[BASE64_DIGEST256_LEN] = '\0'; + int n = base64_encode_nopad(buf, sizeof(buf), + (const uint8_t *)digest, DIGEST256_LEN); + /* These asserts should always succeed, unless there is a bug in + * base64_encode_nopad(). */ + tor_assert(n == BASE64_DIGEST256_LEN); + tor_assert(buf[BASE64_DIGEST256_LEN] == '\0'); memcpy(d64, buf, BASE64_DIGEST256_LEN+1); - return 0; } /** Given a base64 encoded, nul-terminated digest in d64 (without diff --git a/src/lib/crypt_ops/crypto_format.h b/src/lib/crypt_ops/crypto_format.h index 53238cb5ab..41c2b06ec8 100644 --- a/src/lib/crypt_ops/crypto_format.h +++ b/src/lib/crypt_ops/crypto_format.h @@ -33,8 +33,8 @@ ssize_t crypto_read_tagged_contents_from_file(const char *fname, int ed25519_public_from_base64(struct ed25519_public_key_t *pkey, const char *input); -int ed25519_public_to_base64(char *output, - const struct ed25519_public_key_t *pkey); +void ed25519_public_to_base64(char *output, + const struct ed25519_public_key_t *pkey); const char *ed25519_fmt(const struct ed25519_public_key_t *pkey); int ed25519_signature_from_base64(struct ed25519_signature_t *sig, @@ -44,7 +44,7 @@ int ed25519_signature_to_base64(char *output, void digest_to_base64(char *d64, const char *digest); int digest_from_base64(char *digest, const char *d64); -int digest256_to_base64(char *d64, const char *digest); +void digest256_to_base64(char *d64, const char *digest); int digest256_from_base64(char *digest, const char *d64); #endif /* !defined(TOR_CRYPTO_FORMAT_H) */ diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index a5c17b3e6a..fb8046fd22 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -2455,7 +2455,7 @@ test_crypto_ed25519_encode(void *arg) /* Test roundtrip. */ tt_int_op(0, OP_EQ, ed25519_keypair_generate(&kp, 0)); - tt_int_op(0, OP_EQ, ed25519_public_to_base64(buf, &kp.pubkey)); + ed25519_public_to_base64(buf, &kp.pubkey); tt_int_op(ED25519_BASE64_LEN, OP_EQ, strlen(buf)); tt_int_op(0, OP_EQ, ed25519_public_from_base64(&pk, buf)); tt_mem_op(kp.pubkey.pubkey, OP_EQ, pk.pubkey, ED25519_PUBKEY_LEN); diff --git a/src/test/test_dir.c b/src/test/test_dir.c index 6518977b6f..17d6db1e4d 100644 --- a/src/test/test_dir.c +++ b/src/test/test_dir.c @@ -995,9 +995,7 @@ test_dir_formats_rsa_ed25519(void *arg) smartlist_add_strdup(chunks, "master-key-ed25519 "); { char k[ED25519_BASE64_LEN+1]; - tt_int_op(ed25519_public_to_base64(k, - &r2->cache_info.signing_key_cert->signing_key), - OP_GE, 0); + ed25519_public_to_base64(k, &r2->cache_info.signing_key_cert->signing_key); smartlist_add_strdup(chunks, k); smartlist_add_strdup(chunks, "\n"); } diff --git a/src/test/test_hs_cache.c b/src/test/test_hs_cache.c index 9182829116..48e8d3b8c4 100644 --- a/src/test/test_hs_cache.c +++ b/src/test/test_hs_cache.c @@ -238,9 +238,7 @@ helper_fetch_desc_from_hsdir(const ed25519_public_key_t *blinded_key) { char hsdir_cache_key[ED25519_BASE64_LEN+1]; - retval = ed25519_public_to_base64(hsdir_cache_key, - blinded_key); - tt_int_op(retval, OP_EQ, 0); + ed25519_public_to_base64(hsdir_cache_key, blinded_key); tor_asprintf(&hsdir_query_str, GET("/tor/hs/3/%s"), hsdir_cache_key); } diff --git a/src/test/test_hs_control.c b/src/test/test_hs_control.c index 481ef1eb39..7cedc987bb 100644 --- a/src/test/test_hs_control.c +++ b/src/test/test_hs_control.c @@ -107,8 +107,7 @@ test_hs_desc_event(void *arg) memset(&blinded_pk, 'B', sizeof(blinded_pk)); memset(&hsdir_rs, 0, sizeof(hsdir_rs)); memcpy(hsdir_rs.identity_digest, HSDIR_EXIST_ID, DIGEST_LEN); - ret = ed25519_public_to_base64(base64_blinded_pk, &blinded_pk); - tt_int_op(ret, OP_EQ, 0); + ed25519_public_to_base64(base64_blinded_pk, &blinded_pk); memcpy(&ident.identity_pk, &identity_kp.pubkey, sizeof(ed25519_public_key_t)); memcpy(&ident.blinded_pk, &blinded_pk, sizeof(blinded_pk));