1183065 - Certificate Blocklist lacks convenient logging on revocation checks

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Mark Goodwin [:mgoodwin]]

It's tricker than it should be to diagnose why certificate blocklist items are not working correctly. This is easily fixed by adding some debug log messages.

Comment 1

[Mark Goodwin [:mgoodwin]]

Attachment: Bug 1183065.patch

Comment 2

[:Cykesiopka]

Comment on attachment 8632738 [details] [diff] [review]
Bug 1183065.patch

Review of attachment 8632738 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, just some nits.

::: security/manager/ssl/CertBlocklist.cpp
@@ +574,5 @@
>      return rv;
>    }
>  
> +  nsAutoCString encDN;
> +  nsAutoCString encOther;

Optional: move these to just above where they're first used.

@@ +597,5 @@
> +  MOZ_LOG(gCertBlockPRLog, LogLevel::Warning,
> +          ("CertBlocklist::IsCertRevoked issuer %s - serial %s",
> +           encDN.get(), encOther.get()));
> +
> +

Minor nit: unnecessary extra line.

@@ +602,5 @@
>    *_retval = mBlocklist.Contains(issuerSerial);
>  
>    if (*_retval) {
> +    MOZ_LOG(gCertBlockPRLog, LogLevel::Warning,
> +            ("certblocklist::found by issuer / serial; returning"));

Nit: just for consistency, maybe "CertBlocklist::IsCertRevoked found by [...]"?

Comment 3

[Mark Goodwin [:mgoodwin]]

Attachment: Bug 1183065.patch

Feedback addressed

Comment 4

[Mark Goodwin [:mgoodwin]]

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/ec1b5a7d05e941f4b4bb4ef38d949011f877124f
changeset:  ec1b5a7d05e941f4b4bb4ef38d949011f877124f
user:       Mark Goodwin <[e-mail]>
date:       Fri Jul 17 10:03:21 2015 +0100
description:
Bug 1183065 - Add logging on OneCRL revocation checks (r=Cykesiopka)

Comment 5

[Mark Goodwin [:mgoodwin]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2e335112b28e <- try, for reference

Comment 6

[Mark Goodwin [:mgoodwin]]

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/d2f8da515976518f38bdb9916c3e22e9baafb013
changeset:  d2f8da515976518f38bdb9916c3e22e9baafb013
user:       Mark Goodwin <[e-mail]>
date:       Fri Jul 17 10:37:00 2015 +0100
description:
Backed out changeset ec1b5a7d05e9 (bug 1183065)

Comment 7

[Mark Goodwin [:mgoodwin]]

url:        https://hg.mozilla.org/integration/mozilla-inbound/rev/598cb986c19724cb959ad34e9326970a17dcf9ce
changeset:  598cb986c19724cb959ad34e9326970a17dcf9ce
user:       Mark Goodwin <[e-mail]>
date:       Fri Jul 17 17:07:47 2015 +0100
description:
Bug 1183065 - Add logging on OneCRL revocation checks (r=Cykesiopka)

Comment 8

[Mark Goodwin [:mgoodwin]]

A clean try run <- https://treeherder.mozilla.org/#/jobs?repo=try&revision=8ee53e4a4aa4

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/598cb986c197