Don't expire certs used as channel IDs.

Don't expire certs used as channel IDs.


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

[thaidn_google, 2013-03-28 00:57:56]

Matt,

This is the CL that Dirk said in the email. I've removed the expiration check. I
could do more clean up on the cert store as well, but I guess we still want to
store the expiration date somewhere just in case we want to revert this CL and
enforce it again.

[mattm, 2013-03-29 02:59:48]

ALSO: create bug for this at crbug.com and put the bug number in BUG= line.

https://codereview.chromium.org/13130004/diff/1/net/socket/ssl_client_socket_...
File net/socket/ssl_client_socket_nss.cc (left):

https://codereview.chromium.org/13130004/diff/1/net/socket/ssl_client_socket_...
net/socket/ssl_client_socket_nss.cc:1002: DVLOG(1) << "System time is weird, not
enabling channel ID.";
Can not remove this (and associated stuff), since the code still uses the system
time when creating the cert in server_bound_cert_service.cc:GenerateCert.

https://codereview.chromium.org/13130004/diff/1/net/socket/ssl_client_socket_...
File net/socket/ssl_client_socket_nss.cc (right):

https://codereview.chromium.org/13130004/diff/1/net/socket/ssl_client_socket_...
net/socket/ssl_client_socket_nss.cc:2544: CLIENT_NO_SERVER_BOUND_CERT_SERVICE =
4,
see comment above ("Since this enum is used for a histogram, do not change or
re-use values.")

[thaidn_google, 2013-03-29 18:48:00]

Hi Matt,

I've made the changes that you want. Please review again. Thanks!

https://codereview.chromium.org/13130004/diff/1/net/socket/ssl_client_socket_...
File net/socket/ssl_client_socket_nss.cc (left):

https://codereview.chromium.org/13130004/diff/1/net/socket/ssl_client_socket_...
net/socket/ssl_client_socket_nss.cc:1002: DVLOG(1) << "System time is weird, not
enabling channel ID.";
On 2013/03/29 02:59:48, mattm wrote:
> Can not remove this (and associated stuff), since the code still uses the
system
> time when creating the cert in server_bound_cert_service.cc:GenerateCert.

Done.

https://codereview.chromium.org/13130004/diff/1/net/socket/ssl_client_socket_...
File net/socket/ssl_client_socket_nss.cc (right):

https://codereview.chromium.org/13130004/diff/1/net/socket/ssl_client_socket_...
net/socket/ssl_client_socket_nss.cc:2544: CLIENT_NO_SERVER_BOUND_CERT_SERVICE =
4,
On 2013/03/29 02:59:48, mattm wrote:
> see comment above ("Since this enum is used for a histogram, do not change or
> re-use values.")

Done.

[mattm, 2013-04-01 23:03:12]

one nit, but lgtm.

https://codereview.chromium.org/13130004/diff/7001/net/ssl/server_bound_cert_...
File net/ssl/server_bound_cert_service.cc (right):

https://codereview.chromium.org/13130004/diff/7001/net/ssl/server_bound_cert_...
net/ssl/server_bound_cert_service.cc:505: if (type &&
IsSupportedCertType(*type)) {
Can remove the "type && " check, since it is already assumed valid when we
passed it to GetServerBoundCert.  (If you really wanted to, you could add it to
the invalid argument check at the start of the function)

[thaidn_google, 2013-04-01 23:56:14]

Thanks, Matt. Could you please commit this CL for me?

https://codereview.chromium.org/13130004/diff/7001/net/ssl/server_bound_cert_...
File net/ssl/server_bound_cert_service.cc (right):

https://codereview.chromium.org/13130004/diff/7001/net/ssl/server_bound_cert_...
net/ssl/server_bound_cert_service.cc:505: if (type &&
IsSupportedCertType(*type)) {
On 2013/04/01 23:03:12, mattm wrote:
> Can remove the "type && " check, since it is already assumed valid when we
> passed it to GetServerBoundCert.  (If you really wanted to, you could add it
to
> the invalid argument check at the start of the function)

Done.

[commit-bot: I haz the power, 2013-04-02 01:44:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/thaidn@google.com/13130004/12001

[commit-bot: I haz the power, 2013-04-02 04:58:38]

Change committed as 191775