1158489 - (CVE-2015-7575) Prevent MD5 Downgrade in TLS 1.2 Signatures

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Karthikeyan Bhargavan]

Attachment: Report on exploiting hash collisions in TLS

Since 2011 (Bug 650355), NSS was supposed to "stop accepting MD5 as a
hash algorithm in signatures".  While this applies to signatures
within certificates, NSS still allows MD5 signatures in the server
signature in the TLS 1.2 ServerKeyExchange message. This exposes NSS
based clients such as Firefox to theoretical collision-based forgery
attacks. More pragmatically, we believe it is important to draw
attention to this bug before TLS 1.3 is implemented, because a similar
bug in the processing of the TLS 1.3 ServerCertificateVerify message
would lead to a serious server impersonation attack.


To see the bug, connect with Firefox to an openssl 1.0.1m server with
a modified ssl/s3_srvr.c (attached) which always signs the
ServerKeyExchange with RSA-MD5.  When Firefox connects to a server,
its signature algorithms extension only advertises the following
signature/hash algorithms: SHA256/RSA, SHA384/RSA, SHA1/RSA,
SHA256/ECDSA, SHA384/ECDSA, SHA1/ECDSA, SHA256/DSA, SHA1/DSA. Notably,
it should not allow any MD5 signature. However, when our server sends
it an RSA-MD5 signature, NSS does not check that this algorithm is
included in the allowed algorithms and quietly accepts it.

The root cause for this bug is that TLS 1.2 allows the signer to
choose any combination of signature and hash algorithm (even ECDSA-MD5
if it so wishes) and it is up to the verifier to check that the
algorithm is acceptable.  This is a change from TLS 1.1 (which only
allowed MD5SHA1); notably the current TLS 1.3 draft also allows MD5
signatures.

We note that NSS allows any signature/hash algorithm combination in
ServerKeyExchange but in the ClientCertificateVerify it forces the
client to use the hash algorithm that will be used in the Finished
message (SHA-256 for TLS 1.2, MD5SHA1 for TLS 1.1) again ignoring the
signature algorithms extension. We anticipate that a future update to
NSS would also generalize the client signature (to support SHA-512,
for example); and this report warns that any such update should be
careful to avoid MD5 downgrades.

The impact of the bug depends on whether an adversary can trigger
meaningful collisions in TLS client and server signatures. Folklore
about TLS has previously suggested that only second preimage attacks
"break" TLS signatures, collisions do not. We attach a draft research
report that shows that collision resistance is essential for the
security of client and server signatures in TLS. We show how
chosen-prefix collisions in the hash function can be used to forge
client signatures in TLS 1.2 and server signatures in TLS 1.3.
We believe this result is of independent interest. Collisions in
any of the handshake hashes in TLS can be translated to real
man-in-the-middle attacks.

We would be happy to provide further tests (such as a test server)
and discuss countermeasures. The straightforward fix is to check
that the signature/hash algorithm in the ServerKeyExchange message
is one of the algorithms sent in the ClientHello.

Comment 1

[Karthikeyan Bhargavan]

Attachment: modified openssl 1.0.1m s3_srvr.c to demo bug

Comment 2

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

