110161 - (ocspdefault) enable OCSP by default

Status: RESOLVED FIXED

(Core Graveyard :: Security: UI, defect, P2)

Description

[John G. Myers]

The default for security.OCSP.enabled is currently 0, which disables OCSP.  It
should be 1, which enables it for certificates that specify an OCSP service URL.

Comment 1

[Stephane Saux]

this bug must block on an NSS bug. If the ocsp service is down, the browser will
hang for a fixed 30 second until a timeout is reached.
If the ocsp server does respond but fails to send any data, the browser will
hang indefinitely.

Comment 2

[John G. Myers]

Please list blocking bugs in the appropriate bugzilla field.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

*** Bug 110166 has been marked as a duplicate of this bug. ***

Comment 4

[John G. Myers]

Mass reassign ssaux bugs to nobody

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Mass change "Future" target milestone to "--" on bugs that now are assigned to
nobody.  Those targets reflected the prioritization of past PSM management.
Many of these should be marked invalid or wontfix, I think.

Comment 6

[Wan-Teh Chang]

*** Bug 299150 has been marked as a duplicate of this bug. ***

Comment 7

[Ken Johanson]

In addition to default-checking OCSP status, the same should probably apply to CRL anytime a CRL-URL is presented. This is reportedly going to be a default behavior in ie7, and is closer to NIST preferred default behavior (that of 'always check').

Defaulting OCSP and CRL (albeit expensive) to 'on' are critical given that keypair theft and ID forgery are an emerging strategy of phishers. -That's with full fledged CAs, as well as marginally secure intermediate and adhoc root CAs. 

The revocation check is *also* essential for software signing and client certs, such as in the case of 'former' employees of an organization. The default 1-2 years expiry (method-of revocation) of a typical cert has harmed my attempts to deploy x509 (organizational mis-trust in the system).

Comment 8

[vwm]

I encountered similar problems with CAcert.org certificates. As soon as I turn on OCSP (using the OCSP-Service-URL), Firefox (1.5.0.3) returns an -8048 for every https site I try to enter that is based on a CAcert.org certificate.
Example Error: 
“Alert” 
“Error establishing an encrypted connection to secure.cacert.org. Error Code: -8048.”

Moreover Thunderbird (1.5.0.2 (20060308)) refuses to Sign my mails usind CAcert Certificates as long as OCSP is turned on. 

When I look at my personal certificates using the Certificate Manager it says 
“Could not verify this certificate for unknown reasons.” (Firefox)
“Dieses Zertifikat konnte aus unbekannten Gründen nicht verifiziert werden.”(Thunderbird. German Error Message with the same meaning as the above stated English one.)
Note that there is no such message if I lock at the authorities’ certificates “CAcert Class 3 Root” and “CA Cert Signing Authority”.

Even if this is a CAcert Bug, the error messages should be improved.

Comment 9

[vwm]

