A user should be able to easily revoke a certificate they provided an exception for. I'm not even sure how to do this with Firefox's current UI. We should provide a revoke button in control center. Is there a way to do this from frontend code, or do we also need some platform support? +++ This bug was initially created as a clone of Bug #1201437 +++

Depending on the platform work needed here, this might be a good P2 candidate. David, can you tell us if platform has a revoke method that can be called by frontend (or added to an idl and then called)? Thanks!

The current flow to remove a certificate override is about:preferences -> Advanced -> Certificates -> View Certificates -> Servers -> (select the override to delete) -> Delete. nsICertOverrideService.clearValidityOverride(host, port) will remove the override for the given (host, port) pair.

(In reply to David Keeler [:keeler] (use needinfo?) from comment #2) > The current flow to remove a certificate override is about:preferences -> > Advanced -> Certificates -> View Certificates -> Servers -> (select the > override to delete) -> Delete. > nsICertOverrideService.clearValidityOverride(host, port) will remove the > override for the given (host, port) pair. Excellent. Thanks David! Given the complexity to remove a cert, I think this revoke button would definitely improve the user experience.

If we can revoke certificate on the Control Center, then there’s no need to use a separate persistent yellow flag. The behaviour could then be quite simple: * When user clicks on “(Unsafe) Go to domain.com”, permanent exception is automatically added. * When domain.com is opened right after adding an exception, the Control Center panel pops up * What should be shown in this panel is something like “Connection is Not Secure”, along with a “Revoke” button underneath * If user wants to revoke, the panel is already open and it can be done in one click * If user doesn’t want to revoke, the panel can be dismissed easily Is this what you’re envisioning?

(In reply to Bram Pitoyo [:bram] from comment #4) > If we can revoke certificate on the Control Center, then there’s no need to > use a separate persistent yellow flag. > > The behaviour could then be quite simple: > > * When user clicks on “(Unsafe) Go to domain.com”, permanent exception is > automatically added. > * When domain.com is opened right after adding an exception, the Control > Center panel pops up > * What should be shown in this panel is something like “Connection is Not > Secure”, along with a “Revoke” button underneath > * If user wants to revoke, the panel is already open and it can be done in > one click > * If user doesn’t want to revoke, the panel can be dismissed easily > > Is this what you’re envisioning? Hi Bram, I think this is different than the persistent yellow flag (I assume you are talking about a persistent yellow notification bar?). This is for all cases where you bypass a TLS certificate or connection error. Where as the yellow notification bar was meant for a specifc situation, right? What situation was it for? Do you link to the mocks here? This bug might solve the problem you were trying to solve with the notification bar. I am invisioning a lock with a yellow triangle or a lock with a strikethrough in the url bar. Then when users click on it, they can open the subpanel in control center to see a sentence about what's going on ("You have added a security exception for this site" is already in the control center) and a button to revoke the exception right under it.

This patch adds a simple "Revoke" button above the "More Information" button in the control center. I wasn't sure of the exact interaction behavior we want - with this patch, the cert override is cleared and the page is reloaded. I figured this was fine, since if the button was clicked by mistake the user is immediately presented with a chance to re-add the exception once the page is reloaded. Do we want an access key for the button? BTW, this depends on the "secure-user-override" connection attribute value added in bug 1201437.

(In reply to Nihanth Subramanya [:nhnt11] from comment #6) > I wasn't sure of the exact interaction behavior we want - with this patch, > the cert override is cleared and the page is reloaded. I figured this was > fine, since if the button was clicked by mistake the user is immediately > presented with a chance to re-add the exception once the page is reloaded. This sounds good to me. Panos? Note that I tested the button at <https://self-signed.badssl.com/>, and just reloading the page doesn't show the certificate error page again. SHIFT-reload works, though, so we might want to do this type of reload when the button is used. > Do we want an access key for the button? As far as I can tell, the common practice is to add access key to all buttons.

Comment on attachment 8716835 [details] [diff] [review] Patch Review of attachment 8716835 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/browser.js @@ +6705,5 @@ > > + revokeCertException() { > + let host = this._uri.host; > + let port = this._uri.port > 0 ? this._uri.port : 443; > + this._overrideService.clearValidityOverride(host, port); I guess the above .host and .port accessors could fail in rare cases? Just add a comment to that effect, noting that any failure would be reported to the error console and the panel would not close. ::: browser/locales/en-US/chrome/browser/browser.dtd @@ +774,5 @@ > <!ENTITY identity.disableMixedContentBlocking.label "Disable protection for now"> > <!ENTITY identity.disableMixedContentBlocking.accesskey "D"> > <!ENTITY identity.learnMore "Learn More"> > > +<!ENTITY identity.revokeCertException.label "Revoke"> Maybe "Remove exception"? (input from the UX team is welcome)

(In reply to :Paolo Amadini from comment #7) > (In reply to Nihanth Subramanya [:nhnt11] from comment #6) > > I wasn't sure of the exact interaction behavior we want - with this patch, > > the cert override is cleared and the page is reloaded. I figured this was > > fine, since if the button was clicked by mistake the user is immediately > > presented with a chance to re-add the exception once the page is reloaded. > > This sounds good to me. Panos? Panos mentioned we do this for the RC4 case as well.

Nihanth, can you provide a screenshot of the Revoke button?

Here's a screenshot with the current patch (text is still "Revoke")

(In reply to Nihanth Subramanya [:nhnt11] from comment #11) > Created attachment 8717584 [details] > Screenshot > > Here's a screenshot with the current patch (text is still "Revoke") Thanks Nihanth! How about "Revoke exception"? Bram, can you take a look at the screenshot and advise on the text in the button?

I know there are some issues with the More Information button - https://bugzilla.mozilla.org/show_bug.cgi?id=1202280. Particularly when it is along side the mixed content "disable protection" button (see this example: https://people.mozilla.com/~tvyas/mixedcontent.html). Aislinn would like the more info button to look like - https://www.dropbox.com/s/z7n0lda2cr5zaci/Screenshot%202015-08-31%2016.32.09.png?dl=0. In this bug, we just need to be careful about how the different buttons look together. I've asked April to see if she can make a page on badssl that requires a cert override and has mixed active content.

From searching through our support forum, I’ve found some phrases that our users have used to describe this concept: * Undo security exception * Delete certificate exception * Remove exception The button on the sec error page says “Add exception”, it seems apt to say “Delete” or “Remove”: the opposite of “Add”. And in our in-content preferences, we use the word “Remove” instead of delete. So I think we should go with something like “Remove exception”. What do you think?

(In reply to Tanvi Vyas [:tanvi] from comment #13) > In this bug, we just need to be careful about how the different > buttons look together. I think that it’s alright for two button styles to coexist: revoke should be positioned relative to the content above it, and more information is always positioned absolutely on the bottom of the panel (with a special style that Aislinn had indicated).

I would generally prefer 'Remove' over 'Delete' or 'Revoke', since it's less jargon-y, but perhaps Tanvi has strong feeling about it?

"Remove exception" sounds good!

Changed it to "Remove Exception", added an accesskey, and moved the button directly below the description containing the verifier. (In reply to :Paolo Amadini from comment #8) > Comment on attachment 8716835 [details] [diff] [review] > Patch > > Review of attachment 8716835 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: browser/base/content/browser.js > @@ +6705,5 @@ > > > > + revokeCertException() { > > + let host = this._uri.host; > > + let port = this._uri.port > 0 ? this._uri.port : 443; > > + this._overrideService.clearValidityOverride(host, port); > > I guess the above .host and .port accessors could fail in rare cases? Just > add a comment to that effect, noting that any failure would be reported to > the error console and the panel would not close. Good point. I added a Cu.reportError and an early return, I figured that's more graceful than passing an undefined variable to a service.

I included a screenshot with the mixed active content UI forced to be visible, just for an idea of how it'll look. Sorry for the double comment, I hit the submit button on the attachment too early.

(In reply to Nihanth Subramanya [:nhnt11] from comment #20) > I included a screenshot with the mixed active content UI forced to be > visible, just for an idea of how it'll look. > > Sorry for the double comment, I hit the submit button on the attachment too > early. Nice!

Comment on attachment 8717692 [details] [diff] [review] Patch v2 (In reply to Tanvi Vyas [:tanvi] from comment #17) > "Remove exception" sounds good! Glad I guessed right in comment 8 ;-) Ninanth, before landing this patch you should address the issue in comment 7 about the reload not being effective. You can try BrowserReloadSkipCache(), but please test to see if it actually solve the issue.

Oops, forgot about that because I didn't have any issues with the site I was using to test. It was reproducible for https://expired.badssl.com though, and I can confirm that BrowserReloadSkipCache() works.

(In reply to Nihanth Subramanya [:nhnt11] from comment #20) > I included a screenshot with the mixed active content UI forced to be > visible, just for an idea of how it'll look. Looks good. Once the “More Information” button is placed on the footer area, we won’t have to worry about having too many buttons that look alike.

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4b946be33468&selectedJob=17341228 I don't see any bustage related to this.

Verified fixed FX 47.0a1 (2016-03-06) Win 7, Ubuntu 14.04, OS X 10.10.5.