(In reply to Karthikeyan Bhargavan from comment #0)
> We would be happy to provide further tests (such as a test server)
> and discuss countermeasures. The straightforward fix is to check
> that the signature/hash algorithm in the ServerKeyExchange message
> is one of the algorithms sent in the ClientHello.

The conclusion of your paper is that collision resistance is a requirement for the digest algorithm. We basically don't consider SHA-1 to be collision resistant any more. Consequently, SHA-1 should be treated the same way and all the SHA-1 algorithms should be removed from the supported signature algorithms extension.

While there are probably few compatibility concerns regarding removing support for MD5 signatures (especially since they aren't advertised in the extension), there may be more compatibility concerns for removing SHA-1. It would be good to see results of scans indicating (a) How many servers choose a digest weaker than SHA-256 when SHA-1 is advertised in the supported signature algorithms extension, and (b) how many servers break if SHA-1 is removed. But, dropping MD5 support shouldn't be blocked on that.

Comment 3

[Karthikeyan Bhargavan]

I agree that we should prepare for SHA-1 being deprecated, but as far as I know
chosen-prefix attacks for SHA-1 are still not considered practical, although 
they're in the "coming soon" category.

The report focuses on the use of hash functions in signatures, since this is
configurable since TLS 1.2. However, the same argument holds for the use of
hash functions in the Finished message. In TLS 1.2 we use SHA-256 which is considered
safe, but in earlier versions we use MD5+SHA1 and practical chosen-prefix collisions 
on this combined hash function would break the integrity of TLS handshakes.

Comment 4

[Karthikeyan Bhargavan]

Disregarding the general discussion above on collision resistance, the MD5 downgrade should be fixed soon.
Anything I need to do to get this bug confirmed?

Comment 5

[Wan-Teh Chang]

Karthikeyan: thank you very much for reporting the bug. I just gave the
"canconfirm" access to your Bugzilla account, so that future bugs you
file will start in the NEW state.

Martin: I assigned the bug to you. I will be happy to help or review
the patches.

Comment 6

[Martin Thomson [:mt:]]

I've identified what I think the most trivial fix for this bug could be:

Delete this line:
https://dxr.mozilla.org/mozilla-central/source/security/nss/lib/ssl/ssl3con.c#4353

  /* tlsHashOIDMap contains the mapping between TLS hash identifiers and the
   * SECOidTag used internally by NSS. */
  static const struct {
      int tlsHash;
      SECOidTag oid;
  } tlsHashOIDMap[] = {
-     { tls_hash_md5, SEC_OID_MD5 },
      { tls_hash_sha1, SEC_OID_SHA1 },
      { tls_hash_sha224, SEC_OID_SHA224 },
      { tls_hash_sha256, SEC_OID_SHA256 },
      { tls_hash_sha384, SEC_OID_SHA384 },
      { tls_hash_sha512, SEC_OID_SHA512 }
  };

I think that works for now, but it has been suggested that perhaps JC might have more time for doing this work.

If we are going to fix this properly - which I'm going to recommend happen in a follow-up - a more complete solution would have a global hash configuration.  That config could be used in ssl3_ClientSendSigAlgsXtn and it would then also drive ssl3_CheckSignatureAndHashAlgorithmConsistency.  That would allow NSS users, like Firefox, to set a policy that disabled SHA-1 at the point in time where that was considered feasible.

We'd need good numbers to support a decision to do that though.  It might be that this is sooner than we phase out SHA-1 in certificates, but that seems like a reasonable point in time to consider a change in policy.

Kai also pointed out that it's possible to detect attempts at hash collisions as described in http://marc-stevens.nl/research/papers/PhD%20Thesis%20Marc%20Stevens%20-%20Attacks%20on%20Hash%20Functions%20and%20Applications.pdf
I don't think that's worth doing if we're going to be ripping SHA-1 out.  That might be a good backup plan if it turns out that telemetry tells us that is a bad idea.

Comment 7

[Wan-Teh Chang]

Martin: we probably should go for a direct fix by only allowing the
signature algorithms advertised in ClientHello. I agree your trivial
fix should solve this problem, but it is an indirect fix and is
likely to break in the future.

Comment 8

[J.C. Jones [:jcj] (he/they)]

In the immediate term I'm pretty slammed, Martin - racking & configuring ISRG's DR datacenter Sunday-Wednesday. Earliest I can start taking a look would be 7 May. :(

Comment 9

[Karthikeyan Bhargavan]

Could we also fix the handling of signatureAlgorithms in CertificateRequest (and hence in CertificateVerify)? NSS currently ignores the requested signature algos and uses MD5SHA1 in TLS < 1.2
and SHA256 in TLS 1.2. This isn't standards compliant, but I guess not too many people are using client
signatures with other hash algorithms. It also means that a client that support SHA-512 signatures cannot
take advantage of the stronger hash algorithm. Again, clients using certs with hashAlg > SHA256 are probably rare.

Comment 10

[Martin Thomson [:mt:]]

Regarding comment 9, I think that we should try to keep the changes contained.  We can consider expanding scope in a follow-on.

I've done some investigation here and I think that we should add a control surface for selecting what signature algorithms we want to accept.

The hack I described might be OK if we need something quickly.  It's indirect and horrible, as Wan-Teh noted, but it should work.


For the control surface, I've a few API choices that I'd like to run by folks with more familiarity with the code.

A.1. Co-dependent settings for signature and hash.  e.g., 
SECStatus SSL_SignaturePrefSet(PRFileDesc *fd, SSLSignType sigAlg, SSLHashType hashAlg, PRBool enabled);
Noting that SSLHashType doesn't exist (see C).

A.2. Indepedendent settings:
SECStatus SSL_SignatureAlgPrefSet(PRFileDesc *fd, SSLSignType sigAlg, PRBool enabled);
SECStatus SSL_SignatureHashPrefSet(PRFileDesc *fd, SSLHashType hashAlg, PRBool enabled);

A.3. A job lot approach:
SECStatus SSL_SetSignatureAlgorithms(PRFileDesc *fd, SSLSignatureAlgorithm *algorithms, size_t count);


B.1. Include setters for the global defaults, like SSL_CipherPrefSetDefault does for cipher suites.

B.2. Don't.


C.1. A new type in sslt.h for SSLHashType.

C.2. Assuming A1, define a struct for signature algorithms:

typedef struct SSLSignatureAlgorithmStr {
  SSLSignType sign;
  SSLHashType hash;
} SSLSignatureAlgorithm;

C.3. Use the PKCS#11 definitions, such as SEC_OID_MD5.

C.4. Move the tls_hash_* definitions in ssl3prot.h into sslt.h


I'm leaning toward A.1, B.2, and C.1.

Comment 11

[Matt Wobensmith [:mwobensmith][:matt:]]

Martin, we are trying to assign this a security rating, but don't understand the implications of this bug. Can you advise? How easy is it to exploit?

Comment 12

[Martin Thomson [:mt:]]

I'd say that this is probably sec-high.  Collisions on MD5 are relatively cheap.  If second preimage resistance is not sufficient (I'm inclined to trust Karthik on this point, since I'm short on time and haven't read the report) then it is relatively easy to perform server impersonation based on this.

Comment 13

[Matt Wobensmith [:mwobensmith][:matt:]]

Thank you, Martin.

Comment 14

[Martin Thomson [:mt:]]

Attachment: bug1158489-1.patch

Groundwork for larger patch.

Comment 15

[Martin Thomson [:mt:]]

Attachment: bug1158489-2.patch

A rework of how we handle signature algorithms.

Comment 16

[Martin Thomson [:mt:]]

http://codereview.appspot.com/232480043 is available on request.

Comment 17

[Al Billings [:abillings - ex-MoCo]]

Wan-Teh, can you do the patch review here? This has been sitting around for more than a month and we need to get this fixed.

Comment 18

[Karthikeyan Bhargavan]

(In reply to Martin Thomson [:mt:] from comment #12)
> I'd say that this is probably sec-high.  Collisions on MD5 are relatively
> cheap.  If second preimage resistance is not sufficient (I'm inclined to
> trust Karthik on this point, since I'm short on time and haven't read the
> report) then it is relatively easy to perform server impersonation based on
> this.

I think server impersonation, based on this bug, is not really practical yet.
It requires either
(a) a server which chooses a predictable server random and repeats its (EC)DH key share
    [10% of servers reuse key shares but very very few seem to use predictable server randoms,
     but the latter is difficult to test, and may be plausible in virtualized scenarios.]

or

(b) an adversary capable of performing 2^64 queries, storing their results, and then 
peforming 2^64 MD5 hashes 
    [the last of these is practical, but the first two seem to be out of reach for most adversaries]

or

(c) a second preimage attack on MD5
    [this is still an open problem]

So, although I am all for this bug being fixed as soon as possible, I would in this case recommend a lower security rating than sec-high, even if it means no bounty :)

Comment 19

[Martin Thomson [:mt:]]

Thanks Kathik, hopefully we can fix this before this becomes sec-high based on any of those criteria :)

Comment 20

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Al, can you provide your assessment on whether this bug needs to be fixed in the upcoming ESR 38 release? Thanks!

Comment 21

[Al Billings [:abillings - ex-MoCo]]

(In reply to Ritu Kothari (:ritu) from comment #20)
> Al, can you provide your assessment on whether this bug needs to be fixed in
> the upcoming ESR 38 release? Thanks!

Given that it is a external report affecting TLS, I'd suggest taking this on ESR38.

Comment 22

[Al Billings [:abillings - ex-MoCo]]

If this is going into 40, it pretty much needs to get checked in today.

Comment 23

[Kate Glazko]

Tracking for ESR 38 given comment 21.

Comment 24

[Martin Thomson [:mt:]]

This is not going in today.  See comment 18 regarding severity.  Also, the patch needs another round, which it isn't getting on such short notice.

Comment 25

[Kate Glazko]

I noticed that you nominated this bug for esr38 tracking ?, does this mean that you don't want the security team to fix it? In that case we can wontfix and untrack. Let me know!

Comment 26

[Al Billings [:abillings - ex-MoCo]]

No, it means that I'd mark it tracking 41+ in the ESR38 tracking except 41+ doesn't exist yet.

Comment 27

[Martin Thomson [:mt:]]

Release notes draft for this change:

Bug 1158489 - NSS libssl incorrectly accepted signatures in TLS 1.2 using algorithms that it did not advertise in its signature_algorithms extension.  This resulted in libssl accepting MD5 hashes in signatures.  There are several functional changes as a result of this fix:
  - Support for MD5 hashes in signature algorithms has been removed.
  - Support for SHA-224 hashes in signature algorithms has been removed (SHA-224 was previously only partially supported).
  - The set of signature algorithms that libssl advertises and accepts can be configured, where this was previously hard-coded.
  - libssl now selects a signature algorithm to use based on the configured signature algorithms, rather than having a fixed preference order.
  - New API calls have been added:
    + SSL_SignaturePrefSet sets the signature algorithms that libssl will advertise or accept.  The order of this set is used to select the signature algorithm that is used; earlier values are preferred.  This configuration governs what value is advertised in the signature_algorithms extension or the supported_signature_algorithms field of the CertificateRequest message.  libssl rejects signatures in TLS 1.2 that identify a signature and hash algorithm that don't appear in this set. 
    + SSL_SignaturePrefGet retrieves the currently configured signature algorithms.
    + SSL_SignatureMaxCount gets the maximum number of signature algorithms that can be configured at the same time.

Comment 28

[Martin Thomson [:mt:]]

Attachment: bug1158489.patch

Also at https://codereview.appspot.com/256230044/

Comment 29

[Martin Thomson [:mt:]]

Attachment: bug1158489.patch

Updated based on ekr's comments, still awaiting final review.

Comment 30

[Martin Thomson [:mt:]]

I just opened Bug 1195434.  This provides justification/cover for these changes going in, which will happen soon.  It also avoids one concern that ekr raised regarding a mismatch between the configuration in NSS and in mozilla::pkix.  Other users of NSS will have to deal with this on their own, I'm afraid.

(clearing n-i on wtc)

Comment 31

[Martin Thomson [:mt:]]

https://hg.mozilla.org/projects/nss/rev/276a21813f7c

Comment 32

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Martin, since this is a FF41+ tracked bug. Will there be a fix for this issue in Beta41 time frame (1-2 more weeks left now)?

Comment 33

[Martin Thomson [:mt:]]

I will ask what the NSS release schedule is like.  I think that the plan is to get a few more features in before we branch.

Comment 34

[Martin Thomson [:mt:]]

Discussed on today's call.  The decision was to delay inclusion to Firefox 44 and to miss the next ESR 38 build.  There are a few more related changes to land in NSS.  None of the fixes are particularly urgent and we could use the extra time to do some more comprehensive testing.

Comment 35

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Based on comment 34, setting FF41 status to wontfix.

Comment 36

[Karthikeyan Bhargavan]

Can we push this bug along? 
This bug will be publicly disclosed at Real World Crypto (Jan 6-8 2016).
The disclosure page (still work-in-progress) is here:
http://prosecco.gforge.inria.fr/vulnerabilities/sloth.html
(user: vulnerability, pass: prosecco)

To summarize the impact on NSS, an attacker who can make 2^X connections to www.microsoft.com 
and store the server signatures, can subsequently impersonate www.microsoft.com to NSS clients 
by performing 2^(128-X) MD5 hashes. If X = 64 is practical, then the effective security of the connection is brought down to 64 bits, even if the server is using a 2048 bit RSA key with TLS 1.2 ECDHE-RSA-AES-GCM-SHA384. This attack is still out of reach for academics, but may well be in reach for a powerful government-scale adversary.

Comment 37

[Martin Thomson [:mt:]]

Ahh, this is already landed. I am reminded that we should request uplift of NSS 3.21 though.

Comment 38

[Al Billings [:abillings - ex-MoCo]]

(In reply to Martin Thomson [:mt:] from comment #37)
> Ahh, this is already landed. I am reminded that we should request uplift of
> NSS 3.21 though.

We're about to cut release candidates for Firefox 43 so you may have blown the dates there to make it into that release.

Comment 39

[Liz Henry (:lizzard) (relman/hg->git project)]

Martin, it could happen, but we're about to merge beta to mozilla-release and build the release candidate on Monday. If this is critical for the 43 release, we also have the option to do an RC2 in mid week.   It looks like you were planning to release a fix along with 44 and that more testing is needed. I don't think we will have time for that testing over the next week unless this is critical. 

Since this is rated sec-moderate, I wouldn't normally consider doing a second RC build for it. Do you think the planned disclosure in January changes how we should treat this bug?

Comment 40

[Martin Thomson [:mt:]]

As noted, it's sec-moderate, though Comment 36 does make this marginally more serious than the earlier analysis in Comment 18.  Either way, I don't think that we need to create a special build for it.

My recommendation is that we opportunistically uplift as things become available.  That is, take it with the next ESR that goes out, and if there is a second RC, consider including it there.  We've been using NSS 3.21 in Nightly for a while now and I've not heard even a whisper about stability or compatibility issues, so I think that it's safe to uplift.

The upcoming announcement could come with another media splash and another round of panicking from certain people.  If we want to get ahead of it, then it would be wise to at least have a plan in place.  I think that this is the plan: ship NSS 3.21 as the opportunity presents itself.

Note: when uplifting, we need to uplift the changes in bug 1211568, not this one.

Comment 41

[Liz Henry (:lizzard) (relman/hg->git project)]

Since the changes in bug 1211568 are already in 44 I think we should uplift them to beta as well. If you could request uplift there we could get it into Monday's RC build. That doesn't allow for a lot of testing or room for error but it sounds best to give it a try. Thanks Martin.

Comment 42

[Martin Thomson [:mt:]]

I requested uplift for the three relevant bugs.

Comment 43

[Kai Engert (:KaiE:)]

If you consider to uplift to the ESR 38.x update scheduled for next week, we need to discuss the best strategy.

ESR currently uses NSS 3.19.2.1. There have been a lot of changes between that and 3.21. The root CA list has been updated twice, and in the past, it was usually decided that regular CA updates should be excluded from the ESR branch (except in case of emergency CA updates).

Comment 44

[Liz Henry (:lizzard) (relman/hg->git project)]

OK. After some more discussion (thanks everyone for your advice!) let's not uplift this to 43. We still may need to do that, but if so, more planning and a bit more time would be better.

Comment 45

[Kai Engert (:KaiE:)]

Suggestion for Firefox 43 (or 43.1) and for ESR 38.5 (or 38.5.1):

Use Martin's minimal fix from comment 6, which Wan-Teh has reviewed in comment 7.


If this minimal fix can be considered sufficiently secure for the next 2-3 months, it seems more appropriate for a last minute hotfix.

Red Hat would like to make some more time testing NSS 3.21, and is looking for a minimal fix that can be backported to NSS 3.19.2.1


And, if the minimal fix can be considered sufficient for ESR 38, it might also be sufficient for the limited lifetime of the Firefox 43 branch.


If you agree to this approach, my suggestion is:

ESR 38
======
Currently uses NSS 3.19.2.1

Apply the minimal fix from comment 6 on a mini-branch and release NSS 3.19.2.2

Upgrade the ESR branch to 3.19.2.2


Firefox 43
==========
Currently uses NSS 3.20.1

Apply the minimal fix from comment 6 on a mini-branch and release NSS 3.20.2

Upgrade Firefox 43 to 3.20.2


I suggest that we go ahead and create the NSS releases immediately, and you can decide if you want to respin 43/38.5, or if you want to do a followup and do 43.1/38.5.1


Thoughts?

Comment 46

[Kai Engert (:KaiE:)]

Attachment: Martin's minimal fix, has r=wtc, suggested for ESR 38.x and 43.x

Comment 47

[Kai Engert (:KaiE:)]

(In reply to Kai Engert (:kaie) from comment #45)
> 
> I suggest that we go ahead and create the NSS releases immediately, and you
> can decide if you want to respin 43/38.5, or if you want to do a followup
> and do 43.1/38.5.1


Are we allowed to check in the minimal fix publicly on stable branches, or would that make it all too obvious that we found a security vulnerability?

If you're worried that it's too obvious, the alternative suggested timing is:

- on January 4, commit minimal hotfix to NSS, 
                create NSS branch releases,
                and update the 38.x and 43.x branches

- publish the updated 38.x and 43.x releases as soon after January 5 as you can.

Comment 48

[Kai Engert (:KaiE:)]

Could you please comment, which suggestion and which timing you like best?

Thanks

Comment 49

[Karthikeyan Bhargavan]

Hi,

If there's going to be a CVE number for this bug, do let me know so I can include it in our paper.

Many thanks,
Karthik

Comment 50

[Wan-Teh Chang]

Comment on attachment 8696762 [details] [diff] [review]
Martin's minimal fix, has r=wtc, suggested for ESR 38.x and 43.x

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

r=wtc.

I inspected the NSS source code in mozilla-esr38. I agree with Karthikeyan's
analysis that in all other places NSS doesn't sign with MD5 and doesn't allow
MD5 signatures -- partly because NSS properly disallows MD5, and partly because
NSS is hardcoded to use SHA-256 or SHA-1 in CertificateVerify.

Comment 51

[Huzaifa Sidhpurwala]

(In reply to Karthikeyan Bhargavan from comment #49)
> Hi,
> 
> If there's going to be a CVE number for this bug, do let me know so I can
> include it in our paper.
> 
> Many thanks,
> Karthik

Karthik, Red Hat already assigned CVE-2015-7575 to this flaw (i mailed you about this earlier)

Al, Can we use the above CVE please if possible to allow duplication.

Thanks!

Comment 52

[Karthikeyan Bhargavan]

Hi, I wasn't sure whether the Red Hat CVE was for openssl or NSS or both.

I think there is going to be a protocol-level CVE ("Don't use MD5 signatures in TLS 1.2")
asked for by the OpenSSL team.

In addition, maybe should be implementation-level CVEs specifically for those libraries
that accept MD5 signatures even though they are disabled; i.e. not offered in ClientHello/CertificateRequest and therefore not expected to be sent or accepted.
For example, NSS, GnuTLS, BouncyCastle have/had this bug.

Comment 53

[Huzaifa Sidhpurwala]

(In reply to Karthikeyan Bhargavan from comment #52)
> Hi, I wasn't sure whether the Red Hat CVE was for openssl or NSS or both.
> 
> I think there is going to be a protocol-level CVE ("Don't use MD5 signatures
> in TLS 1.2")
> asked for by the OpenSSL team.
> 
Red Hat issued a "protocol-level" CVE

> In addition, maybe should be implementation-level CVEs specifically for
> those libraries
> that accept MD5 signatures even though they are disabled; i.e. not offered
> in ClientHello/CertificateRequest and therefore not expected to be sent or
> accepted.
> For example, NSS, GnuTLS, BouncyCastle have/had this bug.

In several previous cases, no implementation level CVEs were issues. Code is implements the protocol. In essence code is not flawed, so i dont think we should issue code level CVEs

Comment 54

[Karthikeyan Bhargavan]

> Red Hat issued a "protocol-level" CVE

Ok, we will cite this CVE in our disclosure (hopefully other vendors will follow suit)

> In several previous cases, no implementation level CVEs were issues. Code is
> implements the protocol. In essence code is not flawed, so i dont think we
> should issue code level CVEs

I don't mind having only a protocol-level CVE; it simplifies things for disclosure.

Worth adding a word of warning though.  Say you're using Firefox/NSS and look at the SSL Labs Browser test, which says that your browser does not support RSA/MD5. Or you're using an old version of GnuTLS and you set your --priority to "NORMAL" which does not include MD5. You may expect that, as a user, you have now addressed the protocol-level CVE-2015-7575 by "disabling MD5". The bugs in these libraries (and in some others) is that disabling MD5 is not as simple as that. It may be worth adding specific wording to the protocol-level CVE to warn users and TLS implementations against this. 

Of course, we and others will probably have browser and server tests that will test for the vulnerability and provide the right feedback.

Comment 55

[Kai Engert (:KaiE:)]

By tomorrow, I must publicly commit the minimal fix to NSS backported branches, which might cause the issue to be more obvious, than the previous commit for this bug (which had used the text "Enable signature algorithm configuration").

I suggest we'll use the following text for the minimal fix:
  "Remove obsolete entry."

Regarding release notes, I suggest that we'll be vague in the NSS release notes document for now, and update them around after Karthikeyan has published the issue, with more details and credit.

I'd announce the updated NSS releases with vague information at the time Mozilla releases the first of {43.x,38.x} that contains the fix.

Comment 56

[Kai Engert (:KaiE:)]

Clearing needinfo from comment 45, I talked to Liz and Sylvestre on IRC.

A Firefox 43.x release shall happen this week, and because of that Liz confirmed I should go ahead and prepare the NSS release for it.

ESR hasn't been decided yet, waiting for a decision, but I'll create an appropriate release for it, too.

Comment 57

[Ryan Sleevi]

Confirmed with Chrome folks that we're treating this as not requiring a chemspill; we'll likely pick this up with Chrome 49 going to NSS 3.21, which means sometime in late Feb/early March to hit stable, based on reading the release calendar tealeaves.

Comment 58

[Kai Engert (:KaiE:)]

Attachment: upgrade mozilla-release 43 to NSS 3.20.2

The only difference between the suggested NSS 3.20.2 and the 3.20.1 currently being used will be attachment 8696762 [details] [diff] [review] plus NSS version number changes.

Comment 59

[Kai Engert (:KaiE:)]

Attachment: upgrade mozilla-esr38 to NSS 3.19.2.2

The only difference between the suggested NSS 3.19.2.2 and the 3.19.2.1 currently being used will be attachment 8696762 [details] [diff] [review] plus NSS version number changes.

Comment 60

[Liz Henry (:lizzard) (relman/hg->git project)]

OK, after some more discussion with Daniel and Kai, we are planning for Kai to land this on m-r tomorrow (which should be after I build 43.0.1).  We will aim this NSS update for 43.0.2, which we are planning to build and release early next week.   The goal here is to avoid the need for a 43.0.3 in early January.

Comment 61

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Kai, given that we are doing this for ESR38 and FF43, should we also uplift to Beta44? Thanks!

Comment 62

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Actually I just realized that FF44 was upgraded to NSS 3.21 in Bug 1211568 so we may not need to do anything here for FF44. Should I just mark it as fixed?

Comment 63

[Kai Engert (:KaiE:)]

correct, 44 and 45 are already fixed, because they use NSS 3.21

Comment 64

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8698647 [details]
upgrade mozilla-release 43 to NSS 3.20.2

Approved for uplift to mozilla-release. We are set with the 43.0.1 build now, so this work will go into 43.0.2.

Comment 65

[Kai Engert (:KaiE:)]

https://hg.mozilla.org/releases/mozilla-release/rev/fe61b8683f3a

Comment 66

[Kai Engert (:KaiE:)]

Comment on attachment 8696762 [details] [diff] [review]
Martin's minimal fix, has r=wtc, suggested for ESR 38.x and 43.x

BTW, the minimal patch has landed on stable NSS branches:

NSS_3_19_2_X_BRANCH for 3.19.2.2
https://hg.mozilla.org/projects/nss/rev/94e1157f3fbb

NSS_3_20_BRANCH for 3.20.2
https://hg.mozilla.org/projects/nss/rev/891676aa0d85

The commit to mozilla-release upgraded to 3.20.2

Comment 67

[Liz Henry (:lizzard) (relman/hg->git project)]

We are ready to push this with 43.0.2 release today.  Al I just realized, does this mean we should have a security advisory in place? Or is there one already as this upgrade is already released in 44 and onward?

Comment 68

[Liz Henry (:lizzard) (relman/hg->git project)]

I'll just link to the 43 sec advisory page for now from the release notes.

Comment 69

[Doug Turner (:dougt)]

****

Was this bug / fix under embargo?



Al - can you post the text of what we released today here?

Comment 70

[Al Billings [:abillings - ex-MoCo]]

We had an advisory live for about an hour today on this bug before I was advised that it might be under embargo. I pulled the advisory down. Text of advisory was:

Security researcher Karthikeyan Bhargavan reported an issue
in Network Security Services (NSS) where MD5 signatures in the server signature within the TLS 1.2 ServerKeyExchange message are still accepted. This is an issue since NSS has officially disallowed the accepting MD5 as a hash algorithm in signatures since 2011. This issues exposes NSS based clients such as Firefox to theoretical collision-based forgery attacks. This issue was fixed in NSS version 3.20.2.

Comment 71

[Doug Turner (:dougt)]

I mailed [e-mail] to ensure that he sees this.

Comment 72

[Karthikeyan Bhargavan]

What is the procedure for the advisory? Can you add one later?

Otherwise, the text above looks more or less ok, since it refers only to server authentication (which is the harder attack, compared to client authentication.) Any mention of the CVE would definitely be premature. The main risk is that (a) someone will start wondering what servers support MD5 signatures, and the thing might leak before RHEL 6 and 7 release patches, and (b) someone will start wondering about client authentication, before Java releases patches.

Comment 73

[Al Billings [:abillings - ex-MoCo]]

Usually, we write them when we release the fix but we can hold them until later as well.

Comment 74

[Huzaifa Sidhpurwala]

The advisory is already public, should we consider this issue public now?

Comment 75

[Karthikeyan Bhargavan]

Public disclosure of CVE-2015-7575 is in January, and other vendors need the time to fix.
Let's please be circumspect about what we consider public till then. Like I said, I do not
mind if the short text referred to above has already been leaked, but please do not add any
more detail or refer to the CVE at this point. Conversely, please do refer to the CVE
once the bug is publicly disclosed.

Comment 76

[Al Billings [:abillings - ex-MoCo]]

(In reply to Huzaifa Sidhpurwala from comment #74)
> The advisory is already public, should we consider this issue public now?

The advisory was removed. While the text may have leaked, it is down right now and there are no public references to the CVE at this time.

The text I posted above will go back online (with the CVE from here) once the talk has occurred. If you don't want it live then, Karthikeyan, we will need a disclosure date from you. Otherwise, I expect to post it on January 7, based on the presentation being complete. It won't contain any more data than what we posted before then.

In the future, we need to have embargo dates explicitly communicated. I should have seen the presentation dates and put it together myself not to talk about it but no one ever clearly communicated to Mozilla not to talk about it until after a specific date.

Comment 77

[Kai Engert (:KaiE:)]

Karthikeyan, at what time and timezone will your talk be given? Thanks.

Comment 78

[Kai Engert (:KaiE:)]

I just found the answer myself.
Thursday, Jan 7, 16:30 pacific time, which means, early after midnight Jan 8 UTC.

Comment 79

[Karthikeyan Bhargavan]

Hi, we just put sloth-attack.org online.
I'll be giving the talk tomorrow, but let's consider this public disclosure.

Comment 80

[Al Billings [:abillings - ex-MoCo]]

So the advisory with the text given in comment 70 (which has the CVE attached to this bug on it) can go live now?

Comment 81

[Karthikeyan Bhargavan]

Yes. Please cite the CVE number as well.

Thanks!

Comment 82

[Al Billings [:abillings - ex-MoCo]]

https://www.mozilla.org/en-US/security/advisories/mfsa2015-150/

Comment 83

[Liz Henry (:lizzard) (relman/hg->git project)]

The sec advisory says that we fixed this in 38.5.2 but I don't think that is the case.

Comment 84

[Kai Engert (:KaiE:)]

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #83)
> The sec advisory says that we fixed this in 38.5.2 but I don't think that is
> the case.

Al: Liz is right. Could you please fix the advisory from comment 82 and remove the statement about 38.x?

Comment 85

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8698648 [details]
upgrade mozilla-esr38 to NSS 3.19.2.2

Given that we've announced this fix in ESR (comment 83) and that the sec team thinks we should take this fix (comment 21, comment 40), let's get this into ESR 38.6.0.

Comment 86

[Kai Engert (:KaiE:)]

Comment on attachment 8698648 [details]
upgrade mozilla-esr38 to NSS 3.19.2.2

https://hg.mozilla.org/releases/mozilla-esr38/rev/9a92b504c10a

Comment 87

[Al Billings [:abillings - ex-MoCo]]

(In reply to Kai Engert (:kaie) from comment #84)
> 
> Al: Liz is right. Could you please fix the advisory from comment 82 and
> remove the statement about 38.x?

What the hell? We didn't ship this?

Comment 88

[Daniel Veditz [:dveditz]]

(In reply to Al Billings [:abillings] from comment #87)
> (In reply to Kai Engert (:kaie) from comment #84)
> > Al: Liz is right. Could you please fix the advisory from comment 82 and
> > remove the statement about 38.x?
> 
> What the hell? We didn't ship this?

We have _now_: comment 84 was 6 weeks ago; 4 weeks ago we shipped Firefox 38.6 ("44+") with NSS 3.19.2.2 containing the fix.

Comment 89

[Kai Engert (:KaiE:)]

Comment on attachment 8646029 [details] [diff] [review]
bug1158489.patch

Martin, only out of curiousity, was there a special reason why you added sha384/512 entries for rsa and ecdsa, but not for dsa, in this table? Or was it simply the thought they aren't necessary currently? Thanks in advance.

+static const SSLSignatureAndHashAlg defaultSignatureAlgorithms[] = {
+    {ssl_hash_sha256, ssl_sign_rsa},
+    {ssl_hash_sha384, ssl_sign_rsa},
+    {ssl_hash_sha512, ssl_sign_rsa},
+    {ssl_hash_sha1, ssl_sign_rsa},
+#ifndef NSS_DISABLE_ECC
+    {ssl_hash_sha256, ssl_sign_ecdsa},
+    {ssl_hash_sha384, ssl_sign_ecdsa},
+    {ssl_hash_sha512, ssl_sign_ecdsa},
+    {ssl_hash_sha1, ssl_sign_ecdsa},
+#endif
+    {ssl_hash_sha256, ssl_sign_dsa},
+    {ssl_hash_sha1, ssl_sign_dsa}
+};

Comment 90

[Martin Thomson [:mt:]]

Kai, I think that the SHA-256 version is an error.  See this from RFC 5246, Section 7.4.3:

   Because DSA signatures do not contain any secure indication of hash
   algorithm, there is a risk of hash substitution if multiple hashes
   may be used with any key.  Currently, DSA [DSS] may only be used with
   SHA-1.  Future revisions of DSS [DSS-3] are expected [...]

Comment 91

[Martin Thomson [:mt:]]

Oh, I realize now why it's in there.  Otherwise, we can't do DSA with TLS 1.2 because we advertise only SHA-256 paired with the asymmetric signature scheme in signature_algorithms.  It's a bit of a mess really.

Comment 92

[MAWA VANSMOOKEY]

Comment on attachment 8696762 [details] [diff] [review]
Martin's minimal fix, has r=wtc, suggested for ESR 38.x and 43.x

goog

Comment 93

[MAWA VANSMOOKEY]

Comment on attachment 8696762 [details] [diff] [review]
Martin's minimal fix, has r=wtc, suggested for ESR 38.x and 43.x

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

http:mtnuganda-cust.opera-mini.net:80/bugzilla/<html >/good

Comment 94

[MAWA VANSMOOKEY]

Comment on attachment 8696762 [details] [diff] [review]
Martin's minimal fix, has r=wtc, suggested for ESR 38.x and 43.x

good