Sorry, my last posting (Comment #8) was an error. It is for Bug 335874.

Comment 10

[Chandrasekar Kannan]

changing target

Comment 11

[Robert Relyea]

Adding Kai to this bug. I think some of these 'depends on' bugs may have been fixed in the work Kai is doing to turn OCSP on in FF.

Comment 12

[Kai Engert (:KaiE:)]

Hopefully we will soon have an OCSP cache in NSS, see bug 205406.

In addition, a failure in reaching an OCSP responder should not cause a hard SSL connection failure. We'll also implement a "graceful failure" mode within NSS, and have Mozilla use that mode.

Once these changes are made available in NSS, I am proposing to land the code attached to this bug, which contains the following changes:

- in Firefox/Thunderbird and SeaMonkey OCSP preferences, introduce a new checkbox "Require OCSP is available". The checkbox is OFF by default. Some end users in critical environments might prefer the current hard failure behaviour.

- no longer skip OCSP when displaying certs in cert manager.

- reduce the number of retries on a networking failure. When OCSP is enabled, our OCSP networking code currently retries up 5 times, when OCSP networking fails. This was done as an interims measure to circumvent the hard failure mode. Now that we have the graceful failure mode, 2 retries should be sufficient.

- I incorporated the patches from bug 367878 into this one, as I require them for testing this patch.

and finally

- enable OCSP by default

Comment 13

[Kai Engert (:KaiE:)]

Attachment: Patch v1

Comment 14

[Kai Engert (:KaiE:)]

I forgot to explain why this patch is necessary to make use of the OCSP cache.
Without the patch, each time we open cert manager, the OCSP options will get changed, and that will clear the cache.
(Anyway, it isn't really a good idea to switch OCSP for displaying cert manager, because this could also influence OCSP activity that happens on a different thread.)

Comment 15

[Kai Engert (:KaiE:)]

Attachment: Patch v2

The new OCSP cache in NSS is now available in Mozilla trunk.
The new OCSP graceful failure mode has been made available in Mozilla trunk.

I think we are ready to enable OCSP by default.

As explained in comment 12 and comment 14, I'm recommending this patch to turn it on, switch to the graceful failure mode by default, and add the checkbox to the validation prefs UI in order to control the graceful/strict failure mode.

I'd like to ask Bob for code review.

I'd like to ask Gerv for his opinion and/or approve this change in application behaviour.

Comment 16

[Kai Engert (:KaiE:)]

Attachment: firefox prefs checkbox

This screenshot illustrates the change I'm proposing for the validation prefs.

The path to reach this existing dialog is:
- firefox
- edit prefs / tools options
- advanced
- encryption
- verification

New: The checkbox which says "Require the OCSP responder is available"
I'd be glad to hear proposals for better wordings.

(Nelson, I think you had proposed the wording of the checkbox should make it clear, the "requirement" is limited to those situations, where we actually attempt OCSP, only if the cert contains an OCSP URL.)

Comment 17

[Kai Engert (:KaiE:)]

Attachment: seamonkey prefs checkbox

For completeness, the patch for SeaMonkey's prefs.

Comment 18

[Kai Engert (:KaiE:)]

The patch in this bug will also fix bug 151271: Allow browsing sites after OCSP validation failure.

The patch in this bug will also fix bug 340548: Unable to complete wireless network connection if OCSP is turned on by default.

Comment 19

[Kaspar]

Attachment: firefox prefs checkbox - another suggestion

(In reply to comment #16)
> New: The checkbox which says "Require the OCSP responder is available"
> I'd be glad to hear proposals for better wordings.

What about rearranging the OCSP prefs somewhat, as shown in this mockup? Indenting and enabling/disabling the appropriate radio buttons and check boxes would also make the settings easier to understand for the user, IMO.

Second, another point: what about letting the user decide on a case-by-case basis if he wants to ignore an invalid/missing OCSP response? (similar to serverCertExpired.xul, domainMismatch.xul etc.)

Comment 20

[Johnathan Nightingale [:johnath]]

(In reply to comment #19)
> Created an attachment (id=264226) [details]
> firefox prefs checkbox - another suggestion
> 
> What about rearranging the OCSP prefs somewhat, as shown in this mockup?
> Indenting and enabling/disabling the appropriate radio buttons and check boxes
> would also make the settings easier to understand for the user, IMO.

I like the re-arranged UI and updated strings.  Some of the language takes technical skill for granted but this is also a dialog in the nether-regions of the advanced encryption prefs, so we can get away with assuming a little more. 

I would suggest changing the intro text to something like:

Firefox can use the Online Certificate Status Protocol (OCSP) to verify certificates.  When enabled, Firefox will contact the specified OCSP responder to confirm the validity of each certificate, and optionally refuse to use certificates that can't be verified.

Note addition of the word "the" before "Online" and the changed second sentence.  My sense is that the old second sentence wasn't adding much value, and missed an opportunity to help users (the three of them who make it this far :) understand a little more about the implications of their decision.

I would also suggest changing the word from "in _the_ certificate (if present)" to "in _each_ certificate (if present)" lest we mislead into thinking there is some "master OCSP cert" somewhere, or something.

I like the new checkbox text.

> Second, another point: what about letting the user decide on a case-by-case
> basis if he wants to ignore an invalid/missing OCSP response? (similar to
> serverCertExpired.xul, domainMismatch.xul etc.)

I would resist this, at least in the context of this patch.  The decisions we have users make in domain mismatch and cert expiry cases are already too technical.  My preference would actually be to stop asking users those questions (treating them as errors, e.g.) rather than start asking them new ones.  If this change lands and you feel this behaviour should be added, it's probably worth tracking in its own bug instead?

Comment 21

[Nelson Bolyard (seldom reads bugmail)]

In comment 20, Johnathan Nightingale wrote:

> My preference would actually be to stop asking users those questions 
> (treating them as errors, e.g.) rather than start asking them new ones. 

Johnathan, I am SO delighted to read that.  I agree with you completely,
as do (IINM) most of the folks who work on NSS & PSM.  Please feel free 
to further promote that view !  :)

Comment 22

[Ken Johanson]

I would lean toward simplest possible terms so that laymen are not afraid to use the safe setting (especially in case it will not be the default in all distros)

Another variation perhaps:

If you enable this preference and visit websites who's certificate vendors provide the OCSP service (Online Certificate Status Protocol), Firefox will perform frequent validation of the website's certificate (identity). If the website is known to have a recently stolen or fraudulent identity, Firefox will warn you notify you before connecting to that website.

Comment 23

[Kai Engert (:KaiE:)]

(In reply to comment #22)
> If you enable this preference and visit websites who's certificate vendors
> provide the OCSP service (Online Certificate Status Protocol), Firefox will
> perform frequent validation of the website's certificate (identity). If the
> website is known to have a recently stolen or fraudulent identity, Firefox will
> warn you notify you before connecting to that website.

We never "notify before connecting".
We either:
- notify and not connect
or
- not notify and connect

Comment 24

[Kai Engert (:KaiE:)]

I like the new wording.

In addition to the new "require" checkbox, you also proposed to change the 
  current "3 radioboxes" 
to 
  "1 checkbox + 2 radioboxes"

I think this is a reasonable change, although it caused me a couple of hours extra work.

Because the radio button values is directly mapped to the preference string, and because I prefer not to redefine the preference values, I had to implement extra UI magic to synchronize the pref value with the separate checkbox to enable/disable OCSP.

Because I also still care for SeaMonkey, I had to do this in 2 dialog imlementations.

I am attaching a new patch which reflects the wordings proposed by Kaspar and Johnathan. I'll also attach 6 screenshots to illustrate the change. 3 for Firefox, 3 for SeaMonkey.

Please let me know what you think.

Comment 25

[Kai Engert (:KaiE:)]

Attachment: Patch v3

Comment 26

[Kai Engert (:KaiE:)]

Attachment: old screenshot: firefox ocsp prefs

Comment 27

[Kai Engert (:KaiE:)]

Attachment: new screenshot: firefox ocsp prefs, ocsp disabled

Comment 28

[Kai Engert (:KaiE:)]

Attachment: new screenshot: firefox ocsp prefs, ocsp enabled

Comment 29

[Kai Engert (:KaiE:)]

Attachment: old screenshot: seamonkey ocsp prefs

Comment 30

[Kai Engert (:KaiE:)]

Attachment: new screenshot: seamonkey ocsp prefs, ocsp disabled

Comment 31

[Kai Engert (:KaiE:)]

Attachment: new screenshot: seamonkey ocsp prefs, ocsp enabled

Comment 32

[Nelson Bolyard (seldom reads bugmail)]

The new phrase 
"Only connect to sites for which a valid OCSP response has been received"
seems to preclude sites whose certs don't specify an OCSP responder. 

I really don't want to make this UI any more verbose than it is, but I'd
prefer some wording that clarified that this only applies to sites where
an OCSP request is sent.  I've been trying to think of a concise wording.
Here's the best I have so far.  It's opposite of the current meaning.

"Don't connect to sites for which no valid OCSP response is received when 
one is requested"

Comment 33

[Kai Engert (:KaiE:)]

Attachment: Patch v4

We need the changes in this bug to prepare for enabling OCSP. But it might take a little bit longer until everybody is happy with the plan to do the switch.

So, here is a new patch, which does all of the above, but does not yet enable OCSP by default. I think it is safe to land this patch now. And I would like to land ASAP.

(After landing, I would like to make a call for testers to enable OCSP.)

Comment 34

[Kai Engert (:KaiE:)]

Attachment: Separate patch to switch the pref to enabled [checked in]

Comment 35

[Kaspar]

(In reply to comment #20)
> > Second, another point: what about letting the user decide on a case-by-case
> > basis if he wants to ignore an invalid/missing OCSP response? (similar to
> > serverCertExpired.xul, domainMismatch.xul etc.)
> 
> I would resist this, at least in the context of this patch.  The decisions we
> have users make in domain mismatch and cert expiry cases are already too
> technical.

(In reply to comment #21)
> > My preference would actually be to stop asking users those questions 
> > (treating them as errors, e.g.) rather than start asking them new ones. 
> 
> Johnathan, I am SO delighted to read that.  I agree with you completely,
> as do (IINM) most of the folks who work on NSS & PSM.  Please feel free 
> to further promote that view !  :)

I agree that the default behavior should just be to stop with a failure. However, I believe that the user should have some sort of overruling his general decision (set by the prefs). E.g., if he uses strict OCSP validation and receives a corrupt response for one particular site, he shouldn't have to go to the prefs to disable strict mode completely. In the other case (loose OCSP validation mode), it would be nice to have an option (off by default) which would say "Warn about invalid OCSP responses"... otherwise the user might just be lulled into a false sense of security ("I've enabled OCSP, so every cert has been validated, hasn't it?").

Maybe this would be the subject for another RFE, and maybe more aimed at power users... let me know if I should open a new bug for this.

Comment 36

[Kai Engert (:KaiE:)]

(In reply to comment #32)
> I'd
> prefer some wording that clarified that this only applies to sites where
> an OCSP request is sent.  I've been trying to think of a concise wording.
> Here's the best I have so far.  It's opposite of the current meaning.
> 
> "Don't connect to sites for which no valid OCSP response is received when 
> one is requested"

(If we could find a wording that keeps the current meaning of "require response if attempted" it would simplify my life, by not having to change the patch logic.)

I wonder if your proposed wording "valid response" might be confusing to some users. What happens if they disable the checkbox? They might think we allow invalid repsonses? Just a thought.

I think this checkbox is really about servers not responding. So we could word around the connection requirement.

I just realized, in addition to "secure sites" this is also about other uses of certificates, like in email certs! As this dialog is shared amongst apps, we should find a general wording.

I suggest we talk about "treating certificates as valid" and not about connections to sites.

The checkbox could also talk about "ignoring failures in OCSP responders".

Finally we need to be careful in our wording, because we are caching OCSP responses. In the strict mode it will be sufficient if we have a cached recent repsonse.

New proposals:


Attempts to obtain an OCSP response must succeed.

or

When OCSP is enabled for a certificate and <Firefox> fails to use OCSP, treat the certificate as invalid.

Comment 37

[Kaspar]

(In reply to comment #24)
> I think this is a reasonable change, although it caused me a couple of hours
> extra work.
> 
> Because the radio button values is directly mapped to the preference string,
> and because I prefer not to redefine the preference values, I had to implement
> extra UI magic to synchronize the pref value with the separate checkbox to
> enable/disable OCSP.

Sorry about that... ;-) But I think it's worth the effort - thanks!

I have yet another idea about rearranging the cert validation prefs somewhat... currently, "Revocation Lists" (the CRL manager) and "Validation" (the OCSP settings) are two completely separate dialogs in Firefox/Thunderbird. But in essence, they refer to the same thing - would it be feasible to combine the (new) OCSP prefs and the CRL manager into one dialog? Maybe again the topic for a separate bug...

> Please let me know what you think.

Just one minor thing: I would change ocspDialog.title to "Certificate validation" (currently it's "Verification" only, and I changed it to "Certificate verification" in my previous proposal - but validation is probably better).

(In reply to comment #36)
> New proposals:
> 
> 
> Attempts to obtain an OCSP response must succeed.
> 
> or
> 
> When OCSP is enabled for a certificate and <Firefox> fails to use OCSP, treat
> the certificate as invalid.

I prefer the latter.

BTW, what happens in "loose mode" if a corrupt/incorrect response is received (as opposed to an OCSP server not responding)? Are transport failures (network down etc.) and protocol failures (wrong OCSP signer, corrupt signature, ...) treated the same or differently?

Comment 38

[Kai Engert (:KaiE:)]

(In reply to comment #20)
> I would also suggest changing the word from "in _the_ certificate (if present)"
> to "in _each_ certificate (if present)" lest we mislead into thinking there is
> some "master OCSP cert" somewhere, or something.

I have been thinking about this more.
So in the patch I used the text:

- Use the OCSP responder specified in each certificate (if present)

Isn't there a contradiction if we say "in each" and "if present". For me, the wording "in each" implies, something is always present.

The original wording made it clear, that if no OCSP responder is specified in a cert, then no OCSP connection will be attempted. It would be nice to have that still obvious.

New proposal for the initial radio button:


Verify a certificate if it specifies an OCSP responder



In addition I was thinking more about the introduction text. In the new wording we say "will ... confirm the validity of each certificates".

But that's not true. When the first checkbox is selected, we will not attempt to validate each and every certificate, but only those who specify a responder. Other certs will be treated as valid without an OCSP validation attempt.

Because of that I propose to say it more carefully.

I think we should avoid to word the meaning of the settings twice (and differently), once in the prefix text, and once in the individual controls.

Because it has been proposed to use a sensitive wording for the checkbox, I propose we do not use an additional and differing wording in the prefix text.


I propose to use this prefix text:


<Firefox> can use the Online Certificate Status Protocol (OCSP) to verify certificates and contact an OCSP responder to confirm their validity.


Enough new proposal to justify a new round of screenshots. Sorry, I should have been thinking about the wordings more, before attaching screenshots.

Comment 39

[Kai Engert (:KaiE:)]

> When OCSP is enabled for a certificate and <Firefox> fails to use OCSP, treat
> the certificate as invalid.

I want to change "and" to "but". 
I want to change "enabled" to "used", so we don't confuse "enabled in cert" and "enabled in prefs", but simply talk about what we do. The introduction text talks about "using OCSP", too.
I think "fails to use OCSP" could be changed to "the OCSP connection fails".

"When OCSP is enabled for validating certificate but the OCSP connection fails, treat the certificate as invalid."


> Verify a certificate if it specifies an OCSP responder

After I looked at all the new wordings together, I don't like that we mix verify and validate that much.

"Validate a certificate if it specifies an OCSP responder"


> Always use the following OCSP responder

I feel it would be more obvious if we say:

"Validate all certificates using the following OCSP responder:"

Comment 40

[Nelson Bolyard (seldom reads bugmail)]

I really like 
> Attempts to obtain an OCSP response must succeed.
It's so concise!  

I don't like any flavor of "<Firefox> fails to use OCSP," because it 
suggests that firefox is broken, not working properly.  That only leads 
to users filing lots more useless bug reports.  

Comment 41

[Kai Engert (:KaiE:)]

(In reply to comment #37)
> I have yet another idea about rearranging the cert validation prefs somewhat...
> currently, "Revocation Lists" (the CRL manager) and "Validation" (the OCSP
> settings) are two completely separate dialogs in Firefox/Thunderbird. But in
> essence, they refer to the same thing - would it be feasible to combine the
> (new) OCSP prefs and the CRL manager into one dialog? 

Well, before Firefox there was Mozilla/SeaMonkey, and as the screenshots show, they have been combined in SeaMonkey!

I conclude, the people who invented the Firefox UI explicitly decided to separate them.


> Maybe again the topic for a separate bug...

Yes, I really agree, combining dialogs is really out of the scope of this bug. Hey, all I wanted to do was adding a checkbox! :-)


> Just one minor thing: I would change ocspDialog.title to "Certificate
> validation" (currently it's "Verification" only, and I changed it to
> "Certificate verification" in my previous proposal - but validation is probably
> better).

This sounds reasonable when you look at the dialog by itself. However, the dialog title seems to match the button used to reach the dialog. So I will refrain from doing this change now, unless I hear more support for this idea.


> BTW, what happens in "loose mode" if a corrupt/incorrect response is received
> (as opposed to an OCSP server not responding)? Are transport failures (network
> down etc.) and protocol failures (wrong OCSP signer, corrupt signature, ...)
> treated the same or differently?

As of today, in the relaxed failure mode, any failure to obtain or verify the response will be treated identically and ignored. If you want to discuss and propose a different behaviour, please file a separate bug against NSS version 3.11.7.

Comment 42

[Kai Engert (:KaiE:)]

(In reply to comment #40)
> I really like 
> > Attempts to obtain an OCSP response must succeed.
> It's so concise!  
> 
> I don't like any flavor of "<Firefox> fails to use OCSP," because it 
> suggests that firefox is broken, not working properly.  That only leads 
> to users filing lots more useless bug reports.  


Nelson, do you like this?

"When OCSP is used for validating a certificate but the OCSP connection fails, treat the certificate as invalid."

Comment 43

[Kai Engert (:KaiE:)]

Attachment: new screenshot v2: firefox ocsp prefs, ocsp enabled

Comment 44

[Kai Engert (:KaiE:)]

Attachment: new screenshot v2: seamonkey ocsp prefs, ocsp enabled

Comment 45

[Kai Engert (:KaiE:)]

Comment on attachment 264338 [details]
new screenshot v2: firefox ocsp prefs, ocsp enabled

Sorry for the bug traffic. (I propose you use "hide obsolete attachments".)

Comment 46

[Kai Engert (:KaiE:)]

Attachment: Patch v5

Patch that matches the latest screenshots.
Does not yet enable OCSP by default.

Comment 47

[Nelson Bolyard (seldom reads bugmail)]

In reply to comment 42, 
yes, I really like the wording in the latest screen shots.  Good job.

Comment 48

[Johnathan Nightingale [:johnath]]

(In reply to comment #35)
> I agree that the default behavior should just be to stop with a failure.
> However, I believe that the user should have some sort of overruling his
> general decision (set by the prefs). E.g., if he uses strict OCSP validation
> and receives a corrupt response for one particular site, he shouldn't have to
> go to the prefs to disable strict mode completely. In the other case (loose
> OCSP validation mode), it would be nice to have an option (off by default)
> which would say "Warn about invalid OCSP responses"... otherwise the user might
> just be lulled into a false sense of security ("I've enabled OCSP, so every
> cert has been validated, hasn't it?").
> 
> Maybe this would be the subject for another RFE, and maybe more aimed at power
> users... let me know if I should open a new bug for this.

I see your points on both of these, and while I think they might each be border cases that don't crop up very often in actual usage, they're worth at least discussing.  But I do think they are outside the scope of what poor Kai is trying to accomplish in this bug.  :)  

This might also be good extension fodder, since people who know it's a problem are likely, as you say, power users, who are not as opposed to the use of special-purpose tools.

Comment 49

[Gervase Markham [:gerv]]

Kai: thanks for separating out the "switch it on" patch. We can now do all the rest independently, without worrying about that problem.

The current dialog says:

"&brandShortName; can use the Online Certificate Status Protocol (OCSP) to verify certificates and contact an OCSP responder to confirm their validity."

The second part of that sentence says basically the same thing as the first part. Also, I think "responder" is unnecessary technical; people are more familiar with "server", and to all intents and purposes it's the same.

Having taken into account all the comments in this bug, I suggest the following as the wording for the entire UI:

Title: Certificate Validation

---------------------------------------------------------------------------
[ ] Use the Online Certificate Status Protocol (OCSP) to confirm the current validity of certificates

- - - - <the following controls are in a groupbox, as now>

( ) using the OCSP server specified in the certificate, if present

( ) using the following OCSP server for all certificates:
    Response Signer [            |V]
    Service URL     [              ]

[ ] When an OCSP server connection fails, treat the certificate as invalid
---------------------------------------------------------------------------

That seems to cover everything, yet is concise.

However, to stop you having to produce new patch after new patch whenever anyone makes a suggestion, please _don't_ implement this until Johnathan has signed off on it!

Gerv

Comment 50

[Kai Engert (:KaiE:)]

Gerv, thanks for your additional proposal.

I'm ok with the idea to switch "responder" to "server".

I would also be ok with your proposal for the final checkbox
"When an OCSP server connection fails, treat the certificate as invalid"
sounds good to me, if other's like to as well.


However, I have a localization concern regarding your other wording change proposal.

As the introduction text / the initial checkbox text, you are using "the first half of a sentence", and in the two alternative radio boxes, you are "adding the second part of the sentence".

I was told in the past, when localizing, a sentence might have to get completely reorganized. I'm worried that localizers might have a difficult time to follow this "two half sentences" style.

Because of that I'd recommend to use complete, individual statements for each of the controls.

Comment 51

[Gervase Markham [:gerv]]

Kai: I see your point. However, that problem only occurs when the two halves of the sentence are either side of some control, e.g.:

There are [8] widgets.

In this case, because there is no control between, localisers are free to translate the strings as full sentences if the split version doesn't work in their language. So I'd say it's not a problem.

Gerv

Comment 52

[Gervase Markham [:gerv]]

Kai asked me to explain why I think my wording is better, compared to attachment 264338 [details].

- It uses quite a lot fewer words while explaining the same concepts better.

- It uses more understandable terms, such as "server".

- It removes the redundancy in the intro paragraph, where the two halves of the sentence basically say the same thing.

So, to summarise: fewer and clearer words, both of which are good things.

Gerv

Comment 53

[Johnathan Nightingale [:johnath]]

So I think we all agree that the easy changes Gerv suggests - responder->server, for instance, are worth doing, and don't materially disrupt the consensus that led to the most recent screenshot from Kai.

As for the larger change, I had the same reaction as Kai when I saw it, namely that it would be playing with i18n fire.  I do think Gerv's presentation is cleaner (in English, at least).  And Gerv's point, that translators in a given language could just replace each string with complete sentences if required, is a good one.  

Kai - I know you've changed this thing 20 times already.  If you're still concerned about i18n, we can ask someone with translation experience to chime in here, but my own take is that if you are okay with Gerv's wording, I am as well.  

If we do take the new wording, we might want to put a translator comment into pref-validation.dtd reflecting the concern here.

Comment 54

[Gervase Markham [:gerv]]

I emailed Axel earlier today to ask him to pop in and let us know if we are creating a translation problem.

Gerv

Comment 55

[Gervase Markham [:gerv]]

If we don't want it as one single sentence, then the only change required to switch it to separate sentences is to change "using" to "Use" in both bullet points. (We don't normally put full stops on the end of pref labels, I believe.) This simplicity rather proves my point about there being no l10n problem, though.

Gerv

Comment 56

[Axel Hecht [:Pike]]

It doesn't really matter from a language point of view if there's a control or not, if you compose UI strings, you're easily falling into traps. There are assumptions here in the sense that an "additional detail" part of a sentence would come at the end, for example. I'm not sure if that's satisfied for all languages. There may be interesting side-effects for Finish and it's vocal harmony story, too.

We'd need some input from a RTL localization, too, so I'm CCing localizers from Arab, Finish, Hebrew, and Japanese. Japanese is usually touchy with composition.

I'm not 100% sure if that's a blocker to doing it, though. If the localization comments make it clear, affected localizations might be able to drop that reuse and just replace it with whole sentences. Someone should have done a proof of concept version of that in English, though, to verify that it works.

Comment 57

[Ville Pohjanheimo (inactive)]

From my (Finnish) point of view, Kai's strings are easier to translate. Gerv's version doesn't cause any major headaches either, but requires more creativity.

A suggestion: If I had my way as a localizer and as a user, I'd combine the too like this:

Title: Certificate Validation

---------------------------------------------------------------------------
[ ] Use the Online Certificate Status Protocol (OCSP) to confirm the current
validity of certificates

- - - - <the following controls are in a groupbox, as now>

( ) Validate a certificate if it specifies an OCSP server

( ) Validate all certificates using the following OCSP server:
    Response Signer [            |V]
    Service URL     [              ]

[ ] When an OCSP server connection fails, treat the certificate as invalid
---------------------------------------------------------------------------

To me this is clear, concise and has (to a non-native english speaker) less ambiguous options (with Gerv's version I wasn't sure what happens when no certificate is found in the first radio-button option).. but maybe that's just me.

Comment 58

[Kai Engert (:KaiE:)]

(In reply to comment #57)
> To me this is clear, concise and has (to a non-native english speaker) less
> ambiguous options (with Gerv's version I wasn't sure what happens when no
> certificate is found in the first radio-button option).. but maybe that's just
> me.

Yes, I had the same thought about it being ambiguous, thanks for your opinion.

Ville's combined proposal sounds very good to me, and it will not require localizer comments.

Comment 59

[Johnathan Nightingale [:johnath]]

I like this new wording as well.

Moreover, while I think clarity is important, I think the larger question of changes to our OCSP support is more important, and I think Kai's sanity is *very* important, so I humbly suggest that we get this change in with the most recently suggested wording, and any dissenters are welcome to open bugs against the new version.  :)

ui-r=me for the UI in comment 57

Comment 60

[Kai Engert (:KaiE:)]

Attachment: screenshot v3: firefox ocsp prefs

Comment 61

[Kai Engert (:KaiE:)]

Attachment: screenshot v3: seamonkey ocsp prefs

Comment 62

[Kai Engert (:KaiE:)]

Attachment: Patch v6

Comment 63

[Gervase Markham [:gerv]]

I also endorse comment #57. :-)

Gerv

Comment 64

[Daniel Veditz [:dveditz]]

Can I get a clarification of "When an OCSP server connection fails treat the certificate as invalid" in comment 57? (I think it's better w/out the comma.) How does that option relate to cached OCSP responses? From the word "connection" I would assume the OCSP cache is ignored and we must hit the server every time, and in that case what does the caching do?

Maybe we should talk about "response" rather than "connection":
  "Inability to obtain a valid OCSP response invalidates certificate"

or even simply "Enforce strict OCSP validation" and leave the definition of "strict" elsewhere. Users will never see these options until they have a problem and are directed here to fix it, those doing the directing can expound on why making the change might help them.

If you don't mind reversing the sense of the check then it could be
 "Don't invalidate certificates if unable to obtain OCSP response"
 "Skip validation if OCSP response is missing"

Comment 65

[Axel Hecht [:Pike]]

Comment on attachment 264509 [details]
screenshot v3: firefox ocsp prefs

Not that I'm the master-UI person, but it I'd expect the "Use OCSP" top checkbox to affect the rest of the dialog, and thus, for the groupbox to be indented to the same extent that the initial description is. Just 2 of sicking's groszy.

Comment 66

[Gervase Markham [:gerv]]

Dan: the caching works as you'd expect. If the OCSP response is in the cache, then there's no connection, and so there can be no failure. I don't see how you think that text implies that there's no cache.

If there was a checkbox which said "If the HTTP connection fails, show an error message", would that imply that Firefox has no web page cache?

"Inability to obtain a valid OCSP response invalidates certificate" has a double negative. As does "Don't invalidate certificates if unable to obtain OCSP response". 

I don't like "Enforce strict OCSP validation" (with strict defined elsewhere) because there's no point making a description cryptic if we can avoid doing so. I really think the comment #57 text is good, understandable, accurate English.

If you want to talk about responses, then we could say: "When no OCSP response is available, treat the certificate as invalid".

Axel is right about the indentation, I think. We should check other prefs UI and make it consistent.

Gerv

Comment 67

[Kai Engert (:KaiE:)]

(In reply to comment #66)
> Dan: the caching works as you'd expect. If the OCSP response is in the cache,
> then there's no connection, and so there can be no failure.

Correct.


> If you want to talk about responses, then we could say: "When no OCSP response
> is available, treat the certificate as invalid".

Which is also slightly different from reality. You would have to insert the word "fresh" or "recent", as in "when no recent OCSP repsonse is available, treat the certificate as invalid". We currently have a hardcoded value what NSS shall treat as recent.

So, the meaning of strict mode is really directly related to a connection.
If there is a fresh response cached, all is fine.
If there is no response cached, try connection, and if it fails, cert is invalid.
If there is a response cached, but it's no longer fresh, try connection, and if it fails, cert is invalid.


> Axel is right about the indentation, I think. We should check other prefs UI
> and make it consistent.

Let's try to tweak stuff like alignment in a separate incremental patch, because that add on patch will not change any JS logic, not change any wording, and review has already started. I have sent a commented patch to Bob by email, asking him for review. So at this point please let's not invalidate what I sent to him.

Comment 68

[Gervase Markham [:gerv]]

> Which is also slightly different from reality. You would have to insert the
> word "fresh" or "recent", as in "when no recent OCSP repsonse is available,
> treat the certificate as invalid". 

I really don't think this distinction matters to users who are considering this preference. If they understand OCSP at all, they'll know it checks the current status of the certificate - that's implied in the very purpose of it.

> We currently have a hardcoded value what NSS
> shall treat as recent.

I thought OCSP responses contained a validity period? Do we ignore that, then?

Gerv

Comment 69

[Nelson Bolyard (seldom reads bugmail)]

>> We currently have a hardcoded value what NSS shall treat as recent.
> 
> I thought OCSP responses contained a validity period? 

Not exactly.  It has a "nextUpdate" time stamp, which is the LATEST date
on which the CA will have issued a newer response.  The CA is always free
to issue a newer response before the nextUpdate time.  And the OCSP 
response doesn't "expire" on the next update time.  That is merely the
latest time at which we will begin to fetch a newer response.  

> Do we ignore that, then?

No, but we place an upper bound on the length of time NSS will wait, while it has a valid OCSP response in the cache, before it tries to fetch a newer response.  We begin to fetch a new response at the lesser time of (a) the
cert's nextUpdate date, or (b) when the currently cached response has 
reached this maximum age.  This was a response to the silly multiple-year nextUpdate periods you found.

We also place a lower bound on the time between successive tries
to fetch an OCSP response, so that we're not sending out too many requests  too rapidly and so that we don't have multiple identical requests in flight simultaneously.

The whole algorithm is rather complex, as you might imagine, and is described in some detail in Bug 205406.  

But I must say that this is ALL a COLOSSAL WASTE OF TIME if, when a cert
is revoked, we simply give the user another "press enter now to ignore
this dire security error" dialog, for which Mozilla products are now infamous in security circles.  :(

Comment 70

[Gervase Markham [:gerv]]

Kai: can we please go with your current patch and comment #57's language, just so we're not here for ever? That has UI approval from johnath. If there are indentation issues, or we want to switch to my suggestion in response to Dan in comment #66, or something else, let's do it in a follow-up.

Nelson: thanks for the clarification. As for your last comment: I hope and expect that will not be so, but one thing at a time :-)

Gerv

Comment 71

[Kai Engert (:KaiE:)]

(In reply to comment #70)
> Kai: can we please go with your current patch and comment #57's language, just
> so we're not here for ever?

Yes I agree.

> That has UI approval from johnath. If there are
> indentation issues, or we want to switch to my suggestion in response to Dan in
> comment #66, or something else, let's do it in a follow-up.

Yes I agree.

I had already tasked Bob Relyea for review by private email.
I've now marked the r? flag as well.

Comment 72

[Kai Engert (:KaiE:)]

Comment on attachment 264515 [details] [diff] [review]
Patch v6

Mike, at the end of this patch I'm changing 3 files in mozilla/toolkit/

Could you please review that subset of the patch?

(Please ignore the changes in mozilla/security)

Comment 73

[Kai Engert (:KaiE:)]

Mike, here are comments that shall make it easier for you to review this code.

Please note we already have UI review+, as shown in attachment 264509 [details].

This is for ocsp.xul and ocsp.js

You know that it is possible to map a radiobutton and its values directly to a preference value. We did that in the past to map the 3-state OCSP setting directly to a pref. (0=disabled, 1=enabled for certs with OCSP URL, 2=enabled with specified responder)

But people (in this bug) asked me to change this 3-state radio into a checkbox (enabled/disabled) plus a 2 state radio (certs with URL, or, specified responder).

But I don't want to change the pref values.

My solution: We still use a 3-state radiobox, but we hide the first one! The first one is the one to carry the value for disabled.

I use a separate checkbox. And whenever that checkbox gets unchecked, it will select the hidden radiobox. Whenever the checkbox gets checked, the radiobox is set back to one of the enabled states.

In addition I'm using another trick to have the dialog behave more user friendly.

Imagine this sequence:
- UI has "always use responder" radiobutton enabled
- user unchecks checkbox to disable ocsp
- user checks checkbox to enable ocsp

Now we have to enable one of the radioboxes, either the "only certs with URL" or "always given responder".

I think it is nice if the display goes back to the previous selection. Because of that I'm caching the previous selection using a JS variable, named cacheRadio. This caching is only while the dialog is shown.

Also, we want the dialog to "gray out" the options that currently don't make sense. When the first checkbox is disabled, we gray out everything else. If the checkbox is checked, we reenable it.

This is done using function updateUI (for Firefox/Thunderbird).

For simplicity I'm using the same function, for both clicks on the enable/disable checkbox and for the some-certs/all-certs radiobox.

But I'm using a parameter "called_by". When set to 0, the function got called at dialog init time (where we look at the pref and set the initial UI state), we also use 0 when the user clicks the checkbox to enable/disable OCSP, and we use 1 whenever the user clicked the some-certs/all-certs radiobox.

Let me also provide the changes to ocsp.xul as an "ignores whitespace" diff. I'm changing less than it appears:

@@ -53,46 +53,58 @@
            title="&ocspDialog.title;"
            style="width: &window.width; !important;">

  <prefpane id="OCSPDialogPane" onpaneload="gOCSPDialog.init();">
    <script type="application/x-javascript" src="chrome://mozapps/content/preferences/ocsp.js"/>

    <preferences>
      <preference id="security.OCSP.enabled"    name="security.OCSP.enabled"   type="int"/>
      <preference id="security.OCSP.signingCA"  name="security.OCSP.signingCA" type="string"/>
      <preference id="security.OCSP.URL"        name="security.OCSP.URL"       type="string"/>
+      <preference id="security.OCSP.require"    name="security.OCSP.require"   type="bool"/>
    </preferences>
-    <description control="securityOSCPEnabled">&validation.ocsp.description;</description>
+    <checkbox id="enableOCSPBox" label="&enableOCSP.label;" accesskey="&enableOCSP.accesskey;"
+              oncommand="gOCSPDialog._updateUI(1);"/>
+    <groupbox>
    <radiogroup id="securityOCSPEnabled" preference="security.OCSP.enabled"
-                onsyncfrompreference="return gOCSPDialog._updateUI();">
-      <radio value="0" label="&disableOCSP.label;"/>
-      <radio value="1" label="&certOCSP.label;"/>
-      <radio value="2" label="&proxyOCSP.label;"/>
+                  onsyncfrompreference="return gOCSPDialog._updateUI(0);">
+        <radio value="0" hidden="true"/>
+        <radio id="certOCSP" value="1" label="&certOCSP.label;"/>
+        <radio id="proxyOCSP" value="2" label="&proxyOCSP.label;"/>

      <grid class="indent" flex="1">
        <columns>
@@ -94,5 +97,14 @@
      </grid>
    </radiogroup>
    <separator/>
+      <checkbox id="requireWorkingOCSP" preference="security.OCSP.require"
+                label="&requireOCSP.label;"
+                accesskey="&requireOCSP.accesskey;"/>
+    </groupbox>
+    # without extra separators, the dialog buttons overlap the checkbox :-/
+    <separator/>
+    <separator/>
+    <separator/>
+    <separator/>
  </prefpane>
</prefwindow>

Comment 74

[Mike Connor [:mconnor]]

Comment on attachment 264515 [details] [diff] [review]
Patch v6

>Index: mozilla/toolkit/locales/en-US/chrome/mozapps/preferences/ocsp.dtd

accesskey tweaks:

>+<!ENTITY ocspDialog.title             "Certificate Validation">
>+<!ENTITY enableOCSP.label             "Use the Online Certificate Status Protocol (OCSP) to confirm the current validity of certificates">
>+<!ENTITY enableOCSP.accesskey         "O">

U

>+<!ENTITY certOCSP.label               "Validate a certificate if it specifies an OCSP server">
>+<!ENTITY certOCSP.accesskey           "C">

V

>+<!ENTITY proxyOCSP.label              "Validate all certificates using the following OCSP server:">
>+<!ENTITY proxyOCSP.accesskey          "A">

a (note the lowercase, accesskeys search on same case, then opposite case)

>+<!ENTITY requireOCSP.label            "When an OCSP server connection fails, treat the certificate as invalid">
>+<!ENTITY requireOCSP.accesskey        "V">

W


>Index: mozilla/toolkit/mozapps/preferences/ocsp.xul

I think the groupbox is wrong here.  Can you strip it, and add class="indent" to the radiogroup?  I think that would be more in line with other UI... r=me either way, but I think I'd like the latter better.

The separator hack looks bogus as well, is there a height style I'm missing somewhere?

Comment 75

[Kai Engert (:KaiE:)]

(In reply to comment #74)
> accesskey tweaks:

done. I realized that two radiobuttons did not have the accesskeys listed in xul. fixed.


> >Index: mozilla/toolkit/mozapps/preferences/ocsp.xul
> 
> I think the groupbox is wrong here.  Can you strip it, and add class="indent"
> to the radiogroup?

Done. This only indents the groupbox, but not the checkbox at the bottom of the dialog. But I guess that's fine. (I tried to use class="ident" here, too, but it was not aligned.)


> The separator hack looks bogus as well, is there a height style I'm missing
> somewhere?

Once you said that, I saw the width style specified.
I tried to remove the width style, and that indeed produced a correct layout, so I removed the separator hack.

The dialog now gets layouted very wide, the radiobutton doesn't get wrapped any more. But I guess that's ok?

I'd like to check this in now. Feel free to suggest additional tweaks to dialog layout and we can do it in an incremental patch.

Comment 76

[Kai Engert (:KaiE:)]

Attachment: Patch v7 (checked in)

For reference, I checked in this patch, which is v6 plus the minor tweaking described in the previous comment.

Comment 77

[Kai Engert (:KaiE:)]

Attachment: fix accesskeys in seamonkey UI (checked in)

This obvious patch fixes the access keys in SeaMonkey UI in the same way as done for Firefox. r=me. Thanks to IanN for catching this.

Comment 78

[Marek Stępień [:marcoos, inactive]]

Some localization-related issues with attachment 266677 [details] [diff] [review]:

In pref-validation.dtd:

-<!ENTITY certOCSP.label                           "Use OCSP to validate only certificates that specify an OCSP service URL">
-<!ENTITY certOCSP.accesskey                       "U">
-<!ENTITY proxyOCSP.label                          "Use OCSP to validate all certificates using this URL and signer:">
+<!ENTITY enableOCSP.label                         "Use the Online Certificate Status Protocol (OCSP) to confirm the current validity of certificates">
+<!ENTITY enableOCSP.accesskey                     "E">
+<!ENTITY certOCSP.label2                          "Validate a certificate if it specifies an OCSP server">
+<!ENTITY certOCSP.accesskey                       "C">

The '2' should be added to 'certOCSP' and 'enableOCSP', not to the 'label' part. Some l10n tools rely on the labels being called '*.label'.


ocsp.dtd:
The proxyOCSP and certOCSP entities include semantic changes in their content, so they should be renamed (both .labels and .accesskeys to keep them in sync).

Comment 79

[Mike Connor [:mconnor]]

is the .label thing documented somewhere? this is the first I've heard of it.

As for semantic string changes, I think I've been on branch too long, but we should do that too.

Comment 80

[Mike Connor [:mconnor]]

Comment on attachment 264322 [details] [diff] [review]
Separate patch to switch the pref to enabled [checked in]

sr=mconnor.

We should land this change in time for alpha 6.  I understand that there are some concerns from CAs about load, but I don't think that should stop us, especially as IE7 has already shipped with this as default.

Comment 81

[bill]

Agreed.  Actually, Vista (not just IE7) shipped with this setting on by default. Any additional load that we would impart should be small compared to what Vista has already established.

Comment 82

[Kai Engert (:KaiE:)]

Comment on attachment 264322 [details] [diff] [review]
Separate patch to switch the pref to enabled [checked in]

I checked in this patch to enable OCSP by default on the trunk.

(Leaving bug open for the open language issue)

Comment 83

[Kai Engert (:KaiE:)]

Attachment: Additional Patch v8: name2.label [checked in]

I hope I got this right.
I fixed name.label2 to name2.label 
and renamed some more strings to name2.label as proposed.

Can you please review?

Comment 84

[Marek Stępień [:marcoos, inactive]]

Comment on attachment 267313 [details] [diff] [review]
Additional Patch v8: name2.label [checked in]

I'm OK with this, r+. :)

Comment 85

[Kai Engert (:KaiE:)]

All patches checked in. Resolving bug: FIXED

Comment 86

[hopefox34]

In the process of upgrading to TLS 1.2, this error was encountered with some certificates. https://sinbarras.org/ This impacts:

• Any server that uses an OCSP service from a particular CA
• Django and Apache and other servers that use the mod_ssl library
• Agents able to handle OCSP requests from a browser, such as Windows Server 2012 or later versions of Windows operating system (without KB3080079) or Linux distros such as Ubuntu 16.04 or later versions of Red Hat Enterprise Linux 6.