Implement ChannelIDVerifier using NSS.

Implement ChannelIDVerifier using NSS.

Add known answer tests for ChannelIDVerifier. This requires adding a
ChannelIDVerifier::VerifyRaw method that can verify a standard signature
(as opposed to a ChannelID signature) because I only have test vectors
for standard ECDSA signatures.

Disable the QuicCryptoServerStreamTest.ChannelID test.

R=agl@chromium.org,rtenneti@chromium.org
BUG=none
TEST=net_unittests --gtest_filter=ChannelIDTest.VerifyKnownAnswers

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

[agl, 2013-06-12 15:29:57]

lgtm

https://codereview.chromium.org/16638016/diff/1/net/quic/crypto/channel_id_ns...
File net/quic/crypto/channel_id_nss.cc (right):

https://codereview.chromium.org/16638016/diff/1/net/quic/crypto/channel_id_ns...
net/quic/crypto/channel_id_nss.cc:81: SECStatus rv = PK11_Verify(&public_key,
&signature_item, &hash_item, NULL);
nit: seems that these two lines could be merged although, if you think that the
line breaking is worse, no worries.

[wtc, 2013-06-12 18:11:00]

https://codereview.chromium.org/16638016/diff/1/net/quic/crypto/channel_id_ns...
File net/quic/crypto/channel_id_nss.cc (right):

https://codereview.chromium.org/16638016/diff/1/net/quic/crypto/channel_id_ns...
net/quic/crypto/channel_id_nss.cc:81: SECStatus rv = PK11_Verify(&public_key,
&signature_item, &hash_item, NULL);

On 2013/06/12 15:29:57, agl wrote:
> nit: seems that these two lines could be merged although, if you think that
the
> line breaking is worse, no worries.

Done.

[commit-bot: I haz the power, 2013-06-12 18:11:18]

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

[commit-bot: I haz the power, 2013-06-12 21:20:55]

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

[commit-bot: I haz the power, 2013-06-12 21:58:48]

Retried try job too often on ios_dbg_simulator for step(s) net_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=ios_dbg_si...

[wtc, 2013-06-12 23:48:21]

rtenneti: could you please review my new change to
quic_crypto_server_stream_test.cc? Thanks.

https://codereview.chromium.org/16638016/diff/49001/net/quic/quic_crypto_serv...
File net/quic/quic_crypto_server_stream_test.cc (left):

https://codereview.chromium.org/16638016/diff/49001/net/quic/quic_crypto_serv...
net/quic/quic_crypto_server_stream_test.cc:247: // TODO(rtenneti): Enable
testing of ProofVerifier.

This test does not verify certs, so we don't need to change it when
we enable testing of ProofVerifier. Therefore I think this TODO
comment can be removed.

https://codereview.chromium.org/16638016/diff/49001/net/quic/quic_crypto_serv...
File net/quic/quic_crypto_server_stream_test.cc (right):

https://codereview.chromium.org/16638016/diff/49001/net/quic/quic_crypto_serv...
net/quic/quic_crypto_server_stream_test.cc:261: EXPECT_EQ(2,
CompleteCryptoHandshake());

With a real implementation of ChannelIDVerifier::Verify, we get a CHECK
failure deep inside CompleteCryptoHandshake(). (The mock implementation
of ChannelIDVerifier::Verify() always returns true.) So I think we should
simply disable this test.

[ramant (doing other things), 2013-06-12 23:53:24]

lgtm

https://codereview.chromium.org/16638016/diff/49001/net/quic/quic_crypto_serv...
File net/quic/quic_crypto_server_stream_test.cc (left):

https://codereview.chromium.org/16638016/diff/49001/net/quic/quic_crypto_serv...
net/quic/quic_crypto_server_stream_test.cc:247: // TODO(rtenneti): Enable
testing of ProofVerifier.
On 2013/06/12 23:48:21, wtc wrote:
> 
> This test does not verify certs, so we don't need to change it when
> we enable testing of ProofVerifier. Therefore I think this TODO
> comment can be removed.

Sounds great.

https://codereview.chromium.org/16638016/diff/49001/net/quic/quic_crypto_serv...
File net/quic/quic_crypto_server_stream_test.cc (right):

https://codereview.chromium.org/16638016/diff/49001/net/quic/quic_crypto_serv...
net/quic/quic_crypto_server_stream_test.cc:261: EXPECT_EQ(2,
CompleteCryptoHandshake());
On 2013/06/12 23:48:21, wtc wrote:
> 
> With a real implementation of ChannelIDVerifier::Verify, we get a CHECK
> failure deep inside CompleteCryptoHandshake(). (The mock implementation
> of ChannelIDVerifier::Verify() always returns true.) So I think we should
> simply disable this test.

This changes sounds good to me. My pending CL has a similar change.

[commit-bot: I haz the power, 2013-06-13 00:31:29]

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

[commit-bot: I haz the power, 2013-06-13 02:57:52]

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

[commit-bot: I haz the power, 2013-06-13 07:25:49]

Sorry for I got bad news for ya.
Compile failed with a clobber build on mac_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-06-14 17:17:46]

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

[commit-bot: I haz the power, 2013-06-14 20:26:51]

Change committed as 206486