Normalize certificate name verification across all platforms

Normalize certificate name verification across all platforms

This brings Linux/ChromeOS, iOS, and Windows in line with the Android/OS X
implementations by using Chromium's internal RFC 6125 name validation
routines, rather than the platform-specific routines.

In particular, this adds support for iPAddress subjectAltName matching
on Windows, ignores trailing dots for dNSNames, and on Windows, removes
support for matching against non-IDNA commonNames when no subjectAltName
is present.

BUG=72726, 91072
R=wtc@chromium.org

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

[Ryan Sleevi, 2013-08-16 00:42:34]

wtc: This just normalizes across all platforms. It also makes sure each of the
CertVerifyProc's are wired up to consistent name matching - full name parsing is
handled by X509CertificateNameVerifyTest.

https://codereview.chromium.org/22893021/diff/3001/net/cert/cert_verify_proc_...
File net/cert/cert_verify_proc_win.cc (right):

https://codereview.chromium.org/22893021/diff/3001/net/cert/cert_verify_proc_...
net/cert/cert_verify_proc_win.cc:731: 0x00001000;  //
SECURITY_FLAG_IGNORE_CERT_CN_INVALID
Note: This is different than CERT_CHAIN_POLICY_IGNORE_INVALID_NAME_FLAG (which
considers name constraints)

See my description of the algorithm in http://crbug.com/72726#c10 to see why I'm
passing this flag.

[wtc, 2013-08-16 19:59:18]

Patch set 2 LGTM.

https://codereview.chromium.org/22893021/diff/2/net/cert/cert_verify_proc_uni...
File net/cert/cert_verify_proc_unittest.cc (right):

https://codereview.chromium.org/22893021/diff/2/net/cert/cert_verify_proc_uni...
net/cert/cert_verify_proc_unittest.cc:1374: { ".test.example", false },  //
Should not match the dNSName SAN

It would be nice to document what subject CN and alternative names the
test certificate has. This will allow a reader to verify these expected
test results are correct.

Could you document what |valid| means? Does it mean "matches vs. doesn't match"?

https://codereview.chromium.org/22893021/diff/2/net/cert/cert_verify_proc_win.cc
File net/cert/cert_verify_proc_win.cc (right):

https://codereview.chromium.org/22893021/diff/2/net/cert/cert_verify_proc_win...
net/cert/cert_verify_proc_win.cc:720: // Flag certificates that have a Subject
common name with a NULL character.

Perhaps this workaround can be removed?

https://codereview.chromium.org/22893021/diff/2/net/cert/cert_verify_proc_win...
net/cert/cert_verify_proc_win.cc:731: 0x00001000;  //
SECURITY_FLAG_IGNORE_CERT_CN_INVALID

Is this macro not defined in Wincrypt.h?

A comment that announces we will check the cert name below
might be helpful. Alternative, we can call cert->VerifyNameMatch(hostname)
first.

https://codereview.chromium.org/22893021/diff/2/net/cert/cert_verify_proc_win...
net/cert/cert_verify_proc_win.cc:767: 

Nit: delete one blank line.

[Ryan Sleevi, 2013-08-16 20:38:27]

https://codereview.chromium.org/22893021/diff/2/net/cert/cert_verify_proc_win.cc
File net/cert/cert_verify_proc_win.cc (right):

https://codereview.chromium.org/22893021/diff/2/net/cert/cert_verify_proc_win...
net/cert/cert_verify_proc_win.cc:720: // Flag certificates that have a Subject
common name with a NULL character.
On 2013/08/16 19:59:19, wtc wrote:
> 
> Perhaps this workaround can be removed?

I think it's still reasonable to bounce such certs as invalid, independent of
any name matching. It's more of a defense in depth thing, and matches other SSL
libraries behaviours.

https://codereview.chromium.org/22893021/diff/2/net/cert/cert_verify_proc_win...
net/cert/cert_verify_proc_win.cc:731: 0x00001000;  //
SECURITY_FLAG_IGNORE_CERT_CN_INVALID
On 2013/08/16 19:59:19, wtc wrote:
> 
> Is this macro not defined in Wincrypt.h?
> 
> A comment that announces we will check the cert name below
> might be helpful. Alternative, we can call cert->VerifyNameMatch(hostname)
> first.

No. The flags here are similar to the flags on the (old) line 769-786, which are
handled by WinHttp/WinInet.h, not Wincrypt

[commit-bot: I haz the power, 2013-08-16 20:38:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsleevi@chromium.org/22893021/22001

[commit-bot: I haz the power, 2013-08-16 20:49:18]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_clan...

[commit-bot: I haz the power, 2013-08-16 20:58:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rsleevi@chromium.org/22893021/22001

[commit-bot: I haz the power, 2013-08-16 23:56:59]

Change committed as 218121