549460 - The <keygen> handler signs PublicKeyAndChallenge with MD5WithRSA

Status: RESOLVED WONTFIX

(Core :: Security: PSM, defect)

Description

[Wan-Teh Chang]

The <keygen> handler signs PublicKeyAndChallenge with MD5WithRSA:
http://mxr.mozilla.org/security/ident?i=DEFAULT_RSA_KEYGEN_ALG

Is the use of MD5 required or just historical?

The keygen tag documentation on developer.mozilla.org doesn't
specify the signature algorithm:
https://developer.mozilla.org/En/HTML/HTML_Extensions/KEYGEN_Tag

But the HTML 5 spec specifies MD5WithRSA:
http://dev.w3.org/html5/spec/Overview.html#the-keygen-element

Comment 1

[Kai Engert (:KaiE:)]

Wan-Teh, what's the change or action you're requesting?

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

The issue is the use of MD5.  
At a minimum, we should be using SHA1. 
For large key sizes, we should arguably be using larger SHA hashes.

Comment 3

[Kai Engert (:KaiE:)]

I'm worried that simply changing our default behaviour (the output that a KEYGEN element in a form produces) may break consuming servers.

It seems neccessary (to me) that any change to use a different signature algorithm would require the introduction of "HTML attributes" for the KEYGEN element. Would the existing AlgorithmIdentifier attribute be sufficient for this purpose?

If we decided to deprecate the use of MD5 for submitting KEYGEN form data, how could that be done? For example, we could decide:

- the use of the AlgorithmIdentifier is mandatory

- if no AlgorithmIdentifier attribute is specified, the default rendering
  of the keygen element could display a warning message
    (user visible inside the document rendering)
  and the implementation could refuse to create a key pair

Who at Mozilla would be interested to work with CAs and HTML standard groups to make this happen?

Comment 4

[Eddy Nigg (StartCom)]

I'm positive that the hash should be changed to SHA1 - if you have a test build feel free to test at StartSSL.

Comment 5

[Kai Engert (:KaiE:)]

Giving you a test build is easy.
https://tbpl.mozilla.org/?tree=Try&rev=78de52596cdd

Download tomorrow from
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/?C=M;O=D
(A directory will appear containing "kaie").

But I'm not yet convinced that having a single positive test result with one single CA site would be sufficient to change the old default, in particular since HTML 5 standard documents mention that it uses MD5.

Comment 6

[Kai Engert (:KaiE:)]

Eddy, did you ever play with the <keygen AlgorithmIdentifier=...> attribute?

Comment 7

[Eddy Nigg (StartCom)]

Honestly never. My bandwidth is a bit limited at the moment, not sure if I can make somebody available for that.

Comment 8

[Kai Engert (:KaiE:)]

Attachment: Patch v1

attaching for documentation purposes.

As said before, I'm not sure/convinced this simple strategy is the right approach.

Comment 9

[Kai Engert (:KaiE:)]

Unfortunately we didn't get feedback yet for the test builds I had proposed.
I understand it's difficult to use test builds with short availability.
I also believe this change has the potential for regressions, and it's therefore important to ask for broad testing, especially by CAs.

In order to allow for early testing of this proposed patch, I've made binaries available at http://flowerbeetle.org and I hope those binaries will have a better lifetime, so it's more likely that you are able to get and test it.

Thanks in advance for your feedback when testing this potential change.

Comment 10

[Wan-Teh Chang]

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

>-            algTag = DEFAULT_RSA_KEYGEN_ALG;
>+            algTag = SEC_OID_PKCS1_SHA1_WITH_RSA_ENCRYPTION;

It would be better to simply define DEFAULT_RSA_KEYGEN_ALG as
SEC_OID_PKCS1_SHA1_WITH_RSA_ENCRYPTION.

By the way, I tested such a change with Chromium and it worked
with StartSSL CA.

Comment 11

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

Now the question is whether we should use SHA2 instead of SHA1, given that we're starting on the path to deprecating SHA2. I say we try to switch to SHA2 and see what breaks.

Note that the HTML 5 spec itself says that MD5 should be used: "Generate an RSA key pair using the settings given by the user, if appropriate, using the md5WithRSAEncryption RSA signature algorithm (the signature algorithm with MD5 and the RSA encryption algorithm) referenced in section 2.2.1 ("RSA Signature Algorithm") of RFC 3279, and defined in RFC 2313. [RFC3279] [RFC2313]."

So, we also need to change the spec.

Comment 12

[Mike Hoye [:mhoye]]

I'm pulling this out of the [good first bug] pool, because core/security/PSM/seriously-people-wtf. Brian, would you be willing to be a mentor for this bug?

Comment 13

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

(In reply to Mike Hoye [:mhoye] from comment #12)
> I'm pulling this out of the [good first bug] pool, because
> core/security/PSM/seriously-people-wtf. Brian, would you be willing to be a
> mentor for this bug?

I recommend :rbarnes.

Comment 14

[Richard Barnes [:rbarnes]]

I thought that <keygen> was dead technology :)

FWIW, the spec still says MD5.  So the idea would be that we would just unilaterally switch over?  And trust that everyone has SPKI processing code that doesn't make assumptions about hashes?

I guess I can add this to the queue.

Comment 15

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

spec changes + compatibility concerns + keygen = wontfix.