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

Status: RESOLVED FIXED

(Core Graveyard :: Security: UI, defect)

Description

[Max Jonas Werner [:Makkes]]

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".

Comment 1

[Frederik Braun [:freddy]]

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).

Comment 2

[Frederik Braun [:freddy]]

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?

Comment 3

[Tom S [:evilpie]]

Attachment: WIP

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 4

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

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?

Comment 5

[Tom S [:evilpie]]

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.

Comment 6

[Tom S [:evilpie]]

*thing

Comment 7

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

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.

Comment 8

[Tom S [:evilpie]]

For Chome parity we also need to show SHA1.

Comment 9

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

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.

Comment 10

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

Attachment: IRC chat

Comment 11

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

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].

Comment 12

[Gavin Lloyd]

(In reply to Brian Smith (:briansmith), was [e-mail] (: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?

Comment 13

[Tom S [:evilpie]]

Now that we are thinking about using the new cache, can we change the on disk format?

Comment 14

[Honza Bambas (:mayhemer)]

(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.

Comment 15

[Tom S [:evilpie]]

Mhm sounds like we are still blocked by the on disk format. I hope somebody else feels like figuring this out.

Comment 16

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

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.)

Comment 17

[Hubert Kario]

Printing the size of parameters and the used curve for DHE and ECDHE would also be useful.

Comment 20

[Tom S [:evilpie]]

Can we do this nowadays? I would really like to have at least the protocol version.

Comment 21

[Tom S [:evilpie]]

Attachment: v1 Change nsISSLStatus

This adds the protcolVersion and the cipherSuite (id) to nsISSLStatus.

Comment 22

[Tom S [:evilpie]]

Attachment: v1 Add protocol to page info

Comment 23

[Tom S [:evilpie]]

Attachment: v1.1 Add protocol to page info

Comment 24

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

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.

Comment 25

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

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)

Comment 26

[Tom S [:evilpie]]

Attachment: v2 Change nsISSLStatus

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.

Comment 27

[Tom S [:evilpie]]

Attachment: v2 Change nsISSLStatus

Dammit forgot to refresh, see previous comment.

Comment 28

[Tom S [:evilpie]]

Comment on attachment 8507226 [details] [diff] [review]
v1.1 Add protocol to page info

It seems like you reviewed something here before.

Comment 29

[Ettore Atalan]

I would like to see if (perfect) forward secrecy is enabled.

Is it possible to implement such a feature?

Comment 30

[Hubert Kario]

(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)

Comment 31

[Dave Garrett]

(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 32

[Dão Gottwald [:dao]]

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 33

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

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?

Comment 34

[Tom S [:evilpie]]

(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.

Comment 35

[Ettore Atalan]

Attachment: 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/

Comment 36

[Tom S [:evilpie]]

I think the only important information that we can't extract from SSLCipherSuiteInfo is the stuff from SSLChannelInfo, which includes authKeyBits and keaKeyBits.

Comment 37

[Tom S [:evilpie]]

Attachment: v3

I asked on dev-security what information we should expose. Just uploading my current patch, but waiting for feedback on the list before continuing.

Comment 38

[Dave Garrett]

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)

Comment 39

[Tom S [:evilpie]]

Attachment: ssl-frontend

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.

Comment 40

[Dão Gottwald [: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?

Comment 41

[Tom S [:evilpie]]

I am just following the code around it!

Comment 42

[Dão Gottwald [:dao]]

(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 43

[Tom S [:evilpie]]

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.

Comment 44

[Dão Gottwald [:dao]]

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?

Comment 45

[Tom S [:evilpie]]

No.

Comment 46

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

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

Comment 47

[Tom S [:evilpie]]

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

Comment 48

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/eaaa68a7c2d8
https://hg.mozilla.org/mozilla-central/rev/128fd9cbc29b

Comment 49

[Vlado Valastiak [:wladow]]

> # 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

Comment 50

[Dana Keeler (she/her) (use needinfo) [:keeler] (on leave)]

Whoops - thanks for the heads-up. That's going to be removed in bug 947149 anyway.