commit 196499da73afa5c8000154aafe36c10b9ee43901 Author: Yawning Angel yawning@schwanenlied.me Date: Fri Apr 10 11:25:08 2015 +0000

Use a custom Base64 encoder with more control over the output format. --- changes/feature15652 | 4 ++ src/common/crypto.c | 171 ++++++++++++++++++++++++++++++++++++++------ src/common/crypto.h | 5 +- src/common/crypto_format.c | 2 +- src/or/connection.c | 19 +---- src/or/dirvote.c | 5 +- src/or/rendclient.c | 7 +- src/or/rendcommon.c | 3 +- src/or/rendservice.c | 10 ++- src/or/router.c | 2 +- src/or/routerparse.c | 3 +- src/test/test_crypto.c | 36 +++++++++- src/tools/tor-gencert.c | 6 +- 13 files changed, 214 insertions(+), 59 deletions(-)

diff --git a/changes/feature15652 b/changes/feature15652 index d11b76f..61d2b22 100644 --- a/changes/feature15652 +++ b/changes/feature15652 @@ -1,3 +1,7 @@ + o Code simplifications and refactoring: + - Use our own Base64 encoder instead of OpenSSL's, to allow more control + over the output. Part of ticket 15652. + o Removed code: - Remove `USE_OPENSSL_BASE64` and the corresponding fallback code and always use the internal Base64 decoder. The internal decoder has been diff --git a/src/common/crypto.c b/src/common/crypto.c index c7e015b..2cb1c39 100644 --- a/src/common/crypto.c +++ b/src/common/crypto.c @@ -2426,36 +2426,161 @@ smartlist_shuffle(smartlist_t *sl) } }

+#define BASE64_OPENSSL_LINELEN 64 + +/** Return the Base64 encoded size of <b>srclen</b> bytes of data in + * bytes. + * + * If <b>flags</b>&amp;BASE64_ENCODE_MULTILINE is true, return the size + * of the encoded output as multiline output (64 character, `\n' terminated + * lines). + */ +size_t +base64_encode_size(size_t srclen, int flags) +{ + size_t enclen; + tor_assert(srclen < INT_MAX); + + if (srclen == 0) + return 0; + + 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); + return enclen; +} + +/** Internal table mapping 6 bit values to the Base64 alphabet. */ +static const char base64_encode_table[64] = { + 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', + 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', + 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X', + 'Y', 'Z', 'a', 'b', 'c', 'd', 'e', 'f', + 'g', 'h', 'i', 'j', 'k', 'l', 'm', 'n', + 'o', 'p', 'q', 'r', 's', 't', 'u', 'v', + 'w', 'x', 'y', 'z', '0', '1', '2', '3', + '4', '5', '6', '7', '8', '9', '+', '/' +}; + /** Base64 encode <b>srclen</b> bytes of data from <b>src</b>. Write * the result into <b>dest</b>, if it will fit within <b>destlen</b> - * bytes. Return the number of bytes written on success; -1 if + * bytes. Return the number of bytes written on success; -1 if * destlen is too short, or other failure. + * + * If <b>flags</b>&amp;BASE64_ENCODE_MULTILINE is true, return encoded + * output in multiline format (64 character, `\n' terminated lines). */ int -base64_encode(char *dest, size_t destlen, const char *src, size_t srclen) -{ - /* FFFF we might want to rewrite this along the lines of base64_decode, if - * it ever shows up in the profile. */ - EVP_ENCODE_CTX ctx; - int len, ret; - tor_assert(srclen < INT_MAX); +base64_encode(char *dest, size_t destlen, const char *src, size_t srclen, + int flags) +{ + const unsigned char *usrc = (unsigned char *)src; + const unsigned char *eous = usrc + srclen; + char *d = dest; + uint32_t n = 0; + size_t linelen = 0; + size_t enclen; + int n_idx = 0; + + if (!src || !dest) + return -1;

- /* 48 bytes of input -> 64 bytes of output plus newline. - Plus one more byte, in case I'm wrong. - */ - if (destlen < ((srclen/48)+1)*66) + /* Ensure that there is sufficient space, including the NUL. */ + enclen = base64_encode_size(srclen, flags); + if (destlen < enclen + 1) return -1; if (destlen > SIZE_T_CEILING) return -1;

