Implement QUIC key extraction.

Implement QUIC key extraction.

Added a new subkey_secret output to crypto::HKDF which is
saved by the forward-secure key derivation and used for a new
ExportKeyingMaterial method on QuicCryptoStream. This will be used
in Chromium for WebRTC on QUIC.

Generated some tests by making a straightforward alternative
implementation in Python.

Written by Daniel Ziegler.

Merge internal CL: 72073257

R=agl@chromium.org,dmziegler@chromium.org
BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=286738

[wtc, 2014-07-30 02:24:18]

Please review.

Patch set 1 is the original internal CL with merge conflicts resolved and some
StringPiece's qualified with base::. You can diff against patch set 1 to see my
modifications for porting to the Chromium tree.

https://codereview.chromium.org/423333002/diff/40001/crypto/hkdf.h
File crypto/hkdf.h (left):

https://codereview.chromium.org/423333002/diff/40001/crypto/hkdf.h#oldcode11
crypto/hkdf.h:11: #include "base/memory/scoped_ptr.h"

I moved this to the .cc file because the .h file doesn't use scoped_ptr.

https://codereview.chromium.org/423333002/diff/40001/crypto/hkdf.h#oldcode13
crypto/hkdf.h:13: #include "build/build_config.h"

This is not needed.

[agl, 2014-07-30 02:56:29]

Diff from PS 1 LGTM.

(The base code was previously reviewed and I didn't go over it again.)

[dmz, 2014-07-30 17:58:26]

LGTM

https://codereview.chromium.org/423333002/diff/60001/net/quic/test_tools/cryp...
File net/quic/test_tools/crypto_test_utils.cc (right):

https://codereview.chromium.org/423333002/diff/60001/net/quic/test_tools/cryp...
net/quic/test_tools/crypto_test_utils.cc:463: const int kSampleOutputLength =
32;
Do you want to use size_t here like you suggested for me?

[wtc, 2014-07-30 18:05:54]

The CQ bit was checked by wtc@chromium.org

[commit-bot: I haz the power, 2014-07-30 18:07:19]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/wtc@chromium.org/423333002/80001

[wtc, 2014-07-30 18:09:35]

https://codereview.chromium.org/423333002/diff/60001/net/quic/test_tools/cryp...
File net/quic/test_tools/crypto_test_utils.cc (right):

https://codereview.chromium.org/423333002/diff/60001/net/quic/test_tools/cryp...
net/quic/test_tools/crypto_test_utils.cc:463: const int kSampleOutputLength =
32;

On 2014/07/30 17:58:26, dmz wrote:
> Do you want to use size_t here like you suggested for me?

Done. Originally I wanted to see if it caused a compiler warning on any
platform. (The answer seems to be No based on the try bot results.)

[wtc, 2014-07-30 18:16:52]

https://codereview.chromium.org/423333002/diff/80001/crypto/hkdf.h
File crypto/hkdf.h (right):

https://codereview.chromium.org/423333002/diff/80001/crypto/hkdf.h#newcode40
crypto/hkdf.h:40: size_t subkey_secret_bytes_to_generate);

Adam: while porting this change to Chromium, I realized that the HKDF class has
become very QUIC-specific. In the internal QUIC source tree, this class is
inside the quic/crypto/base directory, so a QUIC-specific interface is
acceptable. In Chromium's crypto directory, a QUIC-specific interface seems bad,
although I just noticed the comment on line 17 that says this class generates
output "as needed by QUIC".

I am surprised that I didn't need to change any files outside net/quic. So this
means no other code is using the HKDF class right now.

[agl, 2014-07-30 18:22:09]

On 2014/07/30 18:16:52, wtc wrote:
> https://codereview.chromium.org/423333002/diff/80001/crypto/hkdf.h
> File crypto/hkdf.h (right):
> 
> https://codereview.chromium.org/423333002/diff/80001/crypto/hkdf.h#newcode40
> crypto/hkdf.h:40: size_t subkey_secret_bytes_to_generate);
> 
> Adam: while porting this change to Chromium, I realized that the HKDF class
has
> become very QUIC-specific. In the internal QUIC source tree, this class is
> inside the quic/crypto/base directory, so a QUIC-specific interface is
> acceptable. In Chromium's crypto directory, a QUIC-specific interface seems
bad,
> although I just noticed the comment on line 17 that says this class generates
> output "as needed by QUIC".
> 
> I am surprised that I didn't need to change any files outside net/quic. So
this
> means no other code is using the HKDF class right now.

We could preemptively generalise it, but I would hesitate to do so until we
actually have another user. Moving it inside net/quic/ in Chromium would seem
very reasonable until then however.

[dmz, 2014-07-30 18:25:23]

On 2014/07/30 18:22:09, agl wrote:
> On 2014/07/30 18:16:52, wtc wrote:
> > https://codereview.chromium.org/423333002/diff/80001/crypto/hkdf.h
> > File crypto/hkdf.h (right):
> > 
> > https://codereview.chromium.org/423333002/diff/80001/crypto/hkdf.h#newcode40
> > crypto/hkdf.h:40: size_t subkey_secret_bytes_to_generate);
> > 
> > Adam: while porting this change to Chromium, I realized that the HKDF class
> has
> > become very QUIC-specific. In the internal QUIC source tree, this class is
> > inside the quic/crypto/base directory, so a QUIC-specific interface is
> > acceptable. In Chromium's crypto directory, a QUIC-specific interface seems
> bad,
> > although I just noticed the comment on line 17 that says this class
generates
> > output "as needed by QUIC".
> > 
> > I am surprised that I didn't need to change any files outside net/quic. So
> this
> > means no other code is using the HKDF class right now.
> 
> We could preemptively generalise it, but I would hesitate to do so until we
> actually have another user. Moving it inside net/quic/ in Chromium would seem
> very reasonable until then however.

I noticed this too, actually. I think it might be nice to factor it into two
pieces -- one to do the actual HKDF, and one to extract the right keys for QUIC.
There are several pieces of code that would be better off calling a
non-QUIC-specific HKDF, because all they do is use client_write_key() or
server_write_key() to get |key_bytes_to_generate| HKDF bytes out. See
QuicCryptoServerConfig::DeriveSourceAddressTokenKey or (in this CL)
CryptoUtils::ExportKeyingMaterial. These functions also currently generate twice
as many HKDF output bytes as they actually need.

[commit-bot: I haz the power, 2014-07-31 11:36:40]

Change committed as 286738