732390 - Add preference to configure acceptance of MD5 in signatures, still accept by default

Status: RESOLVED FIXED

(Core :: Security: PSM, defect)

Description

[Kai Engert (:KaiE:)]

Bug 590364 suggests to disable MD5 for signature verification in NSS by default.

This bug proposes to introduce an Mozilla application level preference (hidden, about:config) that is disabled by default, but allows users to reenable MD5.

I propose to use a single preference to cover MD5 in both certificate validation and cms (signed message) validation.

Comment 1

[Kai Engert (:KaiE:)]

Attachment: Patch v1

Comment 2

[Kai Engert (:KaiE:)]

Please note that as part of this bug, I'm also proposing to disable the default of one of the SSL 3 cipher suites, the only one that uses MD5 and is currently enabled by default.

Comment 3

[Kai Engert (:KaiE:)]

Comment on attachment 602330 [details] [diff] [review]
Patch v1

Dan, could you please give feedback that you're OK with the suggested pref?

Bob, could you please review the code in function configureMD5 ?

Honza, could you please review the pref setting code?

Thanks!

Comment 4

[Kyle Hamilton]

I think that a single application-level preference is inappropriate for CMS.  The reason is that certain contacts may have to use MD5 (in the case of old smart cards, for example), but it should only be permitted for those contacts.  I suggest an additional exception per-contact, not per-application.  (It's tantamount to providing a preference to disable all security, rather than disable security in very limited circumstances.)

I think that MD5 should be made an overridable certificate exception, after MD5 is explicitly enabled in about:config.

Comment 5

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

I agree 100% with Kyle. We should not add this preference.

Comment 6

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

(In reply to Kyle Hamilton from comment #4)
> after MD5 is explicitly enabled in about:config.

We don't need a process-wide option at all. We can just make it something that each application (SSL, SMIME) can override in its own application-specific way, if necessary. AFAICT, this means adding the ability to choose whether MD5 is enabled on a per-validation basis to NSS.

Comment 7

[Kyle Hamilton]

I'd rather see people explicitly enable an insecure hash algorithm in a non-exceptional circumstance, so that they are cognizant that they have to be careful with it.  Otherwise, it's just too risky to enable.  MD5 attacks are near realtime at this point, and they're only going to get faster.

(also, security.ssl3.rsa_rc4_128_md5 defaults to true.  I just filed bug 732673 to change it.)

Comment 9

[Honza Bambas (:mayhemer)]

Comment on attachment 602330 [details] [diff] [review]
Patch v1

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

r=honzab

::: security/manager/ssl/src/nsNSSComponent.cpp
@@ +1853,5 @@
>        SEC_PKCS12EnableCipher(PKCS12_DES_56, 1);
>        SEC_PKCS12EnableCipher(PKCS12_DES_EDE3_168, 1);
>        SEC_PKCS12SetPreferredCipher(PKCS12_DES_EDE3_168, 1);
>        PORT_SetUCS2_ASCIIConversionFunction(pip_ucs2_ascii_conversion_fn);
> +      

No new white spaces please.

Comment 10

[Honza Bambas (:mayhemer)]

(In reply to Brian Smith (:bsmith) from comment #6)
> We don't need a process-wide option at all.

We do.  It is a requirement for rapid release and I can imagine broken servers/services that will need to be fix.  A workaround as NSS_SSL_CBC_RANDOM_IV=0 is in place here.

Comment 11

[Kyle Hamilton]

(I think I was unclear, sorry for bugspam)

We do need this preference, and we do need it disabled by default.

UI flow for state disabled:
- forbid exception

UI flow for state enabled:
- Prompt to add exception

I suggest looking down the road and creating preferences security.digest.md5, security.digest.sha1, and so on.  (Perhaps this would be good for all other algorithms, too.  As of January 1, 2011, PIV environments require sha256, and the Federal Bridge CA cross-certificate to the SHA-1 Federal Root CA expires on December 31 2013.  http://www.idmanagement.gov/fpkipa/documents/SHA256_Transition_Lessons_Learned.pdf  In addition, RSA is not permitted in Suite B, nor is RC4.)

Comment 13

[Robert Relyea]

I'm going out on a limb here saying I believe that creating case specific overrides is not necessary. More importantly it's development resource heavy. Kai doesn't have the bandwidth to do this.

Currently MD-2 and MD-4 signatures are already turned off in this way system wide. I think we need to decided if the instance of MD-5 signatures is high enough to warrant the need to have one-off overrides, which either means we don't turn off MD-5 (be cause no one has the bandwidth to special case MD-5 against any other hash function), or we allow the ability to override *all* bad signatures, or we continue to always accept MD-5 signatures.

The Chrome team reported that the only major use of MD-5 that they ran into were MITM corporate firewalls that were configured to sign the MITM certs with MD-5. In those cases it's pretty clear that you just want to accept MD-5 all the time rather than make exceptions for each certificate.

Anyway we need to pick among realistic options:

1) Disable MD-5 for signatures by default (and only allow a pref to turn it back on).
2) Disable MD-5 for signatures by default, but allow bad signature override.
3) Disable MD-5 for signatures by default, allow bad signature override, and a pref to turn it back on.
4) Status quo (continue to accept MD-5 signature).

I believe we should do 1). I can under stand mozilla choosing 3. It think 2 and 4 or just bad ideas. Anything else is not realistic given our resources.

BTW, as some point the NSS team would probably want to disable MD-5 signatures by default, which means PSM would have to do something special to maintain option 4.

bob

Comment 14

[Robert Relyea]

Comment on attachment 602330 [details] [diff] [review]
Patch v1

