Closed Bug 886752 Opened 8 years ago Closed 7 years ago

Add more SSL/TLS connection information to "Page Info"

Categories

(Core Graveyard :: Security: UI, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla36

People

(Reporter: Makkes, Assigned: evilpie)

References

(Blocks 1 open bug, )

Details

(Whiteboard: [chrome-parity])

Attachments

(4 files, 6 obsolete files)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:21.0) Gecko/20100101 Firefox/21.0 (Beta/Release)
Build ID: 20130512194445

Steps to reproduce:

1. Go to https://bugzilla.mozilla.org
2. Click on the lock symbol in the address bar
3. Click on "More Information"


Actual results:

The encryption algorithm (in this case RC4 with 128 bit keys) is shown but not the key exchange method (RSA, ECDHE, ...).


Expected results:

The key exchange method should be show to the user either in the popup after step 2 or in the window that opens after step 3. Also other UI elements could be considered to let the user know if perfect forward secrecy is achieved.

Chromium/Chrome shows the information in the popup under "Connection".
This bug has been filed as a reaction on the "perfect forward secrecy" blog post that I just put in the URL field. It argues that more websites could choose to use ephemeral diffie hellman keys to protect against wiretapping.

The blog post also shows that Chrome allows you to see whether a website has chosen to do so (very little websites do).
Max asked me on IRC if this was a good first bug and if we could find a mentor for it.

Dveditz argued that poking into NSS and PSM is a very good indicator that this is *not* a good first bug ;) How doable is his suggestion in general? Should he maybe go for an add-on first so he gets to know the code base and can start off with a proof-of-concept, so we can defer the whole UI messyness that might come with it to a later point?

Do you have any suggestions, Brian or Camilo?
Attached patch WIP (obsolete) — Splinter Review
I am probably not going to work on this very much, but I was looking at this code, so I thought I might as well write an WIP patch. The protocol and exchange method should really be stringified somewhere in NSS and not in pageinfo.
Comment on attachment 773623 [details] [diff] [review]
WIP

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

Forgive the unsolicited drive-by feedback.
Looks good, from what I can see!

::: browser/base/content/pageinfo/security.js
@@ +83,4 @@
>          isBroken : isBroken,
>          isEV : isEV,
>          cert : null,
>          fullLocation : gWindow.location        

As long as you're here, it's really tempting to get rid of this trailing whitespace...