- EVP_EncodeInit(&ctx); - EVP_EncodeUpdate(&ctx, (unsigned char*)dest, &len, - (unsigned char*)src, (int)srclen); - EVP_EncodeFinal(&ctx, (unsigned char*)(dest+len), &ret); - ret += len; - return ret; + memset(dest, 0, enclen); + + /* XXX/Yawning: If this ends up being too slow, this can be sped up + * by separating the multiline format case and the normal case, and + * processing 48 bytes of input at a time when newlines are desired. + */ +#define ENCODE_CHAR(ch) \ + STMT_BEGIN \ + *d++ = ch; \ + if (flags & BASE64_ENCODE_MULTILINE) { \ + if (++linelen % BASE64_OPENSSL_LINELEN == 0) { \ + linelen = 0; \ + *d++ = '\n'; \ + } \ + } \ + STMT_END + +#define ENCODE_N(idx) \ + ENCODE_CHAR(base64_encode_table[(n >> ((3 - idx) * 6)) & 0x3f]) + +#define ENCODE_PAD() ENCODE_CHAR('=') + + /* Iterate over all the bytes in src. Each one will add 8 bits to the + * value we're encoding. Accumulate bits in <b>n</b>, and whenever we + * have 24 bits, batch them into 4 bytes and flush those bytes to dest. + */ + for ( ; usrc < eous; ++usrc) { + n = (n << 8) | *usrc; + if ((++n_idx) == 3) { + ENCODE_N(0); + ENCODE_N(1); + ENCODE_N(2); + ENCODE_N(3); + n_idx = 0; + n = 0; + } + } + switch (n_idx) { + case 0: + /* 0 leftover bits, no pading to add. */ + break; + case 1: + /* 8 leftover bits, pad to 12 bits, write the 2 6-bit values followed + * by 2 padding characters. + */ + n <<= 4; + ENCODE_N(2); + ENCODE_N(3); + ENCODE_PAD(); + ENCODE_PAD(); + break; + case 2: + /* 16 leftover bits, pad to 18 bits, write the 3 6-bit values followed + * by 1 padding character. + */ + n <<= 2; + ENCODE_N(1); + ENCODE_N(2); + ENCODE_N(3); + ENCODE_PAD(); + break; + default: + /* Something went catastrophically wrong. */ + tor_fragile_assert(); + return -1; + } + +#undef ENCODE_N +#undef ENCODE_PAD +#undef ENCODE_CHAR + + /* Multiline output always includes at least one newline. */ + if (flags & BASE64_ENCODE_MULTILINE && linelen != 0) + *d++ = '\n'; + + tor_assert(d - dest == (ptrdiff_t)enclen); + + *d++ = '\0'; /* NUL terminate the output. */ + + return enclen; }

+#undef BASE64_OPENSSL_LINELEN + /** @{ */ /** Special values used for the base64_decode_table */ #define X 255 @@ -2576,13 +2701,13 @@ base64_decode(char *dest, size_t destlen, const char *src, size_t srclen) #undef PAD

/** Base64 encode DIGEST_LINE bytes from <b>digest</b>, remove the trailing = - * and newline characters, and store the nul-terminated result in the first + * characters, and store the nul-terminated result in the first * BASE64_DIGEST_LEN+1 bytes of <b>d64</b>. */ int digest_to_base64(char *d64, const char *digest) { char buf[256]; - base64_encode(buf, sizeof(buf), digest, DIGEST_LEN); + base64_encode(buf, sizeof(buf), digest, DIGEST_LEN, 0); buf[BASE64_DIGEST_LEN] = '\0'; memcpy(d64, buf, BASE64_DIGEST_LEN+1); return 0; @@ -2601,13 +2726,13 @@ digest_from_base64(char *digest, const char *d64) }