r+, but don't check this in.

The r+ is that the code does what is advertised (option 1).
We still need buy in that this is the right way to go.

bob

(other comments on the patch: in an ideal world, the hidden preference should control the full range of hash functions and full range of NSS policies for them. That full range is currently available by environment variable).

Comment 15

[Kai Engert (:KaiE:)]

I agree with Bob, I agree with Option ==> 1

IMHO it's unrealistic to get Brian's/Kyle's suggestions done, I don't know who would be able to get it done short term. If you would like to get it done anyway and you are able to drive all the required to completion, then you can file a separate bug.

I propose I'll check in after Dan tells me the pref is OK.

Comment 16

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

In bug 590364 comment 14, Kai said that this patch disables the RSA_RC4_MD5 ciphersuite. As the use of MD5 in that ciphersuite is HMAC-MD5, and not plain MD5, I do not think that support for RSA_RC4_MD5 should be changed.

To what extent has the compatibility impact of disabling RSA_RC4_MD5 been evaluated?

Comment 17

[Kai Engert (:KaiE:)]

(In reply to Brian Smith (:bsmith) from comment #16)
> In bug 590364 comment 14, Kai said that this patch disables the RSA_RC4_MD5
> ciphersuite. As the use of MD5 in that ciphersuite is HMAC-MD5, and not
> plain MD5, I do not think that support for RSA_RC4_MD5 should be changed.

Bob, what's your opinion, keep enabled, or disable that MD5-related ciphersuite?


> To what extent has the compatibility impact of disabling RSA_RC4_MD5 been
> evaluated?

No evaluations from me.

Comment 18

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

(In reply to Robert Relyea from comment #13)
> The Chrome team reported that the only major use of MD-5 that they ran into
> were MITM corporate firewalls that were configured to sign the MITM certs
> with MD-5. In those cases it's pretty clear that you just want to accept
> MD-5 all the time rather than make exceptions for each certificate.

Thanks, this is useful to know. I agree that it wouldn't be good to require the user to add an exception for each EE certificate in that situation. However, AFAICT, it would be best to be able to add an exception for each *signer* that uses MD5.

Please send a note to release-drivers about this change, describing what will happen, how much (if any) comparison you did with the behavior of other browsers, and/or how you assessed the compatibility impact. That way, Asa and others that have made compatibility one of the highest priorities can decide whether they are satisfied with the compatibility impact.

Comment 19

[Kai Engert (:KaiE:)]

(In reply to Kai Engert (:kaie) from comment #17)
> (In reply to Brian Smith (:bsmith) from comment #16)
> > In bug 590364 comment 14, Kai said that this patch disables the RSA_RC4_MD5
> > ciphersuite. As the use of MD5 in that ciphersuite is HMAC-MD5, and not
> > plain MD5, I do not think that support for RSA_RC4_MD5 should be changed.
> 
> Bob, what's your opinion, keep enabled, or disable that MD5-related
> ciphersuite?


FWIW, if this detail requires a debate, I propose we move this decision to a separate bug. (I thought it's obvious we want this disabled).

If you want a separate bug and separate discussion, I'll attach another patch with this ciphersuite preference unchanged.

Comment 20

[Kai Engert (:KaiE:)]

(In reply to Brian Smith (:bsmith) from comment #18)
> 
> Please send a note to release-drivers about this change, describing what
> will happen, how much (if any) comparison you did with the behavior of other
> browsers, and/or how you assessed the compatibility impact. That way, Asa
> and others that have made compatibility one of the highest priorities can
> decide whether they are satisfied with the compatibility impact.

Brian, did you misunderstand that it was Mozilla who has requested this change?

I suggest you reread bug 590364.

I don't intend to do anything that you proposed in comment 18.

Comment 21

[Kai Engert (:KaiE:)]

We decided:
- limit this to MD5 in certificate signatures specifically
- rename the preference to security.enable_md5_signatures
- keep the SSL/TLS ciphersuite using MD5 enabled
  (because it's not a problem)
- use a single pref for both Certificate-Signatures and S/MIME message signatures

Comment 22

[Kai Engert (:KaiE:)]

I just noticed we have the separate bug 650355, which was supposed to cover the application level (PSM level) work.

During our most recent NSS conference call, I have proposed:
- let's start by implementing the PSM level pref
- as a first step, keep the application level default,
  in other words,
  keep MD5 in signatures enabled
- the above allows users to play with MD5-signatures disabled at
  the application level and do some testing
- Dan Veditz liked that idea

So I intend to do:

- use this bug 732390 to get the preference implemented,
  using a pref value of MD5-signatures still acceptable

- afterwards, 
  use bug 650355 for switching the pref to "reject MD5-signatures"

Comment 23

[Kai Engert (:KaiE:)]

Attachment: Patch v2

Comment 24

[Kai Engert (:KaiE:)]

moving keywords compat, relnote to bug 650355

Comment 25

[Kai Engert (:KaiE:)]

Comment on attachment 604625 [details] [diff] [review]
Patch v2

This patch has feedback+ from dveditz, as I'm using the new preference name as agreed by phone.

This patch sets the initial value of the pref to "still accept MD5 signatures", as agreed during the conf call.

This patch removes the change of pref for the cipher suite, as requested during the conf call.

This patch didn't change the code that manipulates the pref - therefore this patch still has r=honzab

This patch didn't change the way we call NSS to toggle the MD5 signature acceptable - therefore this patch still has r=relyea

Comment 26

[Kai Engert (:KaiE:)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d15167715965

Comment 27

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/mozilla-central/rev/d15167715965