Chromium Code Reviews| Index: components/cast_certificate/cast_cert_validator.cc |
| diff --git a/components/cast_certificate/cast_cert_validator.cc b/components/cast_certificate/cast_cert_validator.cc |
| index ccddd3757590f8add4ee4f4fa0ff1285ae85f9fa..f91d108a846a660cb1c3547107e6c109f913ae86 100644 |
| --- a/components/cast_certificate/cast_cert_validator.cc |
| +++ b/components/cast_certificate/cast_cert_validator.cc |
| @@ -160,19 +160,9 @@ bool GetCommonNameFromSubject(const net::der::Input& subject_tlv, |
| return false; |
| } |
| -// Returns true if the extended key usage list |ekus| contains client auth. |
| -bool HasClientAuth(const std::vector<net::der::Input>& ekus) { |
| - for (const auto& oid : ekus) { |
| - if (oid == net::ClientAuth()) |
| - return true; |
| - } |
| - return false; |
| -} |
| - |
| // Checks properties on the target certificate. |
| // |
| // * The Key Usage must include Digital Signature |
| -// * The Extended Key Usage must include TLS Client Auth |
| // * May have the policy 1.3.6.1.4.1.11129.2.5.2 to indicate it |
| // is an audio-only device. |
| WARN_UNUSED_RESULT bool CheckTargetCertificate( |
| @@ -187,10 +177,27 @@ WARN_UNUSED_RESULT bool CheckTargetCertificate( |
| if (!cert->key_usage().AssertsBit(net::KEY_USAGE_BIT_DIGITAL_SIGNATURE)) |
| return false; |
| - // Ensure Extended Key Usage contains client auth. |
| - if (!cert->has_extended_key_usage() || |
| - !HasClientAuth(cert->extended_key_usage())) |
| - return false; |
| + // TODO(delete before landing): Doug/Cast OWNERs: the behavior after my |
|
ryanchung
2017/04/07 02:02:47
These changes look fine.
I guess the clientAuth EK
|
| + // change is a bit different than what we had before (but more |
| + // standards compliant). I don't think this will cause any problems for |
| + // Cast, but want to be explicit on what changed so you can properly review: |
| + // |
| + // * [less strict] The EKU can now be omitted in the target |
| + // certificate (will be considered a match for clientAuth per RFC |
| + // 5280). Previously the extension needed to be present. |
| + // |
| + // * [less strict] The OID anyExtendedKeyUsage will also be considered a |
| + // match for clientAuth, per RFC 5280. |
| + // |
| + // * [MORE strict] An extended Key Usage on an intermediate |
| + // restricts the EKU on target certificate. This is NOT part of RFC |
| + // 5280, however is the de-facto standard that both CryptoAPI and |
| + // NSS use, and for Web PKI is now part of the Baseline |
| + // Requirements. More details at |
| + // https://wiki.mozilla.org/CA:CertificatePolicyV2.1#Frequently_Asked_Questions |
| + // I don't expect Cast was setting an EKU on intermediates in the |
| + // first place, and if they were hopefully it wasn't inconsistent |
| + // with the end entity certificate. |
| // Check for an optional audio-only policy extension. |
| *policy = CastDeviceCertPolicy::NONE; |
| @@ -281,9 +288,9 @@ bool VerifyDeviceCertUsingCustomTrustStore( |
| if (!net::der::EncodeTimeAsGeneralizedTime(time, &verification_time)) |
| return false; |
| net::CertPathBuilder::Result result; |
| - net::CertPathBuilder path_builder(target_cert.get(), trust_store, |
| - signature_policy.get(), verification_time, |
| - &result); |
| + net::CertPathBuilder path_builder( |
| + target_cert.get(), trust_store, signature_policy.get(), verification_time, |
| + net::KeyPurpose::KEY_PURPOSE_CLIENT_AUTH, &result); |
| path_builder.AddCertIssuerSource(&intermediate_cert_issuer_source); |
| path_builder.Run(); |
| if (!result.HasValidPath()) { |