Mark DigiNotar as untrusted

Mark DigiNotar as untrusted

BUG=94673
TEST=Visit https://www.diginotar.com and expect an invalid authority error.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=98741

[wtc, 2011-08-30 00:02:44]

LGTM, with the suggested changes below.

http://codereview.chromium.org/7795014/diff/1/net/base/ev_root_ca_metadata.cc
File net/base/ev_root_ca_metadata.cc (right):

http://codereview.chromium.org/7795014/diff/1/net/base/ev_root_ca_metadata.cc...
net/base/ev_root_ca_metadata.cc:279: "2.16.528.1.1001.1.1.1.12.6.1.1.1",
Please also remove this line from this array.

http://codereview.chromium.org/7795014/diff/1/net/base/x509_certificate.cc
File net/base/x509_certificate.cc (right):

http://codereview.chromium.org/7795014/diff/1/net/base/x509_certificate.cc#ne...
net/base/x509_certificate.cc:596: rv = ERR_CERT_AUTHORITY_INVALID;
This line should say:
    rv = MapCertStatusToNetError(verify_result->cert_status);

MapCertStatusToNetError picks the most serious error from
the error bits in verify_result->cert_status.

This way we won't lose the ERR_CERT_REVOKED error if it's
reported by the VerifyInternal() call.

You should add a comment to explain why you are calling
IsPublicKeyBlacklisted after VerifyInternal, in contrast
to the IsBlacklisted call above.  I believe you want Chrome
to hit the CA's OCSP responder.

http://codereview.chromium.org/7795014/diff/1/net/base/x509_certificate.cc#ne...
net/base/x509_certificate.cc:700: // CN=DigiNotar Root
CA/emailAddress=info@diginotar.nl
Nit: you can probably omit the emailAddress component of the
subject name.  Chrome displays the subject name as follows:

E = info@diginotar.nl
CN = DigiNotar Root CA
O = DigiNotar
C = NL

So the complete subject name also has O= and C= components.

http://codereview.chromium.org/7795014/diff/1/net/base/x509_certificate.cc#ne...
net/base/x509_certificate.cc:702:
0x12,0xce,0x48,0x63,0xe4,0x33,0x43,0x78,0x06,0xa8},
Nit: add a space between the bytes (unless they make the line
longer than 80 characters).

http://codereview.chromium.org/7795014/diff/1/net/base/x509_certificate.cc#ne...
net/base/x509_certificate.cc:707: j = public_key_hashes.begin(); j !=
public_key_hashes.end(); j++) {
Nit: j++ => ++j

Nit: omit the curly braces on line 708 and line 710, around
the one-line if statement.

http://codereview.chromium.org/7795014/diff/1/net/base/x509_certificate.h
File net/base/x509_certificate.h (right):

http://codereview.chromium.org/7795014/diff/1/net/base/x509_certificate.h#new...
net/base/x509_certificate.h:432: // are SHA1 hashes of a SubjectPublicKeyInfo
structure) is explicitly blocked.
Nit: a SubjectPublicKeyInfo structure => SubjectPublicKeyInfo structures ?

[agl, 2011-08-30 00:13:48]

http://codereview.chromium.org/7795014/diff/1/net/base/ev_root_ca_metadata.cc
File net/base/ev_root_ca_metadata.cc (right):

http://codereview.chromium.org/7795014/diff/1/net/base/ev_root_ca_metadata.cc...
net/base/ev_root_ca_metadata.cc:279: "2.16.528.1.1001.1.1.1.12.6.1.1.1",
On 2011/08/30 00:02:44, wtc wrote:
> Please also remove this line from this array.

Done.

http://codereview.chromium.org/7795014/diff/1/net/base/x509_certificate.cc
File net/base/x509_certificate.cc (right):

http://codereview.chromium.org/7795014/diff/1/net/base/x509_certificate.cc#ne...
net/base/x509_certificate.cc:596: rv = ERR_CERT_AUTHORITY_INVALID;
On 2011/08/30 00:02:44, wtc wrote:
> This line should say:
>     rv = MapCertStatusToNetError(verify_result->cert_status);
> 
> MapCertStatusToNetError picks the most serious error from
> the error bits in verify_result->cert_status.
> 
> This way we won't lose the ERR_CERT_REVOKED error if it's
> reported by the VerifyInternal() call.
> 
> You should add a comment to explain why you are calling
> IsPublicKeyBlacklisted after VerifyInternal, in contrast
> to the IsBlacklisted call above.  I believe you want Chrome
> to hit the CA's OCSP responder.

Done.

http://codereview.chromium.org/7795014/diff/1/net/base/x509_certificate.cc#ne...
net/base/x509_certificate.cc:700: // CN=DigiNotar Root
CA/emailAddress=info@diginotar.nl
On 2011/08/30 00:02:44, wtc wrote:
> Nit: you can probably omit the emailAddress component of the
> subject name.  Chrome displays the subject name as follows:
> 
> E = mailto:info@diginotar.nl
> CN = DigiNotar Root CA
> O = DigiNotar
> C = NL
> 
> So the complete subject name also has O= and C= components.

Done.

http://codereview.chromium.org/7795014/diff/1/net/base/x509_certificate.cc#ne...
net/base/x509_certificate.cc:702:
0x12,0xce,0x48,0x63,0xe4,0x33,0x43,0x78,0x06,0xa8},
On 2011/08/30 00:02:44, wtc wrote:
> Nit: add a space between the bytes (unless they make the line
> longer than 80 characters).

Done.

http://codereview.chromium.org/7795014/diff/1/net/base/x509_certificate.cc#ne...
net/base/x509_certificate.cc:707: j = public_key_hashes.begin(); j !=
public_key_hashes.end(); j++) {
On 2011/08/30 00:02:44, wtc wrote:
> Nit: j++ => ++j
> 
> Nit: omit the curly braces on line 708 and line 710, around
> the one-line if statement.

Done.

http://codereview.chromium.org/7795014/diff/1/net/base/x509_certificate.h
File net/base/x509_certificate.h (right):

http://codereview.chromium.org/7795014/diff/1/net/base/x509_certificate.h#new...
net/base/x509_certificate.h:432: // are SHA1 hashes of a SubjectPublicKeyInfo
structure) is explicitly blocked.
On 2011/08/30 00:02:44, wtc wrote:
> Nit: a SubjectPublicKeyInfo structure => SubjectPublicKeyInfo structures ?

Done.

[wtc, 2011-08-30 00:19:23]

LGTM on Patch Set 2.