797258 - Stop supporting SHA-1 in WebRTC?

Status: RESOLVED WONTFIX

(Core :: WebRTC: Networking, defect)

Description

[Eric Rescorla (:ekr)]

In https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=790517&attachment=660321 Bsmith requested we stop supporting SHA-1 for verification.

I don't agree with this, but it needs to be resolved.

Comment 1

[Ethan Hugg [:ehugg]]

Attachment: Use SHA-256 instead of SHA-1 in signaling code

Comment 2

[Eric Rescorla (:ekr)]

Comment on attachment 668623 [details] [diff] [review]
Use SHA-256 instead of SHA-1 in signaling code

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

lgtm

Comment 3

[Ethan Hugg [:ehugg]]

Comment on attachment 668623 [details] [diff] [review]
Use SHA-256 instead of SHA-1 in signaling code


Pushed to Alder - https://hg.mozilla.org/projects/alder/rev/5371d2b66b09

Comment 4

[Ethan Hugg [:ehugg]]

Attachment: Increase max sdp line length to work with longer SHA-256 fingerprints.

Comment 5

[Ethan Hugg [:ehugg]]

Comment on attachment 668705 [details] [diff] [review]
Increase max sdp line length to work with longer SHA-256 fingerprints.


The increased length of SHA-256 was going over the old SDP line limits.  Small patch to get PC connecting again with SHA-256

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 668705 [details] [diff] [review]
Increase max sdp line length to work with longer SHA-256 fingerprints.

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

r+, but this reminds me that all these maximum-possible-size buffers are a waste of ram and still get overrun as we add stuff.

Comment 7

[Ethan Hugg [:ehugg]]

Comment on attachment 668705 [details] [diff] [review]
Increase max sdp line length to work with longer SHA-256 fingerprints.


Pushed to Alder - http://hg.mozilla.org/projects/alder/rev/38146b56dbdd

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

Brian - can you comment on the necessity of this (and ekr, can you respond?)  Are there any issues relating to Chrome interop?  Should we ask them to change ASAP, and is it possible?

Comment 9

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

Comment on attachment 668623 [details] [diff] [review]
Use SHA-256 instead of SHA-1 in signaling code

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

I think that this is a good thing to do to avoid interop problems later. We should be minimizing the use of SHA1 by not adding any new uses. Especially since we already have a patch for this, we might as well land it.

AFAICT, it is definitely important that every implementation accept SHA256/SHA384 hashes, so if Chrome or Firefox is not accepting SHA256/SHA384 hashes from the other side, then that is an important interop issue that should be resolved.

Note that I didn't spend any time thinking about the application-specific cryptographic properties needed here. In 2013 it avoiding SHA1 is the default, and any new use of SHA1 should come with a strong argument for why a SHA-2 hash can't be used. If there is such a strong argument then we should reconsider.

Consequently, I am nominating this to block bug 796463 again and setting the r?ekr.

Comment 10

[Eric Rescorla (:ekr)]

Comment on attachment 668705 [details] [diff] [review]
Increase max sdp line length to work with longer SHA-256 fingerprints.

Why are we changing candidate length? This applies to fingerprints....

Comment 11

[Eric Rescorla (:ekr)]

I knew there was something confusing here. These patches landed in alder 10/5
and were migrated to m-c.

Hence we already are emitting sha-256 digests.

The original bug was that we shouldn't accept sha-1. I don't agree with that.

Comment 12

[Eric Rescorla (:ekr)]

bsmith,

We talked this over at the workweek and came to the conclusion that this needed to be a WG decision.

If you want to pursue this, can you post something to [e-mail] to start the discussion?

Comment 13

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

(In reply to Eric Rescorla (:ekr) from comment #12)
> We talked this over at the workweek and came to the conclusion that this
> needed to be a WG decision.

Just to clarify, I don't object much to supporting verification with SHA-1 but I think we should always *send* SHA-256. AFAICT, All the places that allow us to use SHA-1 also allow us to use SHA-256. I don't think that policy requires any working group effort.

If the spec already requires SHA-1 support to verification then I think it is useful to know WHY it requires SHA-1 support. That is something I cannot answer and it would be better to have somebody who is familiar with the decision making explain the need for SHA-1 support.

Comment 14

[Eric Rescorla (:ekr)]

I believe we already send SHA-256.

The spec doesn't require anything, but when it was written it defined digests as far
back as like MD5 (and I think maybe MD2). But to the extent to which there are
deployed stacks (Cisco) they presumably use SHA-1.

Do you object to closing this, then?

Comment 15

[Maire Reavy [:mreavy]]

Ekr, bsmith -- Given the discussions above, I'm removing this from the blocker list for WebRTC.  If you object, please let me know.  Thanks.

Comment 16

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

My understanding, based on talking with Eric, is that we can't stop accepting SHA-1 because some other devices send SHA-1. However, my understanding of that conversation was that we were already supposed to be sending SHA-256 and that we were already able to accept SHA-256. But, the patches in this bug make me think that we don't send SHA-256 and we can't accept SHA-256. Am I misunderstanding them?

Comment 17

[Eric Rescorla (:ekr)]

These patches already landed.

We currently send SHA-256 but accept SHA-1 and SHA-256.

Comment 18

[Eric Rescorla (:ekr)]

Comment on attachment 668623 [details] [diff] [review]
Use SHA-256 instead of SHA-1 in signaling code

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

OBE

Comment 19

[Eric Rescorla (:ekr)]

Comment on attachment 668705 [details] [diff] [review]
Increase max sdp line length to work with longer SHA-256 fingerprints.

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

OBE

Comment 20

[Maire Reavy [:mreavy]]

Ekr - can we close this?

Comment 21

[Eric Rescorla (:ekr)]

Yes.