1335820 - HPKP Warning alert

Status: RESOLVED FIXED

(Infrastructure & Operations Graveyard :: CIDuty, task)

Description

[Hal Wine [:hwine] use NI!]

This popped up in #buildduty today (thanks to :philor for reporting):
[09:04] < nagios-rele>| Wed 09:04:34 PST [4013] nagios1.private.releng.scl3.mozilla.com:HPKP Expiration - Nightly is WARNING: (null) (http://m.mozilla.org/HPKP+Expiration+-+Nightly)

Note the cutover of hg.m.o to https only occurred at ~0800 PT, so if the check used http protocol without following redirects, that could be part of the issue.

Unsure if this is related to bug 1303851

Comment 1

[Chris AtLee [:catlee]]

The format of the file changed in https://hg.mozilla.org/mozilla-central/rev/35aecea31459 due to https://hg.mozilla.org/mozilla-central/rev/b03c9f4ac1b0 (bug 1335294).

We'll need to adjust the nagios check to cope.

Comment 2

[Simon Fraser [:sfraser] ⌚️GMT]

Attachment: proposed fix, before I put in a PR to sysadmins/puppet

Comment 3

[Hal Wine [:hwine] use NI!]

Comment on attachment 8832554 [details]
proposed fix, before I put in a PR to sysadmins/puppet

What's the value of searching for 2 anchors? (with increased complexity)

aiui, the file is auto-generated, so if one is present, the other will be as well. If we're going to handle the corner case of only one, don't we also have to check to ensure both values are identical when both are present?

The file format & check are complex enough that I think an explanation would help.

Comment 4

[Simon Fraser [:sfraser] ⌚️GMT]

They won't both be present, it'll be one or the other, as the change filters through to aurora, and onwards. 

Although I agree about the compexity - I was initially worried about matching on too little, but this should be enough:

-EPOCH_ANCHOR = "static const PRTime kPreloadPKPinsExpirationTime = INT64_C("
+EPOCH_ANCHOR = " kPreloadPKPinsExpirationTime = INT64_C("

Comment 5

[(Away)]

Bug 1335294 was backed out so the file is back to "static const PRTime" at the moment.

I'll need to play with the change a bit before re-landing. I probably won't touch kPreloadPKPinsExpirationTime next time, but I'll report back here if I do. Sorry for the back-and-forth.

Comment 6

[Simon Fraser [:sfraser] ⌚️GMT]

It's likely a good idea to update the monitoring if it'll change in the future - assuming it's only going to be the type that changes rather than the name. Is it possible to say, at this stage?

Comment 7

[(Away)]

(In reply to Simon Fraser [:sfraser] ⌚️GMT from comment #6)
> It's likely a good idea to update the monitoring if it'll change in the
> future - assuming it's only going to be the type that changes rather than
> the name. Is it possible to say, at this stage?

That sounds reasonable. I personally have no plans to change the name.

Comment 8

[Hal Wine [:hwine] use NI!]

Note: IT also runs a similar HPKP check against release -- see bug 1336299. I don't know if this is the same plugin or not.

Comment 9

[:Atoll]

(In reply to Hal Wine [:hwine] (use NI) from comment #8)
> Note: IT also runs a similar HPKP check against release -- see bug 1336299.
> I don't know if this is the same plugin or not.

Yep, it is.

Comment 10

[Simon Fraser [:sfraser] ⌚️GMT]

The MOC have now applied the patch, so a change to this type (but not variable name) won't break the monitoring

Comment 11

[(Away)]

Just a quick status note: I've re-landed bug 1335294 and didn't touch kPreloadPKPinsExpirationTime this time around.

Comment 12

[Chris AtLee [:catlee]]

Comment on attachment 8832554 [details]
proposed fix, before I put in a PR to sysadmins/puppet

clearing dangling review request