commit ee5c624bebbc8e823ed49fe563fd56294fb226d4
Author: Nick Mathewson <nickm(a)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);