870710 - Deal with removal of tooltip code from PSM

Status: RESOLVED FIXED

(SeaMonkey :: Security, defect)

Description

[[e-mail]]

Bug #832848 wants to remove nsISecureBrowserUI's tooltipText attribute.

Comment 1

[[e-mail]]

Attachment: Proposed patch

Comment 2

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Comment on attachment 747950 [details] [diff] [review]
Proposed patch

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

I filed bug 872761 to make Firefox's logic for this more re-usable, but I'm not sure if it will be appropriate for SeaMonkey.

::: suite/browser/nsBrowserStatusHandler.js
@@ +371,5 @@
> +                               .SSLStatus.serverCert;
> +        var issuerName = cert.issuerOrganization ||
> +                         cert.issuerCommonName || cert.issuerName;
> +        if (issuerName == "RSA Data Security, Inc.")
> +          issuerName = "Verisign, Inc.";

I would avoid doing this special name handling. The Firefox code doesn't do it, for example.

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#6517

Comment 3

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Neil, thanks for jumping on this. My understanding is that SeaMonkey bugs like this shouldn't block Platform bugs except in exceptional circumstances, so I'm replacing the blocking relationship with a "See also" relationship. In particular, I would like to make this change soon because there are other Platform bug fixes that depend on this Platform change.

Comment 4

[[e-mail]]

(In reply to Brian Smith from comment #2)
> (From update of attachment 747950 [details] [diff] [review])
> > +        if (issuerName == "RSA Data Security, Inc.")
> > +          issuerName = "Verisign, Inc.";
> 
> I would avoid doing this special name handling. The Firefox code doesn't do
> it, for example.
> 
> http://mxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#6517

Actually I was copying from here:

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/pageinfo/security.js#96

Comment 5

[Philip Chee]

> I would avoid doing this special name handling. The Firefox code doesn't do it,
> for example.

You removed the Verisign code in your patch:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c74ce1905b96#l11.57
But you never explained why. Bug 47435 might be not necessary now but I don't know. Could you ask the security team?

q.v. Bug 47435 Substitute "Verisign" for "RSA Data Security"

Comment 6

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Philip Chee from comment #5)
> > I would avoid doing this special name handling. The Firefox code doesn't do it,
> > for example.
> 
> You removed the Verisign code in your patch:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/c74ce1905b96#l11.57
> But you never explained why. Bug 47435 might be not necessary now but I
> don't know. Could you ask the security team?
> 
> q.v. Bug 47435 Substitute "Verisign" for "RSA Data Security"

I filed bug 872910 to fix the Page Info dialog box.

Either you can be consistent with Firefox's site identity block UI or you can be consistent with the page info UI. Ideally, we'd get a patch for bug 872910 and then remove the name hack from the SeaMonkey site identity UI. But, it's up to you guys as to whether to do that now in this bug or in a separate bug.

I CC'd kwilson, who is probably the only one that could find out whether there is any expectation from VeriSign as far as this naming hack goes.

Comment 7

[[e-mail]]

(In reply to Philip Chee from comment #5)
> You removed the Verisign code in your patch:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/c74ce1905b96#l11.57
He removed all of the code that generates the tooltipText attribute, not just the RSA/Verisign check...

Comment 8

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

(In reply to Kathleen Wilson from bug 872910 comment #1)
> From a representative of Symantec/VeriSign: "I agree that you can remove
> that code. The RSA root expired in 2005 and if it's still in your trust
> store you can remove it."
> 
> Also, I checked my spreadsheet of included root certs, and this root is no
> longer included, and we don't currently have expired root certs included in
> NSS.

Comment 9

[[e-mail]]

Attachment: Removed RSA hack

Comment 10

[Philip Chee]

Comment on attachment 750629 [details] [diff] [review]
Removed RSA hack

> Removed RSA hack
Could you file a followup bug to remove the RSA hack from suite/browser/pageinfo/security.js ? Thanks!

> -#nsContextMenu.js (setWallpaper confirmation dialog)
> -wallpaperConfirmTitle  = Set As Wallpaper
> -wallpaperConfirmButton = Set Wallpaper
> -wallpaperConfirmMsg    = Do you want to set this image as your desktop wallpaper?
Good catch!

Comment 11

[[e-mail]]

Pushed comm-central changeset 9b76619bdfe4.

Rather conveniently the m-c change got backed out and didn't make Gecko 23, thus we don't need a late-l10 aurora landing.