/** Base64 encode DIGEST256_LINE bytes from <b>digest</b>, remove the - * trailing = and newline characters, and store the nul-terminated result in - * the first BASE64_DIGEST256_LEN+1 bytes of <b>d64</b>. */ + * trailing = characters, and store the nul-terminated result in the first + * BASE64_DIGEST256_LEN+1 bytes of <b>d64</b>. */ int digest256_to_base64(char *d64, const char *digest) { char buf[256]; - base64_encode(buf, sizeof(buf), digest, DIGEST256_LEN); + base64_encode(buf, sizeof(buf), digest, DIGEST256_LEN, 0); buf[BASE64_DIGEST256_LEN] = '\0'; memcpy(d64, buf, BASE64_DIGEST256_LEN+1); return 0; diff --git a/src/common/crypto.h b/src/common/crypto.h index 440ff23..2061ec3 100644 --- a/src/common/crypto.h +++ b/src/common/crypto.h @@ -267,7 +267,10 @@ struct smartlist_t; void *smartlist_choose(const struct smartlist_t *sl); void smartlist_shuffle(struct smartlist_t *sl);

-int base64_encode(char *dest, size_t destlen, const char *src, size_t srclen); +#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, + int flags); int base64_decode(char *dest, size_t destlen, const char *src, size_t srclen); /** Characters that can appear (case-insensitively) in a base32 encoding. */ #define BASE32_CHARS "abcdefghijklmnopqrstuvwxyz234567" diff --git a/src/common/crypto_format.c b/src/common/crypto_format.c index 00e0e9e..35a7854 100644 --- a/src/common/crypto_format.c +++ b/src/common/crypto_format.c @@ -19,7 +19,7 @@ curve25519_public_to_base64(char *output, { char buf[128]; base64_encode(buf, sizeof(buf), - (const char*)pkey->public_key, CURVE25519_PUBKEY_LEN); + (const char*)pkey->public_key, CURVE25519_PUBKEY_LEN, 0); buf[CURVE25519_BASE64_PADDED_LEN] = '\0'; memcpy(output, buf, CURVE25519_BASE64_PADDED_LEN+1); return 0; diff --git a/src/or/connection.c b/src/or/connection.c index 369df67..5610815 100644 --- a/src/or/connection.c +++ b/src/or/connection.c @@ -4440,25 +4440,12 @@ alloc_http_authenticator(const char *authenticator) /* an authenticator in Basic authentication * is just the string "username:password" */ const size_t authenticator_length = strlen(authenticator); - /* The base64_encode function needs a minimum buffer length - * of 66 bytes. */ - const size_t base64_authenticator_length = (authenticator_length/48+1)*66; + const size_t base64_authenticator_length = + base64_encode_size(authenticator_length, 0) + 1; char *base64_authenticator = tor_malloc(base64_authenticator_length); if (base64_encode(base64_authenticator, base64_authenticator_length, - authenticator, authenticator_length) < 0) { + authenticator, authenticator_length, 0) < 0) { tor_free(base64_authenticator); /* free and set to null */ - } else { - int i = 0, j = 0; - ssize_t len = strlen(base64_authenticator); - - /* remove all newline occurrences within the string */ - for (i=0; i < len; ++i) { - if ('\n' != base64_authenticator[i]) { - base64_authenticator[j] = base64_authenticator[i]; - ++j; - } - } - base64_authenticator[j]='\0'; } return base64_authenticator; } diff --git a/src/or/dirvote.c b/src/or/dirvote.c index 7a5154d..3009026 100644 --- a/src/or/dirvote.c +++ b/src/or/dirvote.c @@ -2244,7 +2244,8 @@ networkstatus_format_signatures(networkstatus_t *consensus, for_detached_signatures ? flavor_name : "", digest_name, id, sk); } - base64_encode(buf, sizeof(buf), sig->signature, sig->signature_len); + base64_encode(buf, sizeof(buf), sig->signature, sig->signature_len, + BASE64_ENCODE_MULTILINE); strlcat(buf, "-----END SIGNATURE-----\n", sizeof(buf)); smartlist_add(elements, tor_strdup(buf)); } SMARTLIST_FOREACH_END(sig); @@ -3459,7 +3460,7 @@ dirvote_create_microdescriptor(const routerinfo_t *ri, int consensus_method) char kbuf[128]; base64_encode(kbuf, sizeof(kbuf), (const char*)ri->onion_curve25519_pkey->public_key, - CURVE25519_PUBKEY_LEN); + CURVE25519_PUBKEY_LEN, BASE64_ENCODE_MULTILINE); smartlist_add_asprintf(chunks, "ntor-onion-key %s", kbuf); }

diff --git a/src/or/rendclient.c b/src/or/rendclient.c index 162e0ac..de66b6d 100644 --- a/src/or/rendclient.c +++ b/src/or/rendclient.c @@ -685,12 +685,13 @@ directory_get_from_hs_dir(const char *desc_id, const rend_data_t *rend_query) if (rend_query->auth_type != REND_NO_AUTH) { if (base64_encode(descriptor_cookie_base64, sizeof(descriptor_cookie_base64), - rend_query->descriptor_cookie, REND_DESC_COOKIE_LEN)<0) { + rend_query->descriptor_cookie, REND_DESC_COOKIE_LEN, + 0)<0) { log_warn(LD_BUG, "Could not base64-encode descriptor cookie."); return 0; } - /* Remove == signs and newline. */ - descriptor_cookie_base64[strlen(descriptor_cookie_base64)-3] = '\0'; + /* Remove == signs. */ + descriptor_cookie_base64[strlen(descriptor_cookie_base64)-2] = '\0'; } else { strlcpy(descriptor_cookie_base64, "(none)", sizeof(descriptor_cookie_base64)); diff --git a/src/or/rendcommon.c b/src/or/rendcommon.c index 5711f9e..435602f 100644 --- a/src/or/rendcommon.c +++ b/src/or/rendcommon.c @@ -529,7 +529,8 @@ rend_encode_v2_descriptors(smartlist_t *descs_out, } /* Base64-encode introduction points. */ ipos_base64 = tor_calloc(ipos_len, 2); - if (base64_encode(ipos_base64, ipos_len * 2, ipos, ipos_len)<0) { + if (base64_encode(ipos_base64, ipos_len * 2, ipos, ipos_len, + BASE64_ENCODE_MULTILINE)<0) { log_warn(LD_REND, "Could not encode introduction point string to " "base64. length=%d", (int)ipos_len); tor_free(ipos_base64); diff --git a/src/or/rendservice.c b/src/or/rendservice.c index c1c0c46..0a41ed2 100644 --- a/src/or/rendservice.c +++ b/src/or/rendservice.c @@ -941,7 +941,7 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname) } if (base64_encode(desc_cook_out, 3*REND_DESC_COOKIE_LEN_BASE64+1, client->descriptor_cookie, - REND_DESC_COOKIE_LEN) < 0) { + REND_DESC_COOKIE_LEN, 0) < 0) { log_warn(LD_BUG, "Could not base64-encode descriptor cookie."); goto err; } @@ -968,7 +968,6 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname) client->client_key = prkey; } /* Add entry to client_keys file. */ - desc_cook_out[strlen(desc_cook_out)-1] = '\0'; /* Remove newline. */ written = tor_snprintf(buf, sizeof(buf), "client-name %s\ndescriptor-cookie %s\n", client->client_name, desc_cook_out); @@ -1023,12 +1022,11 @@ rend_service_load_auth_keys(rend_service_t *s, const char *hfname) ((int)s->auth_type - 1) << 4; if (base64_encode(desc_cook_out, 3*REND_DESC_COOKIE_LEN_BASE64+1, extended_desc_cookie, - REND_DESC_COOKIE_LEN+1) < 0) { + REND_DESC_COOKIE_LEN+1, 0) < 0) { log_warn(LD_BUG, "Could not base64-encode descriptor cookie."); goto err; } - desc_cook_out[strlen(desc_cook_out)-3] = '\0'; /* Remove A= and - newline. */ + desc_cook_out[strlen(desc_cook_out)-2] = '\0'; /* Remove A=. */ tor_snprintf(buf, sizeof(buf),"%s.onion %s # client: %s\n", service_id, desc_cook_out, client->client_name); } @@ -1124,7 +1122,7 @@ rend_check_authorization(rend_service_t *service, if (!auth_client) { char descriptor_cookie_base64[3*REND_DESC_COOKIE_LEN_BASE64]; base64_encode(descriptor_cookie_base64, sizeof(descriptor_cookie_base64), - descriptor_cookie, REND_DESC_COOKIE_LEN); + descriptor_cookie, REND_DESC_COOKIE_LEN, 0); log_info(LD_REND, "No authorization found for descriptor cookie '%s'! " "Dropping cell!", descriptor_cookie_base64); diff --git a/src/or/router.c b/src/or/router.c index 2ddaa89..b803ba5 100644 --- a/src/or/router.c +++ b/src/or/router.c @@ -2424,7 +2424,7 @@ router_dump_router_to_string(routerinfo_t *router, char kbuf[128]; base64_encode(kbuf, sizeof(kbuf), (const char *)router->onion_curve25519_pkey->public_key, - CURVE25519_PUBKEY_LEN); + CURVE25519_PUBKEY_LEN, BASE64_ENCODE_MULTILINE); smartlist_add_asprintf(chunks, "ntor-onion-key %s", kbuf); }