@@ +255,5 @@
>    }
>    else if (info.encryptionStrength >= 90) {
>      hdr = pkiBundle.getFormattedString("pageInfo_StrongEncryptionWithBits",
> +                                       [info.encryptionAlgorithm, info.encryptionStrength + "",
> +                                        info.protocolVersion, info.keyExchangeMethod]);

These lines are a bit long - can you try to get them closer to 80 chars?

@@ +263,5 @@
>    }
>    else if (info.encryptionStrength > 0) {
>      hdr  = pkiBundle.getFormattedString("pageInfo_WeakEncryptionWithBits",
> +                                        [info.encryptionAlgorithm, info.encryptionStrength + "",
> +                                         info.protocolVersion, info.keyExchangeMethod]);

(here too)

::: security/manager/locales/en-US/chrome/pippki/pippki.properties
@@ +75,5 @@
>  pageInfo_Privacy_None2=Information sent over the Internet without encryption can be seen by other people while it is in transit. 
>  pageInfo_Privacy_None3=The page you are viewing is not encrypted.
>  # LOCALIZATION NOTE (pageInfo_StrongEncryptionWithBits): %1$S is the name of the encryption standard,
>  # %2$S is the key size of the cipher.
> +pageInfo_StrongEncryptionWithBits=Connection Encrypted: High-grade Encryption (%1$S, %2$S bit keys) over %3$S exchanged with %4$S

My understanding is that we can't change the value of any localized strings. We instead must add new strings. :(

@@ +80,5 @@
>  pageInfo_Privacy_Strong1=The page you are viewing was encrypted before being transmitted over the Internet.
>  pageInfo_Privacy_Strong2=Encryption makes it very difficult for unauthorized people to view information traveling between computers. It is therefore very unlikely that anyone read this page as it traveled across the network.
>  # LOCALIZATION NOTE (pageInfo_WeakEncryptionWithBits): %1$S is the name of the encryption standard,
>  # %2$S is the key size of the cipher.
> +pageInfo_WeakEncryptionWithBits=Connection Encrypted: Low-grade Encryption (%1$S, %2$S bit keys) over %3$S exchanged with %4$S

(ditto)

::: security/manager/ssl/public/nsISSLStatus.idl
@@ +15,5 @@
>    readonly attribute string cipherName;
>    readonly attribute unsigned long keyLength;
>    readonly attribute unsigned long secretKeyLength;
>  
> +  readonly attribute unsigned long keyExchangeMethod;

If you use keaTypeName (see below), this can be a string, and you don't have to do the conversion in security.js. There might be an equivalent for protocolVersion, but I couldn't find it in the few minutes I looked.

::: security/manager/ssl/src/nsNSSCallbacks.cpp
@@ +957,5 @@
>      SSLCipherSuiteInfo cipherInfo;
>      if (SSL_GetCipherSuiteInfo(channelInfo.cipherSuite, &cipherInfo,
>                                  sizeof (cipherInfo)) == SECSuccess) {
>        // keyExchange null=0, rsa=1, dh=2, fortezza=3, ecdh=4
> +      status->mKeyExchangeMethod = cipherInfo.keaType;

How about cipherInfo.keaTypeName?
Thank you for taking a look. I don't mind at all. What do you think about the way the stuff is presented to the user?

I think keaTypeName is a horrible name. (I assume this means Key Exchange Algorithm Type Name?)

This function http://mxr.mozilla.org/mozilla-central/source/security/nss/cmd/ssltap/ssltap.c#272 could be used to translate the protocolVersion, however it seems to be more of a debug think and spews tons of information.
*thing
I meant using the keaTypeName field of the struct SSLCipherSuiteInfo (of which cipherInfo is one), which is a const char *. I assume it's a human-readable string describing the key exchange algorithm.

I think the presentation is good. This is only interesting/important/meaningful to a relatively small number of advanced users anyway.
For Chome parity we also need to show SHA1.
Assignee: nobody → evilpies
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Whiteboard: [chrome-parity]
Comment on attachment 773623 [details] [diff] [review]
WIP

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

This bug is highly related to bug 697781. See in particular bug 697781 comment 22. If we're going to change the format of HTTPS entries on disk then we should fix bug 697781 and then base this feature on top of those changes, to minimize the number of times we change the serialization format. Note that the serialization format affects the storage of the information on disk.

Note that if you store the cipher suite ID then you don't need these variables any more:

  uint32_t mKeyLength;
  uint32_t mSecretKeyLength;
  nsXPIDLCString mCipherName;

Also, I am not sure why it is important to show the key exchange method but not the rest of the cipher suite info (e.g. the authentication method) in the UI.

::: security/manager/ssl/public/nsISSLStatus.idl
@@ +15,5 @@
>    readonly attribute string cipherName;
>    readonly attribute unsigned long keyLength;
>    readonly attribute unsigned long secretKeyLength;
>  
> +  readonly attribute unsigned long keyExchangeMethod;

I suggest that you store the cipher suite ID here, and not the key exchange method. instead, calculate the key exchange method from the cipher suite lazily when it is requested.

::: security/manager/ssl/src/nsSSLStatus.cpp
@@ +132,5 @@
>  
> +  rv = stream->Read32(&mProtocolVersion);
> +  NS_ENSURE_SUCCESS(rv, rv);
> +  rv = stream->Read32(&mKeyExchangeMethod);
> +  NS_ENSURE_SUCCESS(rv, rv);

You cannot put these here, at least not without changing the version number written during the serialization.

You need to check the version number to make sure that the version number corresponds to the new version of the serialization. If it doesn't, then either we need to return an error or we need t somehow compensate for the missing information.

You do not need 32-bits to store either of these fields. Protocol versions are 16-bits on the wire, and so are cipher suite IDs. (There can't be more key exchange methods than cipher suite IDs.) So, please store only 16 bits for each.

I suggested storing the cipher suite ID above because it encodes a lot more information than just the key exchange method. If you store the cipher suite ID here then you can calculate all of those components of the cipher suite from that single 16-bit integer. That's better because we'll avoid having to change the serialization format over and over again as we want to keep track of more fields.

@@ +167,5 @@
>  
> +  rv = stream->Write32(mProtocolVersion);
> +  NS_ENSURE_SUCCESS(rv, rv);
> +  rv = stream->Write32(mKeyExchangeMethod);
> +  NS_ENSURE_SUCCESS(rv, rv);

Ditto here. You need to take verisioning into consideration and you need to write a new version number.

Also, we need to test that the previous version of Firefox does something reasonable when it encounters the new version number.
Attachment #773623 - Flags: review-
See bug 820887 comment 13. We accidentally "fixed" things insofar as the original request was concerned (showing the key exchange method in the page info dialog box) with a typo in bug 820887. So, I am morphing this to be about the stuff discussed in the IRC chat in attachment 775090 [details].
Summary: Key Exchange method of SSL/TLS connections is not exposed to user → Add more SSL/TLS connection information to "Page Info"
(In reply to Brian Smith (:briansmith), was bsmith@mozilla.com (:bsmith) from comment #11)
> See bug 820887 comment 13. We accidentally "fixed" things insofar as the
> original request was concerned (showing the key exchange method in the page
> info dialog box) with a typo in bug 820887. So, I am morphing this to be
> about the stuff discussed in the IRC chat in attachment 775090 [details].

If the full suite name is not going to be exposed in the "Page Info" window (showing the formatted example from IRC instead), could/should the suite name still be exposed to JS through another property?
Now that we are thinking about using the new cache, can we change the on disk format?
Flags: needinfo?(honzab.moz)
(In reply to Tom Schuster [:evilpie] from comment #13)
> Now that we are thinking about using the new cache, can we change the on
> disk format?

The sec info on disk format is gonna be the same.
Flags: needinfo?(honzab.moz)
Mhm sounds like we are still blocked by the on disk format. I hope somebody else feels like figuring this out.
Assignee: evilpies → nobody
Status: ASSIGNED → NEW
There are some possible solutions:

1. The SSL metadata part of the disk format of the cache is already versioned. If we are careful, we may be able to take advantage of the versioning information in the disk format of the cache to allow us to store the information we need. This would probably break the display of that information in older versions of Firefox if you used that older version to access a profile that was written/updated by the new version of Firefox, but that would be acceptable.

2. We could simply punt on storing the information into the cache, and change the code that writes the security status string to the cache entry to write "Loaded from the HTTP cache" instead of the information it currently writes, until we can update the disk format of the cache.

3. See bug 942136 comment 5. We probably should move all these details to the network tab of the developer tools, and avoid the misleading display of the data in "Page Info." This would be more precise and would help developers more. (We should remove "Page Info" completely, AFAICT.)
Component: Untriaged → Security: UI
OS: Linux → All
Product: Firefox → Core
Hardware: x86_64 → All
Version: 21 Branch → Trunk
Printing the size of parameters and the used curve for DHE and ECDHE would also be useful.
Duplicate of this bug: 936217
Duplicate of this bug: 1042144
Can we do this nowadays? I would really like to have at least the protocol version.
Attached patch v1 Change nsISSLStatus (obsolete) — Splinter Review
This adds the protcolVersion and the cipherSuite (id) to nsISSLStatus.
Assignee: nobody → evilpies
Attachment #773623 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8507128 - Flags: review?(dkeeler)
Attached patch v1 Add protocol to page info (obsolete) — Splinter Review
Attachment #8507132 - Flags: review?(dkeeler)
Attached patch v1.1 Add protocol to page info (obsolete) — Splinter Review
Attachment #8507132 - Attachment is obsolete: true
Attachment #8507132 - Flags: review?(dkeeler)
Attachment #8507226 - Flags: review?(dkeeler)
Comment on attachment 8507226 [details] [diff] [review]
v1.1 Add protocol to page info

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

Looks good to me, but you'll need a browser peer to review this.
Attachment #8507226 - Flags: review?(dkeeler) → feedback+
Comment on attachment 8507128 [details] [diff] [review]
v1 Change nsISSLStatus

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

This seems reasonable, but I'm not sure I see why having cipherSuite is useful when we already have cipherName, particularly when the patch that depends on this doesn't use it. r=me with that removed (or a strong reason for keeping it) and other comments addressed.

::: security/manager/ssl/public/nsISSLStatus.idl
@@ +15,5 @@
>    readonly attribute string cipherName;
>    readonly attribute unsigned long keyLength;
>    readonly attribute unsigned long secretKeyLength;
>  
> +  readonly attribute unsigned short cipherSuite;

We already have cipherName - is there a particular reason why having cipherSuite as well is useful?

::: security/manager/ssl/src/nsSSLStatus.cpp
@@ +62,5 @@
>  NS_IMETHODIMP
> +nsSSLStatus::GetCipherSuite(uint16_t* _result)
> +{
> +  NS_ASSERTION(_result, "non-NULL destination required");
> +  if (!mHaveKeyLengthAndCipher)

nit: braces around conditional bodies

@@ +161,5 @@
>    NS_ENSURE_SUCCESS(rv, rv);
>    rv = stream->ReadCString(mCipherName);
>    NS_ENSURE_SUCCESS(rv, rv);
>  
> +  rv = stream->Read32(&mProtocolVersion);

TRANSPORTSECURITYINFOMAGIC in TransportSecurityInfo.cpp will need to be updated as a result of this (keep the first 4 bytes the same, though)
Attachment #8507128 - Flags: review?(dkeeler) → review+
Attached patch v2 Change nsISSLStatus (obsolete) — Splinter Review
I thought ciphersuite would be useful if we wanted to add getters for all the stuff it encodes like MAC etc. But I guess you can just parse cipher name in your head.
Attachment #8507128 - Attachment is obsolete: true
Attachment #8507429 - Flags: review?(dkeeler)
Attached patch v2 Change nsISSLStatus (obsolete) — Splinter Review
Dammit forgot to refresh, see previous comment.
Attachment #8507429 - Attachment is obsolete: true
Attachment #8507429 - Flags: review?(dkeeler)
Attachment #8507430 - Flags: review?(dkeeler)
Comment on attachment 8507226 [details] [diff] [review]
v1.1 Add protocol to page info

It seems like you reviewed something here before.
Attachment #8507226 - Flags: review?(dao)
I would like to see if (perfect) forward secrecy is enabled.

Is it possible to implement such a feature?
(In reply to Ettore Atalan from comment #29)
> I would like to see if (perfect) forward secrecy is enabled.
> 
> Is it possible to implement such a feature?

PFS depends on cipher suite used, so it is already possible - just see if the cipher uses DHE or ECDHE, if it does it is.

Problem is, that especially for DHE, Firefox will accept small insecure DH parameters, like 768 bit long.


Tom, any chance to add such info to the structure? (ECDHE curve is secondary prio, since FF supports only secure curves anyway)
(In reply to Hubert Kario from comment #30)
> any chance to add such info to the structure? (ECDHE curve is secondary
> prio, since FF supports only secure curves anyway)

File a new bug for this, please. Let's just get the SSL/TLS version in there first, as that's a problem that's long needed fixing.
Comment on attachment 8507226 [details] [diff] [review]
v1.1 Add protocol to page info

>       try {
>         retval.encryptionAlgorithm = status.cipherName;
>         retval.encryptionStrength = status.secretKeyLength;
>+        var version = ["SSL 3", "TLS 1.0", "TLS 1.1", "TLS 1.2"];
>+        retval.protocolVersion = version[status.protocolVersion];

Why would this throw?
Comment on attachment 8507430 [details] [diff] [review]
v2 Change nsISSLStatus

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

Looks good. Like I said in bug 1085031, I actually think those changes and these could happen together (and, in that case, feel free to add a direct accessor to cipherSuite).

::: security/manager/ssl/public/nsISSLStatus.idl
@@ +15,5 @@
>    readonly attribute string cipherName;
>    readonly attribute unsigned long keyLength;
>    readonly attribute unsigned long secretKeyLength;
>  
> +  readonly attribute unsigned long protocolVersion;

nit: document that 0 = SSL 3.0, 1 = TLS 1.0, 2 = TLS 1.1, 3 = TLS 1.2 (actually, maybe make constants in this interface to that effect?)

@@ +22,5 @@
>    readonly attribute boolean isNotValidAtThisTime;
>  
>    /* Note: To distinguish between 
>     *         "unstrusted because missing or untrusted issuer"
>     *       and 

nit: if you feel like it, remove the unnecessary whitespace at the end of this line and the one two above

::: security/manager/ssl/src/nsSSLStatus.cpp
@@ +59,5 @@
>    return NS_OK;
>  }
>  
>  NS_IMETHODIMP
> +nsSSLStatus::GetProtocolVersion(uint32_t* _result)

nit: maybe aProtocolVersion instead of _result?

@@ +61,5 @@
>  
>  NS_IMETHODIMP
> +nsSSLStatus::GetProtocolVersion(uint32_t* _result)
> +{
> +  NS_ASSERTION(_result, "non-NULL destination required");

nit: use NS_ENSURE_ARG_POINTER like in the patch that updates the style in this file

::: security/manager/ssl/src/nsSSLStatus.h
@@ +35,5 @@
>    uint32_t mKeyLength;
>    uint32_t mSecretKeyLength;
>    nsXPIDLCString mCipherName;
>  
> +  uint32_t mProtocolVersion;

A full 4 bytes to store this seems like a waste. Maybe uint16_t?
Attachment #8507430 - Flags: review?(dkeeler) → review+
(In reply to Dave Garrett from comment #31)
> (In reply to Hubert Kario from comment #30)
> > any chance to add such info to the structure? (ECDHE curve is secondary
> > prio, since FF supports only secure curves anyway)
> 
> File a new bug for this, please. Let's just get the SSL/TLS version in there
> first, as that's a problem that's long needed fixing.

Actually I wouldn't mind adding all the needed information now, otherwise we have to invalidate version again. Sadly my knowledge on this matter is limited so I can't asses which properties would be useful.
Attached image Calomel SSL Validation
The Calomel SSL Validation [1] Add-on could be a help. It offers a lot of useful security information.

[1] https://addons.mozilla.org/de/firefox/addon/calomel-ssl-validation/
I think the only important information that we can't extract from SSLCipherSuiteInfo is the stuff from SSLChannelInfo, which includes authKeyBits and keaKeyBits.
Attachment #8507226 - Flags: review?(dao)
Attached patch v3Splinter Review
I asked on dev-security what information we should expose. Just uploading my current patch, but waiting for feedback on the list before continuing.
Attachment #8507430 - Attachment is obsolete: true
Other stuff I'd really like to see if you can add them:

1) HSTS status (no, yes, or static yes)
2) SNI status (was it negotiated and used)
3) indication that a fallback was performed
4) a full list of all negotiated extensions (though, a UI for this would be difficult)
Attachment #8508974 - Flags: review?(dkeeler)
Attached patch ssl-frontendSplinter Review
That try catch was introduced in bug 402726. I don't think that specific case can still happen, but I think figuring that out would be a good followup. protocolVersion throws in the same case in which cipherName and secretKeyLength would throw.
Attachment #8507226 - Attachment is obsolete: true
Attachment #8510488 - Flags: review?(dao)
Comment on attachment 8510488 [details] [diff] [review]
ssl-frontend

>       var retval = {
>         hostName : hName,
>         cAName : issuerName,
>         encryptionAlgorithm : undefined,
>         encryptionStrength : undefined,
>+        version: undefined,
>         isBroken : isBroken,
>         isEV : isEV,
>         cert : cert,
>         fullLocation : gWindow.location
>       };

Why are you initializing version as undefined here...

>     } else {
>       return {
>         hostName : hName,
>         cAName : "",
>         encryptionAlgorithm : "",
>         encryptionStrength : 0,
>+        version: "",
>         isBroken : isBroken,
>         isEV : isEV,
>         cert : null,

... and as an empty string here?
I am just following the code around it!
(In reply to Tom Schuster [:evilpie] from comment #41)
> I am just following the code around it!

That's not really a satisfying answer. Can you dig a little deeper to make sure you're doing the right thing here?
Comment on attachment 8510488 [details] [diff] [review]
ssl-frontend

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

If that code was better from the beginning this changes might have been clearer.

::: browser/base/content/pageinfo/security.js
@@ +63,2 @@
>        try {
>          retval.encryptionAlgorithm = status.cipherName;

In the unlikely event that those throw, they will be left as undefined.

@@ +79,5 @@
> +          retval.version = "TLS 1.1";
> +          break;
> +        case nsISSLStatus.TLS_VERSION_1_2:
> +          retval.version = "TLS 1.2"
> +          break;

If version was undefined from throwing we will fall through here and retval.version stays 'undefined'.

@@ +89,5 @@
>          hostName : hName,
>          cAName : "",
>          encryptionAlgorithm : "",
>          encryptionStrength : 0,
> +        version: "",

So this doesn't matter at all. If encryptionStrength == 0, we will host use hostName and not encryptionAlgorithm or version below. So the fields might not even exists.
Is it possible that status.protocolVersion would throw but status.cipherName and status.secretKeyLength would not such that encryptionStrength would be > 0 but version would be undefined?
No.
Attachment #8510488 - Flags: review?(dao) → review+
Comment on attachment 8508974 [details] [diff] [review]
v3

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

This looks great! I just have a few nits/comments. Also, I noticed that nsSSLStatus isn't threadsafe despite it being used on multiple threads, so I filed bug 1088275 to fix that (this doesn't need to block on that - this has been a problem for a long time, I think).

::: security/manager/ssl/public/nsISSLStatus.idl
@@ +12,4 @@
>  interface nsISSLStatus : nsISupports {
>    readonly attribute nsIX509Cert serverCert;
>  
> +  readonly attribute ACString cipherName;

Did you want to add an accessor directly for cipherSuite as well? (Since they're backed by the same data, it would pretty much be at no cost).

::: security/manager/ssl/src/nsSSLStatus.cpp
@@ +32,4 @@
>  
> +  SSLCipherSuiteInfo cipherInfo;
> +  if (SSL_GetCipherSuiteInfo(mCipherSuite, &cipherInfo,
> +                             sizeof cipherInfo) != SECSuccess) {

nit: "sizeof(cipherInfo)"

@@ +50,4 @@
>  
> +  SSLCipherSuiteInfo cipherInfo;
> +  if (SSL_GetCipherSuiteInfo(mCipherSuite, &cipherInfo,
> +                             sizeof cipherInfo) != SECSuccess) {

nit: same here, etc.

@@ +73,2 @@
>  
> +  aCipherName = cipherInfo.cipherSuiteName;

I think "aCipherName.Assign(cipherInfo.cipherSuiteName);" would be best here
Attachment #8508974 - Flags: review?(dkeeler) → review+
I didn't add cipherSuite, because this just inspires people to copy the whole table from NSS instead of asking for the proper accessors.

https://hg.mozilla.org/integration/mozilla-inbound/rev/eaaa68a7c2d8
https://hg.mozilla.org/integration/mozilla-inbound/rev/128fd9cbc29b
https://hg.mozilla.org/mozilla-central/rev/eaaa68a7c2d8
https://hg.mozilla.org/mozilla-central/rev/128fd9cbc29b
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
> # LOCALIZATION NOTE (pageInfo_WeakEncryptionWithBits): %1$S is the name of the encryption standard,
> # %2$S is the key size of the cipher.
>-pageInfo_WeakEncryptionWithBits=Connection Encrypted: Low-grade Encryption (%1$S, %2$S bit keys)
>+# %3$S is protocol version like "SSL 3" or "TLS 1.2"
>+pageInfo_WeakEncryptionWithBitsAndProtocol=Connection Encrypted: Low-grade Encryption (%1$S, %2$S bit keys, %3$S)
> pageInfo_Privacy_Weak1=The website %S is using low-grade encryption for the page you are viewing.

nit: The Loc Note needs to be adjusted to reflect updated string name
Whoops - thanks for the heads-up. That's going to be removed in bug 947149 anyway.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.