[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-commits] [tor/master] Followup: Make authority_cert_parse_from_string() take length too
commit 04bb70199be924804708bd4ace18b28b5acdbf19
Author: Nick Mathewson <nickm@xxxxxxxxxxxxxx>
Date: Tue Sep 11 11:37:55 2018 -0400
Followup: Make authority_cert_parse_from_string() take length too
---
src/feature/nodelist/routerlist.c | 3 ++-
src/feature/nodelist/routerparse.c | 19 +++++++++++--------
src/feature/nodelist/routerparse.h | 1 +
src/feature/relay/router.c | 2 +-
src/test/test_dir.c | 12 +++++++++---
src/test/test_dir_common.c | 12 +++++++++---
src/test/test_dir_handle_get.c | 16 ++++++++++++----
src/test/test_routerlist.c | 12 +++++++++---
src/test/test_shared_random.c | 12 +++++++++---
9 files changed, 63 insertions(+), 26 deletions(-)
diff --git a/src/feature/nodelist/routerlist.c b/src/feature/nodelist/routerlist.c
index bcc5c1f07..c3c72e9c7 100644
--- a/src/feature/nodelist/routerlist.c
+++ b/src/feature/nodelist/routerlist.c
@@ -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;
diff --git a/src/feature/nodelist/routerparse.c b/src/feature/nodelist/routerparse.c
index 9abdfb614..8a5efc007 100644
--- a/src/feature/nodelist/routerparse.c
+++ b/src/feature/nodelist/routerparse.c
@@ -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;
}
diff --git a/src/feature/nodelist/routerparse.h b/src/feature/nodelist/routerparse.h
index 390088c94..c60046e8e 100644
--- a/src/feature/nodelist/routerparse.h
+++ b/src/feature/nodelist/routerparse.h
@@ -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,
diff --git a/src/feature/relay/router.c b/src/feature/relay/router.c
index 1f316ebf0..17f63e998 100644
--- a/src/feature/relay/router.c
+++ b/src/feature/relay/router.c
@@ -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;
diff --git a/src/test/test_dir.c b/src/test/test_dir.c
index 0fa5c3103..078a383dd 100644
--- a/src/test/test_dir.c
+++ b/src/test/test_dir.c
@@ -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();
diff --git a/src/test/test_dir_common.c b/src/test/test_dir_common.c
index 6e3bb9945..63bd082ee 100644
--- a/src/test/test_dir_common.c
+++ b/src/test/test_dir_common.c
@@ -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();
diff --git a/src/test/test_dir_handle_get.c b/src/test/test_dir_handle_get.c
index 09799a0e5..ecb7c1b5f 100644
--- a/src/test/test_dir_handle_get.c
+++ b/src/test/test_dir_handle_get.c
@@ -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,
diff --git a/src/test/test_routerlist.c b/src/test/test_routerlist.c
index bf76570b3..e7c577cf6 100644
--- a/src/test/test_routerlist.c
+++ b/src/test/test_routerlist.c
@@ -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);
diff --git a/src/test/test_shared_random.c b/src/test/test_shared_random.c
index 70adf580a..204361364 100644
--- a/src/test/test_shared_random.c
+++ b/src/test/test_shared_random.c
@@ -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;
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits