Followup: Make authority_cert_parse_from_string() take length too

This commit is contained in:
Nick Mathewson 2018-09-11 11:37:55 -04:00
parent 7e3005af30
commit 04bb70199b
9 changed files with 63 additions and 26 deletions

View file

@ -555,7 +555,8 @@ trusted_dirs_load_certs_from_string(const char *contents, int source,
int added_trusted_cert = 0;
for (s = contents; *s; s = eos) {
authority_cert_t *cert = authority_cert_parse_from_string(s, &eos);
authority_cert_t *cert = authority_cert_parse_from_string(s, strlen(s),
&eos);
cert_list_t *cl;
if (!cert) {
failure_code = -1;

View file

@ -2308,7 +2308,8 @@ extrainfo_parse_entry_from_string(const char *s, const char *end,
/** Parse a key certificate from <b>s</b>; point <b>end-of-string</b> to
* the first character after the certificate. */
authority_cert_t *
authority_cert_parse_from_string(const char *s, const char **end_of_string)
authority_cert_parse_from_string(const char *s, size_t maxlen,
const char **end_of_string)
{
/** Reject any certificate at least this big; it is probably an overflow, an
* attack, a bug, or some other nonsense. */
@ -2319,24 +2320,25 @@ authority_cert_parse_from_string(const char *s, const char **end_of_string)
char digest[DIGEST_LEN];
directory_token_t *tok;
char fp_declared[DIGEST_LEN];
char *eos;
const char *eos;
size_t len;
int found;
memarea_t *area = NULL;
const char *end_of_s = s + maxlen;
const char *s_dup = s;
s = eat_whitespace(s);
eos = strstr(s, "\ndir-key-certification");
s = eat_whitespace_eos(s, end_of_s);
eos = tor_memstr(s, end_of_s - s, "\ndir-key-certification");
if (! eos) {
log_warn(LD_DIR, "No signature found on key certificate");
return NULL;
}
eos = strstr(eos, "\n-----END SIGNATURE-----\n");
eos = tor_memstr(eos, end_of_s - eos, "\n-----END SIGNATURE-----\n");
if (! eos) {
log_warn(LD_DIR, "No end-of-signature found on key certificate");
return NULL;
}
eos = strchr(eos+2, '\n');
eos = memchr(eos+2, '\n', end_of_s - (eos+2));
tor_assert(eos);
++eos;
len = eos - s;
@ -2353,7 +2355,7 @@ authority_cert_parse_from_string(const char *s, const char **end_of_string)
log_warn(LD_DIR, "Error tokenizing key certificate");
goto err;
}
if (router_get_hash_impl(s, strlen(s), digest, "dir-key-certificate-version",
if (router_get_hash_impl(s, eos-s, digest, "dir-key-certificate-version",
"\ndir-key-certification", '\n', DIGEST_SHA1) < 0)
goto err;
tok = smartlist_get(tokens, 0);
@ -3465,7 +3467,8 @@ networkstatus_parse_vote_from_string(const char *s,
"\ndir-key-certificate-version")))
goto err;
++cert;
ns->cert = authority_cert_parse_from_string(cert, &end_of_cert);
ns->cert = authority_cert_parse_from_string(cert, end_of_header - cert,
&end_of_cert);
if (!ns->cert || !end_of_cert || end_of_cert > end_of_header)
goto err;
}

View file

@ -93,6 +93,7 @@ smartlist_t *microdescs_parse_from_string(const char *s, const char *eos,
smartlist_t *invalid_digests_out);
authority_cert_t *authority_cert_parse_from_string(const char *s,
size_t maxlen,
const char **end_of_string);
int rend_parse_v2_service_descriptor(rend_service_descriptor_t **parsed_out,
char *desc_id_out,

View file

@ -717,7 +717,7 @@ load_authority_keyset(int legacy, crypto_pk_t **key_out,
fname);
goto done;
}
parsed = authority_cert_parse_from_string(cert, &eos);
parsed = authority_cert_parse_from_string(cert, strlen(cert), &eos);
if (!parsed) {
log_warn(LD_DIR, "Unable to parse certificate in %s", fname);
goto done;

View file

@ -2799,11 +2799,17 @@ test_a_networkstatus(
MOCK(get_my_v3_authority_cert, get_my_v3_authority_cert_m);
/* Parse certificates and keys. */
cert1 = mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1, NULL);
cert1 = mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1,
strlen(AUTHORITY_CERT_1),
NULL);
tt_assert(cert1);
cert2 = authority_cert_parse_from_string(AUTHORITY_CERT_2, NULL);
cert2 = authority_cert_parse_from_string(AUTHORITY_CERT_2,
strlen(AUTHORITY_CERT_2),
NULL);
tt_assert(cert2);
cert3 = authority_cert_parse_from_string(AUTHORITY_CERT_3, NULL);
cert3 = authority_cert_parse_from_string(AUTHORITY_CERT_3,
strlen(AUTHORITY_CERT_3),
NULL);
tt_assert(cert3);
sign_skey_1 = crypto_pk_new();
sign_skey_2 = crypto_pk_new();

View file

@ -40,14 +40,20 @@ dir_common_authority_pk_init(authority_cert_t **cert1,
{
/* Parse certificates and keys. */
authority_cert_t *cert;
cert = authority_cert_parse_from_string(AUTHORITY_CERT_1, NULL);
cert = authority_cert_parse_from_string(AUTHORITY_CERT_1,
strlen(AUTHORITY_CERT_1),
NULL);
tt_assert(cert);
tt_assert(cert->identity_key);
*cert1 = cert;
tt_assert(*cert1);
*cert2 = authority_cert_parse_from_string(AUTHORITY_CERT_2, NULL);
*cert2 = authority_cert_parse_from_string(AUTHORITY_CERT_2,
strlen(AUTHORITY_CERT_2),
NULL);
tt_assert(*cert2);
*cert3 = authority_cert_parse_from_string(AUTHORITY_CERT_3, NULL);
*cert3 = authority_cert_parse_from_string(AUTHORITY_CERT_3,
strlen(AUTHORITY_CERT_3),
NULL);
tt_assert(*cert3);
*sign_skey_1 = crypto_pk_new();
*sign_skey_2 = crypto_pk_new();

View file

@ -1270,7 +1270,9 @@ test_dir_handle_get_server_keys_authority(void* data)
size_t body_used = 0;
(void) data;
mock_cert = authority_cert_parse_from_string(TEST_CERTIFICATE, NULL);
mock_cert = authority_cert_parse_from_string(TEST_CERTIFICATE,
strlen(TEST_CERTIFICATE),
NULL);
MOCK(get_my_v3_authority_cert, get_my_v3_authority_cert_m);
MOCK(connection_write_to_buf_impl_, connection_write_to_buf_mock);
@ -1420,7 +1422,9 @@ test_dir_handle_get_server_keys_sk(void* data)
size_t body_used = 0;
(void) data;
mock_cert = authority_cert_parse_from_string(TEST_CERTIFICATE, NULL);
mock_cert = authority_cert_parse_from_string(TEST_CERTIFICATE,
strlen(TEST_CERTIFICATE),
NULL);
MOCK(get_my_v3_authority_cert, get_my_v3_authority_cert_m);
MOCK(connection_write_to_buf_impl_, connection_write_to_buf_mock);
@ -2388,7 +2392,9 @@ test_dir_handle_get_status_vote_next_authority(void* data)
routerlist_free_all();
dirvote_free_all();
mock_cert = authority_cert_parse_from_string(TEST_CERTIFICATE, NULL);
mock_cert = authority_cert_parse_from_string(TEST_CERTIFICATE,
strlen(TEST_CERTIFICATE),
NULL);
/* create a trusted ds */
ds = trusted_dir_server_new("ds", "127.0.0.1", 9059, 9060, NULL, digest,
@ -2466,7 +2472,9 @@ test_dir_handle_get_status_vote_current_authority(void* data)
routerlist_free_all();
dirvote_free_all();
mock_cert = authority_cert_parse_from_string(TEST_CERTIFICATE, NULL);
mock_cert = authority_cert_parse_from_string(TEST_CERTIFICATE,
strlen(TEST_CERTIFICATE),
NULL);
/* create a trusted ds */
ds = trusted_dir_server_new("ds", "127.0.0.1", 9059, 9060, NULL, digest,

View file

@ -260,7 +260,9 @@ test_router_pick_directory_server_impl(void *arg)
/* Init SR subsystem. */
MOCK(get_my_v3_authority_cert, get_my_v3_authority_cert_m);
mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1, NULL);
mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1,
strlen(AUTHORITY_CERT_1),
NULL);
sr_init(0);
UNMOCK(get_my_v3_authority_cert);
@ -472,7 +474,9 @@ test_directory_guard_fetch_with_no_dirinfo(void *arg)
/* Initialize the SRV subsystem */
MOCK(get_my_v3_authority_cert, get_my_v3_authority_cert_m);
mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1, NULL);
mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1,
strlen(AUTHORITY_CERT_1),
NULL);
sr_init(0);
UNMOCK(get_my_v3_authority_cert);
@ -645,7 +649,9 @@ test_skew_common(void *arg, time_t now, unsigned long *offset)
/* Initialize the SRV subsystem */
MOCK(get_my_v3_authority_cert, get_my_v3_authority_cert_m);
mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1, NULL);
mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1,
strlen(AUTHORITY_CERT_1),
NULL);
sr_init(0);
UNMOCK(get_my_v3_authority_cert);

View file

@ -64,7 +64,9 @@ init_authority_state(void)
MOCK(get_my_v3_authority_cert, get_my_v3_authority_cert_m);
or_options_t *options = get_options_mutable();
mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1, NULL);
mock_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1,
strlen(AUTHORITY_CERT_1),
NULL);
tt_assert(mock_cert);
options->AuthoritativeDir = 1;
tt_int_op(load_ed_keys(options, time(NULL)), OP_GE, 0);
@ -420,7 +422,9 @@ test_sr_commit(void *arg)
{ /* Setup a minimal dirauth environment for this test */
or_options_t *options = get_options_mutable();
auth_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1, NULL);
auth_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1,
strlen(AUTHORITY_CERT_1),
NULL);
tt_assert(auth_cert);
options->AuthoritativeDir = 1;
@ -823,7 +827,9 @@ test_sr_setup_commits(void)
{ /* Setup a minimal dirauth environment for this test */
or_options_t *options = get_options_mutable();
auth_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1, NULL);
auth_cert = authority_cert_parse_from_string(AUTHORITY_CERT_1,
strlen(AUTHORITY_CERT_1),
NULL);
tt_assert(auth_cert);
options->AuthoritativeDir = 1;