Closed Bug 562475 Opened 15 years ago Closed 14 years ago

Bugzilla should use strict-transport-security (STS) headers

Categories

(Bugzilla :: Bugzilla-General, enhancement, P2)

enhancement

Tracking

()

RESOLVED FIXED
Bugzilla 4.0

People

(Reporter: clyon, Assigned: reed)

References

Details

(Whiteboard: [infrasecq2][wanted-bmo])

Attachments

(1 file, 1 obsolete file)

Bugzilla content that is served over https should use STS headers on https responses for better security. Reference: http://lists.w3.org/Archives/Public/www-archive/2009Sep/att-0051/draft-hodges-strict-transport-sec-05.plain.html
Severity: major → enhancement
We've discussed this before, and I seem to recall that aspects of the policy were inappropriate for Bugzilla in general, though it may be a reasonable customization or Advanced parameter. Looking over it again, I can see that one problem is that it denies self-signed certificates, which many Bugzilla installations have.
There actually won't be a problem with self signed certs since the browser requires a fully validated certificate chain and an negotiated HTTPS session with 0 errors before the STS header is considered. So in the scenario of a self-signed cert the browser would detect the self signed cert and then ignore the STS header. Aside from the above issue, can't we just add this feature as a configurable preference? If someone wants to take advantage of STS then they can enable the feature.
(In reply to comment #2) > There actually won't be a problem with self signed certs since the browser > requires a fully validated certificate chain and an negotiated HTTPS session > with 0 errors before the STS header is considered. So in the scenario of a > self-signed cert the browser would detect the self signed cert and then ignore > the STS header. Okay. Could you point me to the part of the spec that says that? > Aside from the above issue, can't we just add this feature as a configurable > preference? If someone wants to take advantage of STS then they can enable the > feature. That's what I said in comment 1.
(In reply to comment #3) > (In reply to comment #2) > > There actually won't be a problem with self signed certs since the browser > > requires a fully validated certificate chain and an negotiated HTTPS session > > with 0 errors before the STS header is considered. So in the scenario of a > > self-signed cert the browser would detect the self signed cert and then ignore > > the STS header. > > Okay. Could you point me to the part of the spec that says that? Section 7.1 If an HTTP response, received over a secure transport, includes a Strict-Transport-Security HTTP Response Header field and there are no underlying secure transport errors, the UA must:
Hmm, the spec should probably be clarified there to indicate that certificate errors are underlying transport errors (as is made clear in the other places in the document "errors" is used). However, if your interpretation is indeed correct, then I wouldn't see any harm in Bugzilla always sending these headers if the "ssl" option is on and we're in an https connection.
Priority: -- → P2
Target Milestone: --- → Bugzilla 3.8
I was talking with Sid yesterday, who's implementing STS in firefox and that's how he explained it to me. Any SSL related errors results in the STS being ignored by the browser. Agreed, the spec could be more clear.
Whiteboard: [infrasecq2] → [infrasecq2][wanted-bmo]
Attached patch patch - v1 (obsolete) (deleted) — Splinter Review
Something like this? Untested, but I'll test it soon. Just wanted to see if I'm doing the right thing.
Assignee: general → reed
Status: NEW → ASSIGNED
Attachment #453564 - Flags: review?(mkanat)
Basic testing complete, and it works. I'll do some more testing later.
Comment on attachment 453564 [details] [diff] [review] patch - v1 >+ # Note: 2628743 seconds is equal to one month. >+ if ($self->https && Bugzilla->params->{'ssl_redirect'}) { >+ unshift(@_, '-strict-transport-security' => 'max-age=2628743'); Actually, it should be 2629744, as per http://www.google.com/search?hl=en&q=number+of+seconds+in+a+month.
Comment on attachment 453564 [details] [diff] [review] patch - v1 I think a month is too long--there's a chance that a site could legitimately downgrade to a self-signed cert (perhaps they couldn't afford a new signed cert?) and then users would be locked out for a month. It we set this to a day, do we lose most of its value?
Attachment #453564 - Flags: review?(mkanat) → review-
> It we set this to a day, do we lose most of its value? A day? yes. The point is that the browser needs to remember for longer than the gap between most users' visits. I suppose you could try to say only "active" BMO users have something valuable to lose, but even I go on vacation sometimes and lots of people fail to work weekends (laggards!). The spec (or at least an early version of it) recommends that user agents have some way of manually forgetting the state for just this kind of purpose. Firefox's implementation definitely will.
Okay, in that case, put the max-age value into a constant, so people can change it if they really want to. It should still be shorter than a month, though. The thing is, you're putting this into Bugzilla by default, and it's a thing that very few people know about or understand yet. So this will affect every installation using SSL, and although you and I may know that Firefox will have a UI to clear the STS memory, no other users will know that, and neither will the site admins, since they didn't explicitly decide to start sending STS headers--we made that decision for them.
> (perhaps they couldn't afford a new signed cert?) and then > users would be locked out for a month. Then they're doing it wrong. http://cert.startcom.org/ Or get a 30-day-free trial cert from one of the other guys to fill the gap. But really there doesn't need to be a gap. You can turn down the max-age and it will replace the previous data. So if you know your cert is expiring and you're not going to replace it start sending a max-age of 1 for the last month and problem's solved. If you don't have a full month notice at least you'll catch most people. And then there's always the override in the client. I kind of agree with your r- for generic bugzilla now that I look at the patch. Hardcoded is fine for BMO but generically it should be a tunable config setting for admins to tweak.
Attached patch patch - v2 (deleted) — Splinter Review
Attachment #453564 - Attachment is obsolete: true
Attachment #454201 - Flags: review?(mkanat)
Comment on attachment 454201 [details] [diff] [review] patch - v2 Okay, this looks good. I hope that the UA error messages are clear, here, when this fails.
Attachment #454201 - Flags: review?(mkanat) → review+
Flags: approval+
Keywords: relnote
Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/ modified Bugzilla/CGI.pm modified Bugzilla/Constants.pm Committed revision 7253.
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Blocks: 574173
Blocks: 575097
Added to the release notes in bug 604256.
Keywords: relnote
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: