commit ee5c624bebbc8e823ed49fe563fd56294fb226d4 Author: Nick Mathewson <nickm@torproject.org> Date: Fri Jan 26 13:55:25 2018 -0500 When a tor_cert_T check fails, log the reason why. Diagnostic attempt for 24972. --- changes/bug24972 | 4 ++++ src/or/hs_client.c | 6 ++++-- src/or/hs_descriptor.c | 9 ++++++--- src/or/routerkeys.c | 11 ++++++++--- src/or/torcert.c | 21 ++++++++++++++++++++- src/or/torcert.h | 1 + 6 files changed, 43 insertions(+), 9 deletions(-) diff --git a/changes/bug24972 b/changes/bug24972 new file mode 100644 index 000000000..5adf970ab --- /dev/null +++ b/changes/bug24972 @@ -0,0 +1,4 @@ + o Minor features (logging, diagnostic): + - When logging a failure to check a hidden service's certificate, + also log what the problem with the certificate was. Diagnostic + for ticket 24972. diff --git a/src/or/hs_client.c b/src/or/hs_client.c index 9ac653c72..551cf5055 100644 --- a/src/or/hs_client.c +++ b/src/or/hs_client.c @@ -1229,10 +1229,12 @@ hs_client_decode_descriptor(const char *desc_str, /* Make sure the descriptor signing key cross certifies with the computed * blinded key. Without this validation, anyone knowing the subcredential * and onion address can forge a descriptor. */ - if (tor_cert_checksig((*desc)->plaintext_data.signing_key_cert, + tor_cert_t *cert = (*desc)->plaintext_data.signing_key_cert; + if (tor_cert_checksig(cert, &blinded_pubkey, approx_time()) < 0) { log_warn(LD_GENERAL, "Descriptor signing key certificate signature " - "doesn't validate with computed blinded key."); + "doesn't validate with computed blinded key: %s", + tor_cert_describe_signature_status(cert)); goto err; } diff --git a/src/or/hs_descriptor.c b/src/or/hs_descriptor.c index 170886694..9683fca50 100644 --- a/src/or/hs_descriptor.c +++ b/src/or/hs_descriptor.c @@ -1233,7 +1233,8 @@ cert_is_valid(tor_cert_t *cert, uint8_t type, const char *log_obj_type) /* The following will not only check if the signature matches but also the * expiration date and overall validity. */ if (tor_cert_checksig(cert, &cert->signing_key, approx_time()) < 0) { - log_warn(LD_REND, "Invalid signature for %s.", log_obj_type); + log_warn(LD_REND, "Invalid signature for %s: %s", log_obj_type, + tor_cert_describe_signature_status(cert)); goto err; } @@ -1728,7 +1729,8 @@ decode_introduction_point(const hs_descriptor_t *desc, const char *start) /* Validate authentication certificate with descriptor signing key. */ if (tor_cert_checksig(ip->auth_key_cert, &desc->plaintext_data.signing_pubkey, 0) < 0) { - log_warn(LD_REND, "Invalid authentication key signature"); + log_warn(LD_REND, "Invalid authentication key signature: %s", + tor_cert_describe_signature_status(ip->auth_key_cert)); goto err; } @@ -1765,7 +1767,8 @@ decode_introduction_point(const hs_descriptor_t *desc, const char *start) } if (tor_cert_checksig(ip->enc_key_cert, &desc->plaintext_data.signing_pubkey, 0) < 0) { - log_warn(LD_REND, "Invalid encryption key signature"); + log_warn(LD_REND, "Invalid encryption key signature: %s", + tor_cert_describe_signature_status(ip->enc_key_cert)); goto err; } /* It is successfully cross certified. Flag the object. */ diff --git a/src/or/routerkeys.c b/src/or/routerkeys.c index 7295c1965..f0973044b 100644 --- a/src/or/routerkeys.c +++ b/src/or/routerkeys.c @@ -536,7 +536,8 @@ ed_key_init_from_file(const char *fname, uint32_t flags, bad_cert = 1; } else if (signing_key && tor_cert_checksig(cert, &signing_key->pubkey, now) < 0) { - tor_log(severity, LD_OR, "Can't check certificate"); + tor_log(severity, LD_OR, "Can't check certificate: %s", + tor_cert_describe_signature_status(cert)); bad_cert = 1; } else if (cert->cert_expired) { tor_log(severity, LD_OR, "Certificate is expired"); @@ -883,8 +884,12 @@ load_ed_keys(const or_options_t *options, time_t now) if (! ed25519_pubkey_eq(&sign_cert->signing_key, &id->pubkey)) FAIL("The signing cert we have was not signed with the master key " "we loaded!"); - if (tor_cert_checksig(sign_cert, &id->pubkey, 0) < 0) - FAIL("The signing cert we loaded was not signed correctly!"); + if (tor_cert_checksig(sign_cert, &id->pubkey, 0) < 0) { + log_warn(LD_OR, "The signing cert we loaded was not signed " + "correctly: %s!", + tor_cert_describe_signature_status(sign_cert)); + goto err; + } } if (want_new_signing_key && sign_signing_key_with_id) { diff --git a/src/or/torcert.c b/src/or/torcert.c index befb39d6e..212534d31 100644 --- a/src/or/torcert.c +++ b/src/or/torcert.c @@ -93,7 +93,8 @@ tor_cert_sign_impl(const ed25519_keypair_t *signing_key, if (tor_cert_checksig(torcert, &signing_key->pubkey, now) < 0) { /* LCOV_EXCL_START */ - log_warn(LD_BUG, "Generated a certificate whose signature we can't check"); + log_warn(LD_BUG, "Generated a certificate whose signature we can't " + "check: %s", tor_cert_describe_signature_status(torcert)); goto err; /* LCOV_EXCL_STOP */ } @@ -267,6 +268,24 @@ tor_cert_checksig(tor_cert_t *cert, } } +/** Return a string describing the status of the signature on <b>cert</b> + * + * Will always be "unchecked" unless tor_cert_checksig has been called. + */ +const char * +tor_cert_describe_signature_status(const tor_cert_t *cert) +{ + if (cert->cert_expired) { + return "expired"; + } else if (cert->sig_bad) { + return "mis-signed"; + } else if (cert->sig_ok) { + return "okay"; + } else { + return "unchecked"; + } +} + /** Return a new copy of <b>cert</b> */ tor_cert_t * tor_cert_dup(const tor_cert_t *cert) diff --git a/src/or/torcert.h b/src/or/torcert.h index c77ae2089..ac227db20 100644 --- a/src/or/torcert.h +++ b/src/or/torcert.h @@ -66,6 +66,7 @@ int tor_cert_get_checkable_sig(ed25519_checkable_t *checkable_out, int tor_cert_checksig(tor_cert_t *cert, const ed25519_public_key_t *pubkey, time_t now); +const char *tor_cert_describe_signature_status(const tor_cert_t *cert); tor_cert_t *tor_cert_dup(const tor_cert_t *cert); int tor_cert_eq(const tor_cert_t *cert1, const tor_cert_t *cert2);