commit cb466ee7d6f7f475783847a7201eaa6a3e1c711b Author: David Goulet dgoulet@torproject.org Date: Wed Aug 22 11:32:57 2018 -0400

key: Make ed_key_init_from_file() take an or_options_t

Part of #27215, we need to call the ed_key_init_from_file function during option_validate() which is before the global_options variable is set.

This commit make ed_key_init_from_file() stop using get_options() and instead now has a or_options_t parameter.

Signed-off-by: David Goulet dgoulet@torproject.org --- src/feature/hs/hs_service.c | 4 ++-- src/feature/relay/routerkeys.c | 21 +++++++++++++-------- src/feature/relay/routerkeys.h | 3 ++- src/test/test_routerkeys.c | 36 +++++++++++++++++++----------------- 4 files changed, 36 insertions(+), 28 deletions(-)

diff --git a/src/feature/hs/hs_service.c b/src/feature/hs/hs_service.c index 8a9429271..a9364a751 100644 --- a/src/feature/hs/hs_service.c +++ b/src/feature/hs/hs_service.c @@ -1037,7 +1037,7 @@ load_service_keys(hs_service_t *service) /* Don't ask for key creation, we want to know if we were able to load it or * we had to generate it. Better logging! */ kp = ed_key_init_from_file(fname, INIT_ED_KEY_SPLIT, LOG_INFO, NULL, 0, 0, - 0, NULL); + 0, NULL, NULL); if (!kp) { log_info(LD_REND, "Unable to load keys from %s. Generating it...", fname); /* We'll now try to generate the keys and for it we want the strongest @@ -1045,7 +1045,7 @@ load_service_keys(hs_service_t *service) uint32_t key_flags = INIT_ED_KEY_CREATE | INIT_ED_KEY_EXTRA_STRONG | INIT_ED_KEY_SPLIT; kp = ed_key_init_from_file(fname, key_flags, LOG_WARN, NULL, 0, 0, 0, - NULL); + NULL, NULL); if (!kp) { log_warn(LD_REND, "Unable to generate keys and save in %s.", fname); goto end; diff --git a/src/feature/relay/routerkeys.c b/src/feature/relay/routerkeys.c index f12eb3d33..294d452e1 100644 --- a/src/feature/relay/routerkeys.c +++ b/src/feature/relay/routerkeys.c @@ -248,6 +248,9 @@ write_secret_key(const ed25519_secret_key_t *key, int encrypted, * <b>fname</b>, with certificate type <b>cert_type</b>. On failure, return * NULL; on success return the keypair. * + * The <b>options</b> is used to look at the change_key_passphrase value when + * writing to disk a secret key. It is safe to be NULL even in that case. + * * If INIT_ED_KEY_CREATE is set in <b>flags</b>, then create the key (and * certificate if requested) if it doesn't exist, and save it to disk. * @@ -276,9 +279,6 @@ write_secret_key(const ed25519_secret_key_t *key, int encrypted, * secret key unless no public key is found. Do not return a secret key. (but * create and save one if needed). * - * If INIT_ED_KEY_NO_LOAD_SECRET is set in <b>flags</b>, don't try to load - * a secret key, no matter what. - * * If INIT_ED_KEY_TRY_ENCRYPTED is set, we look for an encrypted secret key * and consider encrypting any new secret key. * @@ -291,6 +291,9 @@ write_secret_key(const ed25519_secret_key_t *key, int encrypted, * * If INIT_ED_KEY_EXPLICIT_FNAME is set, use the provided file name for the * secret key file, encrypted or not. + * + * If INIT_ED_KEY_OFFLINE_SECRET is set, we won't try to load the master + * secret key and we log a message at <b>severity</b> that we've done so. */ ed25519_keypair_t * ed_key_init_from_file(const char *fname, uint32_t flags, @@ -299,7 +302,8 @@ ed_key_init_from_file(const char *fname, uint32_t flags, time_t now, time_t lifetime, uint8_t cert_type, - struct tor_cert_st **cert_out) + struct tor_cert_st **cert_out, + const or_options_t *options) { char *secret_fname = NULL; char *encrypted_secret_fname = NULL; @@ -503,7 +507,8 @@ ed_key_init_from_file(const char *fname, uint32_t flags,

/* Write it to disk if we're supposed to do with a new passphrase, or if * we just created it. */ - if (created_sk || (have_secret && get_options()->change_key_passphrase)) { + if (created_sk || (have_secret && options != NULL && + options->change_key_passphrase)) { if (write_secret_key(&keypair->seckey, encrypt_key, secret_fname, tag, encrypted_secret_fname) < 0 @@ -734,7 +739,7 @@ load_ed_keys(const or_options_t *options, time_t now) INIT_ED_KEY_NEEDCERT| INIT_ED_KEY_INCLUDE_SIGNING_KEY_IN_CERT, LOG_INFO, - NULL, 0, 0, CERT_TYPE_ID_SIGNING, &sign_cert); + NULL, 0, 0, CERT_TYPE_ID_SIGNING, &sign_cert, options); tor_free(fname); check_signing_cert = sign_cert; use_signing = sign; @@ -836,7 +841,7 @@ load_ed_keys(const or_options_t *options, time_t now) id = ed_key_init_from_file( fname, flags, - LOG_WARN, NULL, 0, 0, 0, NULL); + LOG_WARN, NULL, 0, 0, 0, NULL, options); tor_free(fname); if (!id) { if (need_new_signing_key) { @@ -904,7 +909,7 @@ load_ed_keys(const or_options_t *options, time_t now) flags, LOG_WARN, sign_signing_key_with_id, now, options->SigningKeyLifetime, - CERT_TYPE_ID_SIGNING, &sign_cert); + CERT_TYPE_ID_SIGNING, &sign_cert, options); tor_free(fname); if (!sign) FAIL("Missing signing key"); diff --git a/src/feature/relay/routerkeys.h b/src/feature/relay/routerkeys.h index a6f06f6e2..f52ed0f30 100644 --- a/src/feature/relay/routerkeys.h +++ b/src/feature/relay/routerkeys.h @@ -27,7 +27,8 @@ ed25519_keypair_t *ed_key_init_from_file(const char *fname, uint32_t flags, time_t now, time_t lifetime, uint8_t cert_type, - struct tor_cert_st **cert_out); + struct tor_cert_st **cert_out, + const or_options_t *options); ed25519_keypair_t *ed_key_new(const ed25519_keypair_t *signing_key, uint32_t flags, time_t now, diff --git a/src/test/test_routerkeys.c b/src/test/test_routerkeys.c index 1a1bf63ba..1a9664aa0 100644 --- a/src/test/test_routerkeys.c +++ b/src/test/test_routerkeys.c @@ -262,13 +262,14 @@ test_routerkeys_ed_key_init_basic(void *arg) unlink(fname2);

/* Fail to load a key that isn't there. */ - kp1 = ed_key_init_from_file(fname1, 0, LOG_INFO, NULL, now, 0, 7, &cert); + kp1 = ed_key_init_from_file(fname1, 0, LOG_INFO, NULL, now, 0, 7, &cert, + NULL); tt_assert(kp1 == NULL); tt_assert(cert == NULL);

/* Create the key if requested to do so. */ kp1 = ed_key_init_from_file(fname1, INIT_ED_KEY_CREATE, LOG_INFO, - NULL, now, 0, 7, &cert); + NULL, now, 0, 7, &cert, NULL); tt_assert(kp1 != NULL); tt_assert(cert == NULL); tt_int_op(stat(get_fname("test_ed_key_1_cert"), &st), OP_LT, 0); @@ -276,24 +277,24 @@ test_routerkeys_ed_key_init_basic(void *arg)

/* Fail to load if we say we need a cert */ kp2 = ed_key_init_from_file(fname1, INIT_ED_KEY_NEEDCERT, LOG_INFO, - NULL, now, 0, 7, &cert); + NULL, now, 0, 7, &cert, NULL); tt_assert(kp2 == NULL);

/* Fail to load if we say the wrong key type */ kp2 = ed_key_init_from_file(fname1, 0, LOG_INFO, - NULL, now, 0, 6, &cert); + NULL, now, 0, 6, &cert, NULL); tt_assert(kp2 == NULL);

