Uprev NSS to 3.23 on iOS

net/quic/crypto/chacha20_poly1305_decrypter_nss.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/quic/crypto/chacha20_poly1305_decrypter.h"
 
 #include <pk11pub.h>
 
 using base::StringPiece;
 
 namespace net {
 
 namespace {
 
 const size_t kKeySize = 32;
 const size_t kNoncePrefixSize = 0;
 
 }  // namespace
 
 ChaCha20Poly1305Decrypter::ChaCha20Poly1305Decrypter()
     : AeadBaseDecrypter(CKM_NSS_CHACHA20_POLY1305,

[davidben, 2016/03/30 18:39:25]

Oof, I hadn't realized we actually supported ChaCha20-Poly1305 on NSS in QUIC
too when I suggested we remove that patch. I think this would have to switch to
this cipher being disabled and chacha20_poly1305_rfc7539_decrypter_nss.cc being
enabled.

rch: What is the state of chacha20_poly1305_rfc7539_decrypter_*.cc? I assume the
servers all speak both variants now, so it's okay if any iOS code switches from
only supporting the old to only supporting the new?

[Ryan Sleevi, 2016/03/31 02:52:58]

Do we have tests that cover the old behaviour?

And yeah, I missed that the implementation in NSS slightly differs, to reflect
the RFC status. So... rch, advice on what needs to change here? Do I need to
create a new broken_chacha patch to hack in the old mechanism?

[davidben, 2016/03/31 05:23:00]

I believe both sets of ChaCha20Poly1305 classes in QUIC have tests with test
vectors, so we'd notice. Note that the standardized variant is
chacha20_poly1305_rfc7539_*.cc. So I think you'd want to disable this set of
files and enable the other one. There's a
ChaCha20Poly1305Rfc7539Encrypter::IsSupported that I believe you want to move
over to ChaCha20Poly1305Encrypter instead.
I'm not rch, but I would vote we don't try to support both variants in
third_party/nss and instead only support the RFC construction. The RFC
construction was added a while ago, so I assume this is fine now. (Get rch to
confirm, of course.)

[Ryan Hamilton, 2016/04/04 14:37:01]

Yes, that's right. Do the unit tests pass with this change?
*nod*
Yes, I think that should be fine. Since QUIC is not used for web browsing on iOS
Chrome, we don't actually enable QUIC on iOS any longer. We will, however, use
QUIC on iOS for Cronet.

[davidben, 2016/04/04 14:55:52]

In that case, should we just take the code out? We can land that as a
prerequisite to this roll and avoid having to fuss with stuff. QUIC/iOS/Cronet
would be using the BoringSSL port anyway. Or is there still some intermediate
stage where we care about QUIC/iOS/NSS right now? (I notice you say "will"
rather than "do", so I'm guessing no?)

[Ryan Hamilton, 2016/04/04 16:00:50]

Perhaps I should have said "do". I know that we have external cronet customers
actively trying to get Cronet + QUIC + iOS working. I'm not sure if they've
succeeded, but they're trying. If they're not working yet, they will be RSN, as
I understand it. So I don't *think* we want to remove this code. But switching
from agl-ChaCha20/Poly1305 to RFC-ChaCha20/Poly1305 on iOS should be fine.

When you say that "QUIC/iOS/Cronet would be using the BoringSSL port anyway" do
you mean that is already happening, or that it will be happening soon? If this
is already happening, then perhaps we could remove this code. What's the plan
for iOS + Chrome? Will that be switching to BoringSSL too?

                         kKeySize,
                         kAuthTagSize,
                         kNoncePrefixSize) {
   static_assert(kKeySize <= kMaxKeySize, "key size too big");
   static_assert(kNoncePrefixSize <= kMaxNoncePrefixSize,
                 "nonce prefix size too big");
 }
 
 ChaCha20Poly1305Decrypter::~ChaCha20Poly1305Decrypter() {}
 
 void ChaCha20Poly1305Decrypter::FillAeadParams(StringPiece nonce,
                                                StringPiece associated_data,
                                                size_t auth_tag_size,
                                                AeadParams* aead_params) const {
   aead_params->len = sizeof(aead_params->data.nss_aead_params);
   CK_NSS_AEAD_PARAMS* nss_aead_params = &aead_params->data.nss_aead_params;
-  nss_aead_params->pIv =
+  nss_aead_params->pNonce =
       reinterpret_cast<CK_BYTE*>(const_cast<char*>(nonce.data()));
-  nss_aead_params->ulIvLen = nonce.size();
+  nss_aead_params->ulNonceLen = nonce.size();

[davidben, 2016/03/30 18:39:25]

This would be contingent on a src/third_party/nss DEPS roll, right? Not a
src/net/third_party/nss update. So I think this file's changes want to be in the
CL that updates DEPS.

[Ryan Sleevi, 2016/03/31 02:52:58]

Correct. Sorry, I didn't upload the DEPS bit yet, because the two had to be kept
in lock-step to run unittests, and this was the more time consuming one.

[davidben, 2016/03/31 05:23:00]

Got it.

   nss_aead_params->pAAD =
       reinterpret_cast<CK_BYTE*>(const_cast<char*>(associated_data.data()));
   nss_aead_params->ulAADLen = associated_data.size();
   nss_aead_params->ulTagLen = auth_tag_size;
 }
 
 const char* ChaCha20Poly1305Decrypter::cipher_name() const {
   // TODO(rtenneti): Use TLS1_TXT_ECDHE_RSA_WITH_CHACHA20_POLY1305 instead of
   // hard coded string.
   // return TLS1_TXT_ECDHE_RSA_WITH_CHACHA20_POLY1305;
   return "ECDHE-RSA-CHACHA20-POLY1305";
 }
 
 uint32_t ChaCha20Poly1305Decrypter::cipher_id() const {
   // TODO(rtenneti): when Chromium requires NSS 3.15.2 or later, use
   // TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305 instead of 0xCC13.
   // "OR" 0x03000000 to match OpenSSL/BoringSSL implementations.
   return 0x03000000 | 0xCC13;
 }
 
 }  // namespace net