1227905 - Support ChaCha20+Poly1305 cipher suites

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Tim Taubert [:ttaubert] (inactive)]

Bug 917571 will add the cipher, this bug is about bringing it to TLS.

Comment 1

[Tim Taubert [:ttaubert] (inactive)]

Patch at: https://codereview.appspot.com/277090043

Based on the cipher added in bug 917571, and Adam's initial implementation. Fixed/Changed how the IV is constructed according to the current draft. ssl_gtests.sh runs successfully with the new cipher.

Any tips for TLS loopback tests I could write? I see that the cipher is selected and the tests succeed, but should we exchange actual application data and assert the chosen cipher suite?

Comment 2

[Eric Rescorla (:ekr)]

(In reply to Tim Taubert [:ttaubert] from comment #1)
> Patch at: https://codereview.appspot.com/277090043
> 
> Based on the cipher added in bug 917571, and Adam's initial implementation.
> Fixed/Changed how the IV is constructed according to the current draft.
> ssl_gtests.sh runs successfully with the new cipher.
>

OK. Will try to take a look today or over the weekend.


> Any tips for TLS loopback tests I could write? I see that the cipher is
> selected and the tests succeed, but should we exchange actual application
> data and assert the chosen cipher suite?

Yes. And there are existing tests for that, I believe.

Comment 3

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Eric Rescorla (:ekr) from comment #2)
> OK. Will try to take a look today or over the weekend.

Thanks!

> > Any tips for TLS loopback tests I could write? I see that the cipher is
> > selected and the tests succeed, but should we exchange actual application
> > data and assert the chosen cipher suite?
> 
> Yes. And there are existing tests for that, I believe.

Just found TlsConnectTestBase::SendReceive(), will work on a test.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

Added a test and updated the patch.

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

New version of the patch at: https://codereview.appspot.com/277090043

Updated cipher suite numbers according to the latest draft at https://tools.ietf.org/html/draft-ietf-tls-chacha20-poly1305-03

Comment 6

[Martin Thomson [:mt:]]

r+ and nits on rietveld.

Comment 7

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Martin Thomson [:mt:] from comment #6)
> r+ and nits on rietveld.

I don't see new comment on rietveld, did you forget to submit them?

Comment 8

[Martin Thomson [:mt:]]

Must have lost it.  I looked again.

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Martin Thomson [:mt:] from comment #8)
> Must have lost it.  I looked again.

Thanks! Updated patch to address your feedback.

Comment 10

[Tim Taubert [:ttaubert] (inactive)]

Patch updated to reflect changes from draft-ietf-tls-chacha20-poly1305-04. Rebased due to Bob's patch too.

Comment 11

[Tim Taubert [:ttaubert] (inactive)]

Adding Wan-Teh because I added him as a reviewer on Rietveld.

Comment 12

[Wan-Teh Chang]

I reviewed the patch at https://codereview.appspot.com/277090043/.

Comment 13

[Tim Taubert [:ttaubert] (inactive)]

Posted the hopefully final version of the patch on Rietveld.

Comment 14

[Wan-Teh Chang]

I reviewed patch set 12 at https://codereview.appspot.com/277090043/.

Comment 15

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/projects/nss/rev/83e27ac21329

Comment 16

[Tim Taubert [:ttaubert] (inactive)]

Backed out due to Windows bustage:

https://hg.mozilla.org/projects/nss/rev/459978f0cd8a

Comment 17

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/projects/nss/rev/d60719dd22fd

Comment 18

[Tim Taubert [:ttaubert] (inactive)]

Had to back out again, there was some more breakage and I didn't want to block the tree for too long:

https://hg.mozilla.org/projects/nss/rev/11c4a1c6f906

Comment 19

[Tim Taubert [:ttaubert] (inactive)]

Will merge the patch [1] addressing the PKCS#11 bypass assertion into this one. It has r=ekr.

[1] https://codereview.appspot.com/286200043

Comment 20

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/projects/nss/rev/b6c9ec057991

Comment 21

[Tim Taubert [:ttaubert] (inactive)]

FTR, I just built a custom Nightly with latest NSS and was able to communicate successfully with OpenSSL 1.1.0-pre2 via TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256.