Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(291)

Unified Diff: components/cast_certificate/cast_cert_validator.cc

Issue 2918233002: Add tests for Cast certificate interpretation of policies. (Closed)
Patch Set: Add more tests, and use less restrictive approach Created 3 years, 6 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | components/cast_certificate/cast_cert_validator_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 8ea624babc4f5ebd83a6024b57e23bc9899c8669..f10080e6153d74a85e5e8a1863fc1d9996a02954 100644
--- a/components/cast_certificate/cast_cert_validator.cc
+++ b/components/cast_certificate/cast_cert_validator.cc
@@ -158,15 +158,48 @@ bool GetCommonNameFromSubject(const net::der::Input& subject_tlv,
return false;
}
+// Cast device certificates use the policy 1.3.6.1.4.1.11129.2.5.2 to indicate
+// it is *restricted* to an audio-only device whereas the absence of a policy
+// means it is unrestricted.
+//
+// This is somewhat different than RFC 5280's notion of policies, so policies
+// are checked separately outside of path building.
+//
+// See the unit-tests VerifyCastDeviceCertTest.Policies* for some
+// concrete examples of how this works.
+void DetermineDeviceCertificatePolicy(
+ const net::CertPathBuilder::ResultPath* result_path,
+ CastDeviceCertPolicy* policy) {
+ // Iterate over all the certificates, including the root certificate. If any
+ // certificate contains the audio-only policy, the whole chain is considered
+ // constrained to audio-only device certificates.
+ //
+ // Policy mappings are not accounted for. The expectation is that top-level
+ // intermediates issued with audio-only will have no mappings. If subsequent
+ // certificates in the chain do, it won't matter as the chain is already
+ // restricted to being audio-only.
+ bool audio_only = false;
+ for (const auto& cert : result_path->path.certs) {
+ if (cert->has_policy_oids()) {
+ const std::vector<net::der::Input>& policies = cert->policy_oids();
+ if (std::find(policies.begin(), policies.end(), AudioOnlyPolicyOid()) !=
+ policies.end()) {
+ audio_only = true;
+ break;
+ }
+ }
+ }
+
+ *policy = audio_only ? CastDeviceCertPolicy::AUDIO_ONLY
+ : CastDeviceCertPolicy::NONE;
+}
+
// Checks properties on the target certificate.
//
// * The Key Usage must include Digital Signature
-// * 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(
const net::ParsedCertificate* cert,
- std::unique_ptr<CertVerificationContext>* context,
- CastDeviceCertPolicy* policy) {
+ std::unique_ptr<CertVerificationContext>* context) {
// Get the Key Usage extension.
if (!cert->has_key_usage())
return false;
@@ -175,22 +208,6 @@ WARN_UNUSED_RESULT bool CheckTargetCertificate(
if (!cert->key_usage().AssertsBit(net::KEY_USAGE_BIT_DIGITAL_SIGNATURE))
return false;
- // Check for an optional audio-only policy extension.
- //
- // TODO(eroman): Use |user_constrained_policy_set| that was output from
- // verification instead. (Checking just the leaf certificate's policy
- // assertion doesn't take into account policy restrictions on intermediates,
- // policy constraints/inhibits, or policy re-mappings).
- *policy = CastDeviceCertPolicy::NONE;
- if (cert->has_policy_oids()) {
- const std::vector<net::der::Input>& policies = cert->policy_oids();
- // Look for an audio-only policy. Disregard any other policy found.
- if (std::find(policies.begin(), policies.end(), AudioOnlyPolicyOid()) !=
- policies.end()) {
- *policy = CastDeviceCertPolicy::AUDIO_ONLY;
- }
- }
-
// Get the Common Name for the certificate.
std::string common_name;
if (!GetCommonNameFromSubject(cert->tbs().subject_tlv, &common_name))
@@ -281,9 +298,13 @@ bool VerifyDeviceCertUsingCustomTrustStore(
return false;
}
- // Check properties of the leaf certificate (key usage, policy), and construct
- // a CertVerificationContext that uses its public key.
- if (!CheckTargetCertificate(target_cert.get(), context, policy))
+ // Determine whether this device certificate is restricted to audio-only.
+ DetermineDeviceCertificatePolicy(result.GetBestValidPath(), policy);
+
+ // Check properties of the leaf certificate not already verified by path
+ // building (key usage), and construct a CertVerificationContext that uses
+ // its public key.
+ if (!CheckTargetCertificate(target_cert.get(), context))
return false;
// Check if a CRL is available.
« no previous file with comments | « no previous file | components/cast_certificate/cast_cert_validator_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698