Move Window's specific logic for disabling ECDSA from QuicCryptoyclientConfig

Move Window's specific logic for disabling ECDSA from QuicCryptoyclientConfig
to QuicStreamFactory.

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

[Ryan Hamilton, 2014-03-23 02:29:38]

One more bit of cleanup

[wtc, 2014-03-25 14:58:10]

Patch set 1 LGTM.

https://codereview.chromium.org/209413005/diff/1/net/quic/crypto/quic_crypto_...
File net/quic/crypto/quic_crypto_client_config.cc (left):

https://codereview.chromium.org/209413005/diff/1/net/quic/crypto/quic_crypto_...
net/quic/crypto/quic_crypto_client_config.cc:293: // certificates.

We should move this comment to either quic_crypto_client_config.h or
quic_stream_factory.cc. For example, in quic_crypto_client_config.h we can say:

  // Call this method on platforms that do not support ECDSA
  // certificates to not request ECDSA proofs.

("to not request ECDSA proofs" sounds awkward to me. Maybe we should
remove it.)

We may also want to add a reminder that this method can be removed
when we drop support for Windows XP.

https://codereview.chromium.org/209413005/diff/1/net/quic/crypto/quic_crypto_...
File net/quic/crypto/quic_crypto_client_config.cc (right):

https://codereview.chromium.org/209413005/diff/1/net/quic/crypto/quic_crypto_...
net/quic/crypto/quic_crypto_client_config.cc:716: void
QuicCryptoClientConfig::DisableEcdsa() {

This should follow the definition of AddCanonicalSuffix() to match their
declaration order in the header file.

https://codereview.chromium.org/209413005/diff/1/net/quic/crypto/quic_crypto_...
File net/quic/crypto/quic_crypto_client_config.h (right):

https://codereview.chromium.org/209413005/diff/1/net/quic/crypto/quic_crypto_...
net/quic/crypto/quic_crypto_client_config.h:227: // Disables the use of ECDSA
for certificate validation.

Nit: we should refer to this as "proof validation" or "proof verification". The
proof can also be an ECDSA signature, so the lack of ECDSA support affects more
than certificate verification.

[Ryan Hamilton, 2014-03-27 19:06:39]

https://codereview.chromium.org/209413005/diff/1/net/quic/crypto/quic_crypto_...
File net/quic/crypto/quic_crypto_client_config.cc (left):

https://codereview.chromium.org/209413005/diff/1/net/quic/crypto/quic_crypto_...
net/quic/crypto/quic_crypto_client_config.cc:293: // certificates.
On 2014/03/25 14:58:11, wtc wrote:
> 
> We should move this comment to either quic_crypto_client_config.h or
> quic_stream_factory.cc. For example, in quic_crypto_client_config.h we can
say:
> 
>   // Call this method on platforms that do not support ECDSA
>   // certificates to not request ECDSA proofs.
> 
> ("to not request ECDSA proofs" sounds awkward to me. Maybe we should
> remove it.)
> 
> We may also want to add a reminder that this method can be removed
> when we drop support for Windows XP.

Done.

https://codereview.chromium.org/209413005/diff/1/net/quic/crypto/quic_crypto_...
File net/quic/crypto/quic_crypto_client_config.cc (right):

https://codereview.chromium.org/209413005/diff/1/net/quic/crypto/quic_crypto_...
net/quic/crypto/quic_crypto_client_config.cc:716: void
QuicCryptoClientConfig::DisableEcdsa() {
On 2014/03/25 14:58:11, wtc wrote:
> 
> This should follow the definition of AddCanonicalSuffix() to match their
> declaration order in the header file.

Done.

https://codereview.chromium.org/209413005/diff/1/net/quic/crypto/quic_crypto_...
File net/quic/crypto/quic_crypto_client_config.h (right):

https://codereview.chromium.org/209413005/diff/1/net/quic/crypto/quic_crypto_...
net/quic/crypto/quic_crypto_client_config.h:227: // Disables the use of ECDSA
for certificate validation.
On 2014/03/25 14:58:11, wtc wrote:
> 
> Nit: we should refer to this as "proof validation" or "proof verification".
The
> proof can also be an ECDSA signature, so the lack of ECDSA support affects
more
> than certificate verification.

Done.

[Ryan Hamilton, 2014-03-27 19:06:44]

The CQ bit was checked by rch@chromium.org

[commit-bot: I haz the power, 2014-03-27 19:11:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rch@chromium.org/209413005/50001

[wtc, 2014-03-27 19:40:42]

Patch set 3 LGTM.

[commit-bot: I haz the power, 2014-03-27 22:21:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-27 22:21:08]

Retried try job too often on win_rel for step(s) content_browsertests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[Ryan Hamilton, 2014-03-27 22:27:19]

The CQ bit was checked by rch@chromium.org

[commit-bot: I haz the power, 2014-03-27 22:29:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rch@chromium.org/209413005/50001

[commit-bot: I haz the power, 2014-03-28 04:48:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-28 04:48:18]

Retried try job too often on win_rel for step(s) content_browsertests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[Ryan Hamilton, 2014-03-28 14:54:24]

The CQ bit was checked by rch@chromium.org

[commit-bot: I haz the power, 2014-03-28 14:55:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rch@chromium.org/209413005/50001

[commit-bot: I haz the power, 2014-03-28 21:46:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-28 21:46:11]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[Ryan Hamilton, 2014-03-28 21:47:45]

The CQ bit was checked by rch@chromium.org

[commit-bot: I haz the power, 2014-03-28 21:53:21]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rch@chromium.org/209413005/50001

[commit-bot: I haz the power, 2014-03-28 21:57:34]

Change committed as 260295