/* Load successfully if we're not picky, whether we say "create" or not. */ kp2 = ed_key_init_from_file(fname1, INIT_ED_KEY_CREATE, LOG_INFO, - NULL, now, 0, 7, &cert); + NULL, now, 0, 7, &cert, NULL); tt_assert(kp2 != NULL); tt_assert(cert == NULL); tt_mem_op(kp1, OP_EQ, kp2, sizeof(*kp1)); ed25519_keypair_free(kp2); kp2 = NULL;

kp2 = ed_key_init_from_file(fname1, 0, LOG_INFO, - NULL, now, 0, 7, &cert); + NULL, now, 0, 7, &cert, NULL); tt_assert(kp2 != NULL); tt_assert(cert == NULL); tt_mem_op(kp1, OP_EQ, kp2, sizeof(*kp1)); @@ -302,7 +303,7 @@ test_routerkeys_ed_key_init_basic(void *arg) /* Now create a key with a cert. */ kp2 = ed_key_init_from_file(fname2, (INIT_ED_KEY_CREATE| INIT_ED_KEY_NEEDCERT), - LOG_INFO, kp1, now, 7200, 7, &cert); + LOG_INFO, kp1, now, 7200, 7, &cert, NULL); tt_assert(kp2 != NULL); tt_assert(cert != NULL); tt_mem_op(kp1, OP_NE, kp2, sizeof(*kp1)); @@ -315,7 +316,7 @@ test_routerkeys_ed_key_init_basic(void *arg) /* Now verify we can load the cert... */ kp3 = ed_key_init_from_file(fname2, (INIT_ED_KEY_CREATE| INIT_ED_KEY_NEEDCERT), - LOG_INFO, kp1, now, 7200, 7, &cert2); + LOG_INFO, kp1, now, 7200, 7, &cert2, NULL); tt_mem_op(kp2, OP_EQ, kp3, sizeof(*kp2)); tt_mem_op(cert2->encoded, OP_EQ, cert->encoded, cert->encoded_len); ed25519_keypair_free(kp3); kp3 = NULL; @@ -323,7 +324,7 @@ test_routerkeys_ed_key_init_basic(void *arg)

