From c493f7233e12569bbd6186c387b132e274a27d51 Mon Sep 17 00:00:00 2001 From: "mark a. foltz" Date: Thu, 8 Jul 2021 13:24:29 -0700 Subject: [Open Screen] Capture error messages in cast_auth_util_internal.cc. This converts DVLOGs which were removed in 3001340 to messages passed along with the Error object returned by functions in cast_auth_util_internal.cc. It then propagates the messages via the wrapped Error returned by VerifyCredentialsImpl(). Bug: b/159172782 Change-Id: I2a2b801aeaec71648ff195f7e917d40574ae05f8 Reviewed-on: https://chromium-review.googlesource.com/c/openscreen/+/3012114 Commit-Queue: mark a. foltz Reviewed-by: Jordan Bayles --- .../certificate/cast_cert_validator_internal.cc | 36 ++++++++++++----- cast/sender/BUILD.gn | 5 +-- cast/sender/channel/cast_auth_util.cc | 47 ++++++++++++++-------- 3 files changed, 60 insertions(+), 28 deletions(-) (limited to 'cast') diff --git a/cast/common/certificate/cast_cert_validator_internal.cc b/cast/common/certificate/cast_cert_validator_internal.cc index 7bdcacca..073b76ac 100644 --- a/cast/common/certificate/cast_cert_validator_internal.cc +++ b/cast/common/certificate/cast_cert_validator_internal.cc @@ -18,6 +18,7 @@ #include #include +#include "absl/strings/str_cat.h" #include "cast/common/certificate/types.h" #include "util/crypto/pem_helpers.h" #include "util/osp_logging.h" @@ -407,23 +408,30 @@ Error FindCertificatePath(const std::vector& der_certs, result_path->intermediate_certs; target_cert.reset(ParseX509Der(der_certs[0])); if (!target_cert) { - return Error::Code::kErrCertsParse; + return Error(Error::Code::kErrCertsParse, + "FindCertificatePath: Invalid target certificate"); } for (size_t i = 1; i < der_certs.size(); ++i) { intermediate_certs.emplace_back(ParseX509Der(der_certs[i])); if (!intermediate_certs.back()) { - return Error::Code::kErrCertsParse; + return Error( + Error::Code::kErrCertsParse, + absl::StrCat( + "FindCertificatePath: Failed to parse intermediate certificate ", + i, " of ", der_certs.size())); } } // Basic checks on the target certificate. - Error::Code error = VerifyCertTime(target_cert.get(), time); - if (error != Error::Code::kNone) { - return error; + Error::Code valid_time = VerifyCertTime(target_cert.get(), time); + if (valid_time != Error::Code::kNone) { + return Error(valid_time, + "FindCertificatePath: Failed to verify certificate time"); } bssl::UniquePtr public_key{X509_get_pubkey(target_cert.get())}; if (!VerifyPublicKeyLength(public_key.get())) { - return Error::Code::kErrCertsVerifyGeneric; + return Error(Error::Code::kErrCertsVerifyGeneric, + "FindCertificatePath: Failed with invalid public key length"); } const X509_ALGOR* sig_alg; X509_get0_signature(nullptr, &sig_alg, target_cert.get()); @@ -432,12 +440,14 @@ Error FindCertificatePath(const std::vector& der_certs, } bssl::UniquePtr key_usage = GetKeyUsage(target_cert.get()); if (!key_usage) { - return Error::Code::kErrCertsRestrictions; + return Error(Error::Code::kErrCertsRestrictions, + "FindCertificatePath: Failed with no key usage"); } int bit = ASN1_BIT_STRING_get_bit(key_usage.get(), KeyUsageBits::kDigitalSignature); if (bit == 0) { - return Error::Code::kErrCertsRestrictions; + return Error(Error::Code::kErrCertsRestrictions, + "FindCertificatePath: Failed to get digital signature"); } X509* path_head = target_cert.get(); @@ -470,6 +480,8 @@ Error FindCertificatePath(const std::vector& der_certs, Error::Code last_error = Error::Code::kNone; for (;;) { X509_NAME* target_issuer_name = X509_get_issuer_name(path_head); + OSP_VLOG << "FindCertificatePath: Target certificate issuer name: " + << X509_NAME_oneline(target_issuer_name, 0, 0); // The next issuer certificate to add to the current path. X509* next_issuer = nullptr; @@ -478,6 +490,8 @@ Error FindCertificatePath(const std::vector& der_certs, X509* trust_store_cert = trust_store->certs[i].get(); X509_NAME* trust_store_cert_name = X509_get_subject_name(trust_store_cert); + OSP_VLOG << "FindCertificatePath: Trust store certificate issuer name: " + << X509_NAME_oneline(trust_store_cert_name, 0, 0); if (X509_NAME_cmp(trust_store_cert_name, target_issuer_name) == 0) { CertPathStep& next_step = path[--path_index]; next_step.cert = trust_store_cert; @@ -512,7 +526,9 @@ Error FindCertificatePath(const std::vector& der_certs, if (path_index == first_index) { // There are no more paths to try. Ensure an error is returned. if (last_error == Error::Code::kNone) { - return Error::Code::kErrCertsVerifyUntrustedCert; + return Error(Error::Code::kErrCertsVerifyUntrustedCert, + "FindCertificatePath: Failed after trying all " + "certificate paths, no matches"); } return last_error; } else { @@ -542,6 +558,8 @@ Error FindCertificatePath(const std::vector& der_certs, result_path->path.push_back(path[i].cert); } + OSP_VLOG + << "FindCertificatePath: Succeeded at validating receiver certificates"; return Error::Code::kNone; } diff --git a/cast/sender/BUILD.gn b/cast/sender/BUILD.gn index ae17f7cf..09e39453 100644 --- a/cast/sender/BUILD.gn +++ b/cast/sender/BUILD.gn @@ -13,6 +13,7 @@ source_set("channel") { ] deps = [ + "../../third_party/abseil", "../common:channel", "../common/certificate/proto:certificate_proto", "../common/channel/proto:channel_proto", @@ -65,9 +66,7 @@ source_set("test_helpers") { "../receiver:channel", ] - public_deps = [ - ":channel", - ] + public_deps = [ ":channel" ] } source_set("unittests") { diff --git a/cast/sender/channel/cast_auth_util.cc b/cast/sender/channel/cast_auth_util.cc index f76a1dd8..6e9cf626 100644 --- a/cast/sender/channel/cast_auth_util.cc +++ b/cast/sender/channel/cast_auth_util.cc @@ -9,6 +9,7 @@ #include #include +#include "absl/strings/str_cat.h" #include "cast/common/certificate/cast_cert_validator.h" #include "cast/common/certificate/cast_cert_validator_internal.h" #include "cast/common/certificate/cast_crl.h" @@ -104,41 +105,55 @@ class CastNonce { std::chrono::seconds nonce_generation_time_; }; -// Maps Error::Code from certificate verification to Error. -// If crl_required is set to false, all revocation related errors are ignored. -Error MapToOpenscreenError(Error::Code error, bool crl_required) { - switch (error) { +// Maps an error from certificate verification to an error reported to the +// library client. If crl_required is set to false, all revocation related +// errors are ignored. +// +// TODO(https://issuetracker.google.com/issues/193164666): It would be simpler +// to just pass the underlying verification error directly to the client. +Error MapToOpenscreenError(Error verify_error, bool crl_required) { + switch (verify_error.code()) { case Error::Code::kErrCertsMissing: return Error(Error::Code::kCastV2PeerCertEmpty, - "Failed to locate certificates."); + absl::StrCat("Failed to locate certificates: ", + verify_error.message())); case Error::Code::kErrCertsParse: return Error(Error::Code::kErrCertsParse, - "Failed to parse certificates."); + absl::StrCat("Failed to parse certificates: ", + verify_error.message())); case Error::Code::kErrCertsDateInvalid: - return Error(Error::Code::kCastV2CertNotSignedByTrustedCa, - "Failed date validity check."); + return Error( + Error::Code::kCastV2CertNotSignedByTrustedCa, + absl::StrCat("Failed date validity check: ", verify_error.message())); case Error::Code::kErrCertsVerifyGeneric: - return Error(Error::Code::kCastV2CertNotSignedByTrustedCa, - "Failed with a generic certificate verification error."); + return Error( + Error::Code::kCastV2CertNotSignedByTrustedCa, + absl::StrCat("Failed with a generic certificate verification error: ", + verify_error.message())); case Error::Code::kErrCertsRestrictions: return Error(Error::Code::kCastV2CertNotSignedByTrustedCa, - "Failed certificate restrictions."); + absl::StrCat("Failed certificate restrictions: ", + verify_error.message())); case Error::Code::kErrCertsVerifyUntrustedCert: return Error(Error::Code::kCastV2CertNotSignedByTrustedCa, - "Failed with untrusted certificate."); + absl::StrCat("Failed with untrusted certificate: ", + verify_error.message())); case Error::Code::kErrCrlInvalid: // This error is only encountered if |crl_required| is true. OSP_DCHECK(crl_required); return Error(Error::Code::kErrCrlInvalid, - "Failed to provide a valid CRL."); + absl::StrCat("Failed to provide a valid CRL: ", + verify_error.message())); case Error::Code::kErrCertsRevoked: return Error(Error::Code::kErrCertsRevoked, - "Failed certificate revocation check."); + absl::StrCat("Failed certificate revocation check: ", + verify_error.message())); case Error::Code::kNone: return Error::None(); default: return Error(Error::Code::kCastV2CertNotSignedByTrustedCa, - "Failed verifying cast device certificate."); + absl::StrCat("Failed verifying cast device certificate: ", + verify_error.message())); } } @@ -355,7 +370,7 @@ ErrorOr VerifyCredentialsImpl( &device_policy, crl.get(), crl_policy, cast_trust_store); // Handle and report errors. - Error result = MapToOpenscreenError(verify_result.code(), + Error result = MapToOpenscreenError(verify_result, crl_policy == CRLPolicy::kCrlRequired); if (!result.ok()) { return result; -- cgit v1.2.3