diff --git a/src/or/routerparse.c b/src/or/routerparse.c index fd3971c..b9a36a3 100644 --- a/src/or/routerparse.c +++ b/src/or/routerparse.c @@ -664,7 +664,8 @@ router_get_dirobj_signature(const char *digest, goto truncated;

i = strlen(buf); - if (base64_encode(buf+i, buf_len-i, signature, siglen) < 0) { + if (base64_encode(buf+i, buf_len-i, signature, siglen, + BASE64_ENCODE_MULTILINE) < 0) { log_warn(LD_BUG,"couldn't base64-encode signature"); goto err; } diff --git a/src/test/test_crypto.c b/src/test/test_crypto.c index e9fb8bf..299373e 100644 --- a/src/test/test_crypto.c +++ b/src/test/test_crypto.c @@ -14,6 +14,8 @@ #include "crypto_ed25519.h" #include "ed25519_vectors.inc"

+#include <openssl/evp.h> + extern const char AUTHORITY_SIGNKEY_3[]; extern const char AUTHORITY_SIGNKEY_A_DIGEST[]; extern const char AUTHORITY_SIGNKEY_A_DIGEST256[]; @@ -601,6 +603,22 @@ test_crypto_digests(void *arg) crypto_pk_free(k); }

+/** Encode src into dest with OpenSSL's EVP Encode interface, returning the + * length of the encoded data in bytes. + */ +static int +base64_encode_evp(char *dest, char *src, size_t srclen) +{ + const unsigned char *s = (unsigned char*)src; + EVP_ENCODE_CTX ctx; + int len, ret; + + EVP_EncodeInit(&ctx); + EVP_EncodeUpdate(&ctx, (unsigned char *)dest, &len, s, srclen); + EVP_EncodeFinal(&ctx, (unsigned char *)(dest + len), &ret); + return ret+ len; +} + /** Run unit tests for misc crypto formatting functionality (base64, base32, * fingerprints, etc) */ static void @@ -618,7 +636,7 @@ test_crypto_formats(void *arg) /* Base64 tests */ memset(data1, 6, 1024); for (idx = 0; idx < 10; ++idx) { - i = base64_encode(data2, 1024, data1, idx); + i = base64_encode(data2, 1024, data1, idx, 0); tt_int_op(i, OP_GE, 0); j = base64_decode(data3, 1024, data2, i); tt_int_op(j,OP_EQ, idx); @@ -628,7 +646,7 @@ test_crypto_formats(void *arg) strlcpy(data1, "Test string that contains 35 chars.", 1024); strlcat(data1, " 2nd string that contains 35 chars.", 1024);

- i = base64_encode(data2, 1024, data1, 71); + i = base64_encode(data2, 1024, data1, 71, 0); tt_int_op(i, OP_GE, 0); j = base64_decode(data3, 1024, data2, i); tt_int_op(j,OP_EQ, 71); @@ -647,6 +665,20 @@ test_crypto_formats(void *arg)

tt_assert(digest_from_base64(data3, "###") < 0);

+ for (i = 0; i < 256; i++) { + /* Test the multiline format Base64 encoder with 0 .. 256 bytes of + * output against OpenSSL. + */ + const size_t enclen = base64_encode_size(i, BASE64_ENCODE_MULTILINE); + data1[i] = i; + j = base64_encode(data2, 1024, data1, i, BASE64_ENCODE_MULTILINE); + tt_int_op(j, OP_EQ, enclen); + j = base64_encode_evp(data3, data1, i); + tt_int_op(j, OP_EQ, enclen); + tt_mem_op(data2, OP_EQ, data3, enclen); + tt_int_op(j, OP_EQ, strlen(data2)); + } + /* Encoding SHA256 */ crypto_rand(data2, DIGEST256_LEN); memset(data2, 100, 1024); diff --git a/src/tools/tor-gencert.c b/src/tools/tor-gencert.c index 5ae1556..c0cab7e 100644 --- a/src/tools/tor-gencert.c +++ b/src/tools/tor-gencert.c @@ -486,7 +486,8 @@ generate_certificate(void) EVP_PKEY_get1_RSA(signing_key), RSA_PKCS1_PADDING); signed_len = strlen(buf); - base64_encode(buf+signed_len, sizeof(buf)-signed_len, signature, r); + base64_encode(buf+signed_len, sizeof(buf)-signed_len, signature, r, + BASE64_ENCODE_MULTILINE);

strlcat(buf, "-----END ID SIGNATURE-----\n" @@ -501,7 +502,8 @@ generate_certificate(void) RSA_PKCS1_PADDING); strlcat(buf, "-----BEGIN SIGNATURE-----\n", sizeof(buf)); signed_len = strlen(buf); - base64_encode(buf+signed_len, sizeof(buf)-signed_len, signature, r); + base64_encode(buf+signed_len, sizeof(buf)-signed_len, signature, r, + BASE64_ENCODE_MULTILINE); strlcat(buf, "-----END SIGNATURE-----\n", sizeof(buf));

if (!(f = fopen(certificate_file, "w"))) {