/* ... even without create... */ kp3 = ed_key_init_from_file(fname2, INIT_ED_KEY_NEEDCERT, - LOG_INFO, kp1, now, 7200, 7, &cert2); + LOG_INFO, kp1, now, 7200, 7, &cert2, NULL); tt_mem_op(kp2, OP_EQ, kp3, sizeof(*kp2)); tt_mem_op(cert2->encoded, OP_EQ, cert->encoded, cert->encoded_len); ed25519_keypair_free(kp3); kp3 = NULL; @@ -331,13 +332,13 @@ test_routerkeys_ed_key_init_basic(void *arg)

/* ... but that we don't crash or anything if we say we don't want it. */ kp3 = ed_key_init_from_file(fname2, INIT_ED_KEY_NEEDCERT, - LOG_INFO, kp1, now, 7200, 7, NULL); + LOG_INFO, kp1, now, 7200, 7, NULL, NULL); tt_mem_op(kp2, OP_EQ, kp3, sizeof(*kp2)); ed25519_keypair_free(kp3); kp3 = NULL;

/* Fail if we're told the wrong signing key */ kp3 = ed_key_init_from_file(fname2, INIT_ED_KEY_NEEDCERT, - LOG_INFO, kp2, now, 7200, 7, &cert2); + LOG_INFO, kp2, now, 7200, 7, &cert2, NULL); tt_assert(kp3 == NULL); tt_assert(cert2 == NULL);

@@ -368,13 +369,14 @@ test_routerkeys_ed_key_init_split(void *arg) unlink(fname2);

/* Can't load key that isn't there. */ - kp1 = ed_key_init_from_file(fname1, flags, LOG_INFO, NULL, now, 0, 7, &cert); + kp1 = ed_key_init_from_file(fname1, flags, LOG_INFO, NULL, now, 0, 7, &cert, + NULL); tt_assert(kp1 == NULL); tt_assert(cert == NULL);

/* Create a split key */ kp1 = ed_key_init_from_file(fname1, flags|INIT_ED_KEY_CREATE, - LOG_INFO, NULL, now, 0, 7, &cert); + LOG_INFO, NULL, now, 0, 7, &cert, NULL); tt_assert(kp1 != NULL); tt_assert(cert == NULL); tt_int_op(stat(get_fname("test_ed_key_3_cert"), &st), OP_LT, 0); @@ -383,7 +385,7 @@ test_routerkeys_ed_key_init_split(void *arg)

/* Load it. */ kp2 = ed_key_init_from_file(fname1, flags|INIT_ED_KEY_CREATE, - LOG_INFO, NULL, now, 0, 7, &cert); + LOG_INFO, NULL, now, 0, 7, &cert, NULL); tt_assert(kp2 != NULL); tt_assert(cert == NULL); tt_mem_op(kp1, OP_EQ, kp2, sizeof(*kp2)); @@ -392,7 +394,7 @@ test_routerkeys_ed_key_init_split(void *arg) /* Okay, try killing the secret key and loading it. */ unlink(get_fname("test_ed_key_3_secret_key")); kp2 = ed_key_init_from_file(fname1, flags, - LOG_INFO, NULL, now, 0, 7, &cert); + LOG_INFO, NULL, now, 0, 7, &cert, NULL); tt_assert(kp2 != NULL); tt_assert(cert == NULL); tt_mem_op(&kp1->pubkey, OP_EQ, &kp2->pubkey, sizeof(kp2->pubkey)); @@ -402,7 +404,7 @@ test_routerkeys_ed_key_init_split(void *arg)

/* Even when we're told to "create", don't create if there's a public key */ kp2 = ed_key_init_from_file(fname1, flags|INIT_ED_KEY_CREATE, - LOG_INFO, NULL, now, 0, 7, &cert); + LOG_INFO, NULL, now, 0, 7, &cert, NULL); tt_assert(kp2 != NULL); tt_assert(cert == NULL); tt_mem_op(&kp1->pubkey, OP_EQ, &kp2->pubkey, sizeof(kp2->pubkey)); @@ -412,7 +414,7 @@ test_routerkeys_ed_key_init_split(void *arg)

/* Make sure we fail on a tag mismatch, though */ kp2 = ed_key_init_from_file(fname1, flags, - LOG_INFO, NULL, now, 0, 99, &cert); + LOG_INFO, NULL, now, 0, 99, &cert, NULL); tt_assert(kp2 == NULL);

done: