Use InsertBraces - and reformat pki as such
Bug: 659
Change-Id: I48eeda0bcd0de45d70644c321138225f83cb6c60
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64107
Reviewed-by: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: Bob Beck <bbe@google.com>
diff --git a/.clang-format b/.clang-format
index 33b77f9..4ab7f98 100644
--- a/.clang-format
+++ b/.clang-format
@@ -8,6 +8,7 @@
# IncludeCategories does not recognize <openssl/header.h>. We should
# reconfigure IncludeCategories to match. For now, keep it at Preserve.
IncludeBlocks: Preserve
+InsertBraces: true
TypenameMacros: ['LHASH_OF', 'STACK_OF']
StatementMacros:
- "DECLARE_ASN1_ALLOC_FUNCTIONS"
diff --git a/pki/asn1_util.cc b/pki/asn1_util.cc
index 4e34412..3c9cef7 100644
--- a/pki/asn1_util.cc
+++ b/pki/asn1_util.cc
@@ -36,15 +36,18 @@
der::Parser parser(in);
der::Parser certificate;
- if (!parser.ReadSequence(&certificate))
+ if (!parser.ReadSequence(&certificate)) {
return false;
+ }
// We don't allow junk after the certificate.
- if (parser.HasMore())
+ if (parser.HasMore()) {
return false;
+ }
- if (!certificate.ReadSequence(tbs_certificate))
+ if (!certificate.ReadSequence(tbs_certificate)) {
return false;
+ }
bool unused;
if (!tbs_certificate->SkipOptionalTag(
@@ -53,17 +56,21 @@
}
// serialNumber
- if (!tbs_certificate->SkipTag(der::kInteger))
+ if (!tbs_certificate->SkipTag(der::kInteger)) {
return false;
+ }
// signature
- if (!tbs_certificate->SkipTag(der::kSequence))
+ if (!tbs_certificate->SkipTag(der::kSequence)) {
return false;
+ }
// issuer
- if (!tbs_certificate->SkipTag(der::kSequence))
+ if (!tbs_certificate->SkipTag(der::kSequence)) {
return false;
+ }
// validity
- if (!tbs_certificate->SkipTag(der::kSequence))
+ if (!tbs_certificate->SkipTag(der::kSequence)) {
return false;
+ }
return true;
}
@@ -89,8 +96,9 @@
der::Parser *extensions_parser) {
bool present;
der::Parser tbs_cert_parser;
- if (!SeekToSPKI(in, &tbs_cert_parser))
+ if (!SeekToSPKI(in, &tbs_cert_parser)) {
return false;
+ }
// From RFC 5280, section 4.1
// TBSCertificate ::= SEQUENCE {
@@ -101,8 +109,9 @@
// extensions [3] EXPLICIT Extensions OPTIONAL }
// subjectPublicKeyInfo
- if (!tbs_cert_parser.SkipTag(der::kSequence))
+ if (!tbs_cert_parser.SkipTag(der::kSequence)) {
return false;
+ }
// issuerUniqueID
if (!tbs_cert_parser.SkipOptionalTag(der::kTagContextSpecific | 1,
&present)) {
@@ -134,11 +143,13 @@
// |extensions| was EXPLICITly tagged, so we still need to remove the
// ASN.1 SEQUENCE header.
der::Parser explicit_extensions_parser(extensions.value());
- if (!explicit_extensions_parser.ReadSequence(extensions_parser))
+ if (!explicit_extensions_parser.ReadSequence(extensions_parser)) {
return false;
+ }
- if (explicit_extensions_parser.HasMore())
+ if (explicit_extensions_parser.HasMore()) {
return false;
+ }
*extensions_present = true;
return true;
@@ -154,8 +165,9 @@
ParsedExtension *out_extension) {
der::Parser extensions;
bool extensions_present;
- if (!SeekToExtensions(der::Input(cert), &extensions_present, &extensions))
+ if (!SeekToExtensions(der::Input(cert), &extensions_present, &extensions)) {
return false;
+ }
if (!extensions_present) {
*out_extension_present = false;
return true;
@@ -183,22 +195,26 @@
bool ExtractSubjectFromDERCert(std::string_view cert,
std::string_view *subject_out) {
der::Parser parser;
- if (!SeekToSubject(der::Input(cert), &parser))
+ if (!SeekToSubject(der::Input(cert), &parser)) {
return false;
+ }
der::Input subject;
- if (!parser.ReadRawTLV(&subject))
+ if (!parser.ReadRawTLV(&subject)) {
return false;
+ }
*subject_out = subject.AsStringView();
return true;
}
bool ExtractSPKIFromDERCert(std::string_view cert, std::string_view *spki_out) {
der::Parser parser;
- if (!SeekToSPKI(der::Input(cert), &parser))
+ if (!SeekToSPKI(der::Input(cert), &parser)) {
return false;
+ }
der::Input spki;
- if (!parser.ReadRawTLV(&spki))
+ if (!parser.ReadRawTLV(&spki)) {
return false;
+ }
*spki_out = spki.AsStringView();
return true;
}
@@ -217,17 +233,20 @@
// Step into SubjectPublicKeyInfo sequence.
der::Parser parser((der::Input(spki)));
der::Parser spki_parser;
- if (!parser.ReadSequence(&spki_parser))
+ if (!parser.ReadSequence(&spki_parser)) {
return false;
+ }
// Step over algorithm field (a SEQUENCE).
- if (!spki_parser.SkipTag(der::kSequence))
+ if (!spki_parser.SkipTag(der::kSequence)) {
return false;
+ }
// Extract the subjectPublicKey field.
der::Input spk;
- if (!spki_parser.ReadTag(der::kBitString, &spk))
+ if (!spki_parser.ReadTag(der::kBitString, &spk)) {
return false;
+ }
*spk_out = spk.AsStringView();
return true;
}
@@ -273,12 +292,14 @@
der::Parser parser((der::Input(cert)));
der::Parser certificate;
- if (!parser.ReadSequence(&certificate))
+ if (!parser.ReadSequence(&certificate)) {
return false;
+ }
der::Parser tbs_certificate;
- if (!certificate.ReadSequence(&tbs_certificate))
+ if (!certificate.ReadSequence(&tbs_certificate)) {
return false;
+ }
bool unused;
if (!tbs_certificate.SkipOptionalTag(
@@ -287,16 +308,19 @@
}
// serialNumber
- if (!tbs_certificate.SkipTag(der::kInteger))
+ if (!tbs_certificate.SkipTag(der::kInteger)) {
return false;
+ }
// signature
der::Input tbs_algorithm;
- if (!tbs_certificate.ReadRawTLV(&tbs_algorithm))
+ if (!tbs_certificate.ReadRawTLV(&tbs_algorithm)) {
return false;
+ }
der::Input cert_algorithm;
- if (!certificate.ReadRawTLV(&cert_algorithm))
+ if (!certificate.ReadRawTLV(&cert_algorithm)) {
return false;
+ }
*cert_signature_algorithm_sequence = cert_algorithm.AsStringView();
*tbs_signature_algorithm_sequence = tbs_algorithm.AsStringView();
@@ -314,10 +338,12 @@
ParsedExtension extension;
if (!ExtractExtensionWithOID(cert, der::Input(extension_oid),
- out_extension_present, &extension))
+ out_extension_present, &extension)) {
return false;
- if (!*out_extension_present)
+ }
+ if (!*out_extension_present) {
return true;
+ }
*out_extension_critical = extension.critical;
*out_contents = extension.value.AsStringView();
diff --git a/pki/cert_errors.cc b/pki/cert_errors.cc
index 80f28d0..da95b49 100644
--- a/pki/cert_errors.cc
+++ b/pki/cert_errors.cc
@@ -52,8 +52,9 @@
result += CertErrorIdToDebugString(id);
result += +"\n";
- if (params)
+ if (params) {
AppendLinesWithIndentation(params->ToDebugString(), " ", &result);
+ }
return result;
}
@@ -84,16 +85,18 @@
std::string CertErrors::ToDebugString() const {
std::string result;
- for (const CertError &node : nodes_)
+ for (const CertError &node : nodes_) {
result += node.ToDebugString();
+ }
return result;
}
bool CertErrors::ContainsError(CertErrorId id) const {
for (const CertError &node : nodes_) {
- if (node.id == id)
+ if (node.id == id) {
return true;
+ }
}
return false;
}
@@ -101,8 +104,9 @@
bool CertErrors::ContainsAnyErrorWithSeverity(
CertError::Severity severity) const {
for (const CertError &node : nodes_) {
- if (node.severity == severity)
+ if (node.severity == severity) {
return true;
+ }
}
return false;
}
@@ -115,14 +119,16 @@
CertPathErrors::~CertPathErrors() = default;
CertErrors *CertPathErrors::GetErrorsForCert(size_t cert_index) {
- if (cert_index >= cert_errors_.size())
+ if (cert_index >= cert_errors_.size()) {
cert_errors_.resize(cert_index + 1);
+ }
return &cert_errors_[cert_index];
}
const CertErrors *CertPathErrors::GetErrorsForCert(size_t cert_index) const {
- if (cert_index >= cert_errors_.size())
+ if (cert_index >= cert_errors_.size()) {
return nullptr;
+ }
return &cert_errors_[cert_index];
}
@@ -130,12 +136,14 @@
bool CertPathErrors::ContainsError(CertErrorId id) const {
for (const CertErrors &errors : cert_errors_) {
- if (errors.ContainsError(id))
+ if (errors.ContainsError(id)) {
return true;
+ }
}
- if (other_errors_.ContainsError(id))
+ if (other_errors_.ContainsError(id)) {
return true;
+ }
return false;
}
@@ -143,12 +151,14 @@
bool CertPathErrors::ContainsAnyErrorWithSeverity(
CertError::Severity severity) const {
for (const CertErrors &errors : cert_errors_) {
- if (errors.ContainsAnyErrorWithSeverity(severity))
+ if (errors.ContainsAnyErrorWithSeverity(severity)) {
return true;
+ }
}
- if (other_errors_.ContainsAnyErrorWithSeverity(severity))
+ if (other_errors_.ContainsAnyErrorWithSeverity(severity)) {
return true;
+ }
return false;
}
@@ -162,8 +172,9 @@
// then continue.
const CertErrors &errors = cert_errors_[i];
std::string cert_errors_string = errors.ToDebugString();
- if (cert_errors_string.empty())
+ if (cert_errors_string.empty()) {
continue;
+ }
// Add a header that identifies which certificate this CertErrors pertains
// to.
diff --git a/pki/cert_issuer_source_static.cc b/pki/cert_issuer_source_static.cc
index 9d395d3..bbf967f 100644
--- a/pki/cert_issuer_source_static.cc
+++ b/pki/cert_issuer_source_static.cc
@@ -21,8 +21,9 @@
ParsedCertificateList *issuers) {
auto range =
intermediates_.equal_range(cert->normalized_issuer().AsStringView());
- for (auto it = range.first; it != range.second; ++it)
+ for (auto it = range.first; it != range.second; ++it) {
issuers->push_back(it->second);
+ }
}
void CertIssuerSourceStatic::AsyncGetIssuersOf(
diff --git a/pki/cert_issuer_source_sync_unittest.h b/pki/cert_issuer_source_sync_unittest.h
index 12b44e7..c9e25c1 100644
--- a/pki/cert_issuer_source_sync_unittest.h
+++ b/pki/cert_issuer_source_sync_unittest.h
@@ -34,8 +34,9 @@
::testing::AssertionResult r =
ReadTestPem("testdata/cert_issuer_source_static_unittest/" + file_name,
"CERTIFICATE", &der);
- if (!r)
+ if (!r) {
return r;
+ }
CertErrors errors;
*result = ParsedCertificate::Create(
bssl::UniquePtr<CRYPTO_BUFFER>(CRYPTO_BUFFER_new(
@@ -95,17 +96,20 @@
source().SyncGetIssuersOf(cert.get(), &matches);
std::vector<der::Input> der_result_matches;
- for (const auto &it : matches)
+ for (const auto &it : matches) {
der_result_matches.push_back(it->der_cert());
+ }
std::sort(der_result_matches.begin(), der_result_matches.end());
std::vector<der::Input> der_expected_matches;
- for (const auto &it : expected_matches)
+ for (const auto &it : expected_matches) {
der_expected_matches.push_back(it->der_cert());
+ }
std::sort(der_expected_matches.begin(), der_expected_matches.end());
- if (der_expected_matches == der_result_matches)
+ if (der_expected_matches == der_result_matches) {
return true;
+ }
// Print some extra information for debugging.
EXPECT_EQ(der_expected_matches, der_result_matches);
diff --git a/pki/certificate_policies.cc b/pki/certificate_policies.cc
index 69de164..9ae941d 100644
--- a/pki/certificate_policies.cc
+++ b/pki/certificate_policies.cc
@@ -64,8 +64,9 @@
}
// policyQualifierId PolicyQualifierId,
der::Input qualifier_oid;
- if (!policy_information_parser.ReadTag(der::kOid, &qualifier_oid))
+ if (!policy_information_parser.ReadTag(der::kOid, &qualifier_oid)) {
return false;
+ }
if (restrict_to_known_qualifiers &&
qualifier_oid != der::Input(kCpsPointerId) &&
qualifier_oid != der::Input(kUserNoticeId)) {
@@ -85,8 +86,9 @@
return false;
}
- if (policy_qualifiers)
+ if (policy_qualifiers) {
policy_qualifiers->push_back({qualifier_oid, qualifier_tlv});
+ }
}
return true;
}
@@ -136,11 +138,13 @@
// certificatePolicies ::= SEQUENCE SIZE (1..MAX) OF PolicyInformation
der::Parser extension_parser(extension_value);
der::Parser policies_sequence_parser;
- if (!extension_parser.ReadSequence(&policies_sequence_parser))
+ if (!extension_parser.ReadSequence(&policies_sequence_parser)) {
return false;
+ }
// Should not have trailing data after certificatePolicies sequence.
- if (extension_parser.HasMore())
+ if (extension_parser.HasMore()) {
return false;
+ }
// The certificatePolicies sequence should have at least 1 element.
if (!policies_sequence_parser.HasMore()) {
errors->AddError(kPoliciesEmptySequence);
@@ -148,18 +152,21 @@
}
policy_oids->clear();
- if (policy_informations)
+ if (policy_informations) {
policy_informations->clear();
+ }
while (policies_sequence_parser.HasMore()) {
// PolicyInformation ::= SEQUENCE {
der::Parser policy_information_parser;
- if (!policies_sequence_parser.ReadSequence(&policy_information_parser))
+ if (!policies_sequence_parser.ReadSequence(&policy_information_parser)) {
return false;
+ }
// policyIdentifier CertPolicyId,
der::Input policy_oid;
- if (!policy_information_parser.ReadTag(der::kOid, &policy_oid))
+ if (!policy_information_parser.ReadTag(der::kOid, &policy_oid)) {
return false;
+ }
policy_oids->push_back(policy_oid);
@@ -170,8 +177,9 @@
policy_qualifiers = &policy_informations->back().policy_qualifiers;
}
- if (!policy_information_parser.HasMore())
+ if (!policy_information_parser.HasMore()) {
continue;
+ }
// policyQualifiers SEQUENCE SIZE (1..MAX) OF
// PolicyQualifierInfo OPTIONAL }
@@ -249,8 +257,9 @@
// PolicyConstraints ::= SEQUENCE {
der::Parser sequence_parser;
- if (!parser.ReadSequence(&sequence_parser))
+ if (!parser.ReadSequence(&sequence_parser)) {
return false;
+ }
// RFC 5280 prohibits CAs from issuing PolicyConstraints as an empty sequence:
//
@@ -259,8 +268,9 @@
// or the requireExplicitPolicy field MUST be present. The behavior of
// clients that encounter an empty policy constraints field is not
// addressed in this profile.
- if (!sequence_parser.HasMore())
+ if (!sequence_parser.HasMore()) {
return false;
+ }
std::optional<der::Input> require_value;
if (!sequence_parser.ReadOptionalTag(der::ContextSpecificPrimitive(0),
@@ -295,8 +305,9 @@
}
// There should be no remaining data.
- if (sequence_parser.HasMore() || parser.HasMore())
+ if (sequence_parser.HasMore() || parser.HasMore()) {
return false;
+ }
return true;
}
@@ -337,34 +348,41 @@
// PolicyMappings ::= SEQUENCE SIZE (1..MAX) OF SEQUENCE {
der::Parser sequence_parser;
- if (!parser.ReadSequence(&sequence_parser))
+ if (!parser.ReadSequence(&sequence_parser)) {
return false;
+ }
// Must be at least 1 mapping.
- if (!sequence_parser.HasMore())
+ if (!sequence_parser.HasMore()) {
return false;
+ }
while (sequence_parser.HasMore()) {
der::Parser mapping_parser;
- if (!sequence_parser.ReadSequence(&mapping_parser))
+ if (!sequence_parser.ReadSequence(&mapping_parser)) {
return false;
+ }
ParsedPolicyMapping mapping;
- if (!mapping_parser.ReadTag(der::kOid, &mapping.issuer_domain_policy))
+ if (!mapping_parser.ReadTag(der::kOid, &mapping.issuer_domain_policy)) {
return false;
- if (!mapping_parser.ReadTag(der::kOid, &mapping.subject_domain_policy))
+ }
+ if (!mapping_parser.ReadTag(der::kOid, &mapping.subject_domain_policy)) {
return false;
+ }
// There shouldn't be extra unconsumed data.
- if (mapping_parser.HasMore())
+ if (mapping_parser.HasMore()) {
return false;
+ }
mappings->push_back(mapping);
}
// There shouldn't be extra unconsumed data.
- if (parser.HasMore())
+ if (parser.HasMore()) {
return false;
+ }
return true;
}
diff --git a/pki/crl.cc b/pki/crl.cc
index 2ac7948..187a034 100644
--- a/pki/crl.cc
+++ b/pki/crl.cc
@@ -56,32 +56,38 @@
// CertificateList ::= SEQUENCE {
der::Parser certificate_list_parser;
- if (!parser.ReadSequence(&certificate_list_parser))
+ if (!parser.ReadSequence(&certificate_list_parser)) {
return false;
+ }
// tbsCertList TBSCertList
- if (!certificate_list_parser.ReadRawTLV(out_tbs_cert_list_tlv))
+ if (!certificate_list_parser.ReadRawTLV(out_tbs_cert_list_tlv)) {
return false;
+ }
// signatureAlgorithm AlgorithmIdentifier,
- if (!certificate_list_parser.ReadRawTLV(out_signature_algorithm_tlv))
+ if (!certificate_list_parser.ReadRawTLV(out_signature_algorithm_tlv)) {
return false;
+ }
// signatureValue BIT STRING }
std::optional<der::BitString> signature_value =
certificate_list_parser.ReadBitString();
- if (!signature_value)
+ if (!signature_value) {
return false;
+ }
*out_signature_value = signature_value.value();
// There isn't an extension point at the end of CertificateList.
- if (certificate_list_parser.HasMore())
+ if (certificate_list_parser.HasMore()) {
return false;
+ }
// By definition the input was a single CertificateList, so there shouldn't be
// unconsumed data.
- if (parser.HasMore())
+ if (parser.HasMore()) {
return false;
+ }
return true;
}
@@ -91,21 +97,25 @@
// TBSCertList ::= SEQUENCE {
der::Parser tbs_parser;
- if (!parser.ReadSequence(&tbs_parser))
+ if (!parser.ReadSequence(&tbs_parser)) {
return false;
+ }
// version Version OPTIONAL,
// -- if present, MUST be v2
std::optional<der::Input> version_der;
- if (!tbs_parser.ReadOptionalTag(der::kInteger, &version_der))
+ if (!tbs_parser.ReadOptionalTag(der::kInteger, &version_der)) {
return false;
+ }
if (version_der.has_value()) {
uint64_t version64;
- if (!der::ParseUint64(*version_der, &version64))
+ if (!der::ParseUint64(*version_der, &version64)) {
return false;
+ }
// If version is present, it MUST be v2(1).
- if (version64 != 1)
+ if (version64 != 1) {
return false;
+ }
out->version = CrlVersion::V2;
} else {
// Uh, RFC 5280 doesn't actually say it anywhere, but presumably if version
@@ -114,16 +124,19 @@
}
// signature AlgorithmIdentifier,
- if (!tbs_parser.ReadRawTLV(&out->signature_algorithm_tlv))
+ if (!tbs_parser.ReadRawTLV(&out->signature_algorithm_tlv)) {
return false;
+ }
// issuer Name,
- if (!tbs_parser.ReadRawTLV(&out->issuer_tlv))
+ if (!tbs_parser.ReadRawTLV(&out->issuer_tlv)) {
return false;
+ }
// thisUpdate Time,
- if (!ReadUTCOrGeneralizedTime(&tbs_parser, &out->this_update))
+ if (!ReadUTCOrGeneralizedTime(&tbs_parser, &out->this_update)) {
return false;
+ }
// nextUpdate Time OPTIONAL,
der::Tag maybe_next_update_tag;
@@ -133,8 +146,9 @@
(maybe_next_update_tag == der::kUtcTime ||
maybe_next_update_tag == der::kGeneralizedTime)) {
der::GeneralizedTime next_update_time;
- if (!ReadUTCOrGeneralizedTime(&tbs_parser, &next_update_time))
+ if (!ReadUTCOrGeneralizedTime(&tbs_parser, &next_update_time)) {
return false;
+ }
out->next_update = next_update_time;
} else {
out->next_update = std::nullopt;
@@ -147,8 +161,9 @@
&unused_revoked_certificates) &&
maybe_revoked_certifigates_tag == der::kSequence) {
der::Input revoked_certificates_tlv;
- if (!tbs_parser.ReadRawTLV(&revoked_certificates_tlv))
+ if (!tbs_parser.ReadRawTLV(&revoked_certificates_tlv)) {
return false;
+ }
out->revoked_certificates_tlv = revoked_certificates_tlv;
} else {
out->revoked_certificates_tlv = std::nullopt;
@@ -161,8 +176,9 @@
return false;
}
if (out->crl_extensions_tlv.has_value()) {
- if (out->version != CrlVersion::V2)
+ if (out->version != CrlVersion::V2) {
return false;
+ }
}
if (tbs_parser.HasMore()) {
@@ -172,8 +188,9 @@
// By definition the input was a single sequence, so there shouldn't be
// unconsumed data.
- if (parser.HasMore())
+ if (parser.HasMore()) {
return false;
+ }
return true;
}
@@ -185,14 +202,16 @@
der::Parser idp_extension_value_parser(extension_value);
// IssuingDistributionPoint ::= SEQUENCE {
der::Parser idp_parser;
- if (!idp_extension_value_parser.ReadSequence(&idp_parser))
+ if (!idp_extension_value_parser.ReadSequence(&idp_parser)) {
return false;
+ }
// 5.2.5. Conforming CRLs issuers MUST NOT issue CRLs where the DER
// encoding of the issuing distribution point extension is an empty
// sequence.
- if (!idp_parser.HasMore())
+ if (!idp_parser.HasMore()) {
return false;
+ }
// distributionPoint [0] DistributionPointName OPTIONAL,
std::optional<der::Input> distribution_point;
@@ -220,8 +239,9 @@
CertErrors errors;
*out_distribution_point_names =
GeneralNames::CreateFromValue(*der_full_name, &errors);
- if (!*out_distribution_point_names)
+ if (!*out_distribution_point_names) {
return false;
+ }
if (dp_name_parser.HasMore()) {
// CHOICE represents a single value.
@@ -239,10 +259,12 @@
}
if (only_contains_user_certs.has_value()) {
bool bool_value;
- if (!der::ParseBool(*only_contains_user_certs, &bool_value))
+ if (!der::ParseBool(*only_contains_user_certs, &bool_value)) {
return false;
- if (!bool_value)
+ }
+ if (!bool_value) {
return false; // DER-encoding requires DEFAULT values be omitted.
+ }
*out_only_contains_cert_type = ContainedCertsType::USER_CERTS;
}
@@ -254,10 +276,12 @@
}
if (only_contains_ca_certs.has_value()) {
bool bool_value;
- if (!der::ParseBool(*only_contains_ca_certs, &bool_value))
+ if (!der::ParseBool(*only_contains_ca_certs, &bool_value)) {
return false;
- if (!bool_value)
+ }
+ if (!bool_value) {
return false; // DER-encoding requires DEFAULT values be omitted.
+ }
if (*out_only_contains_cert_type != ContainedCertsType::ANY_CERTS) {
// 5.2.5. at most one of onlyContainsUserCerts, onlyContainsCACerts,
// and onlyContainsAttributeCerts may be set to TRUE.
@@ -271,8 +295,9 @@
// onlyContainsAttributeCerts [5] BOOLEAN DEFAULT FALSE }
// onlySomeReasons, indirectCRL, and onlyContainsAttributeCerts are not
// supported, fail parsing if they are present.
- if (idp_parser.HasMore())
+ if (idp_parser.HasMore()) {
return false;
+ }
return true;
}
@@ -291,62 +316,73 @@
// revokedCertificates SEQUENCE OF SEQUENCE {
der::Parser revoked_certificates_parser;
- if (!parser.ReadSequence(&revoked_certificates_parser))
+ if (!parser.ReadSequence(&revoked_certificates_parser)) {
return CRLRevocationStatus::UNKNOWN;
+ }
// RFC 5280 Section 5.1.2.6: "When there are no revoked certificates, the
// revoked certificates list MUST be absent."
- if (!revoked_certificates_parser.HasMore())
+ if (!revoked_certificates_parser.HasMore()) {
return CRLRevocationStatus::UNKNOWN;
+ }
// By definition the input was a single Extensions sequence, so there
// shouldn't be unconsumed data.
- if (parser.HasMore())
+ if (parser.HasMore()) {
return CRLRevocationStatus::UNKNOWN;
+ }
bool found_matching_serial = false;
while (revoked_certificates_parser.HasMore()) {
// revokedCertificates SEQUENCE OF SEQUENCE {
der::Parser crl_entry_parser;
- if (!revoked_certificates_parser.ReadSequence(&crl_entry_parser))
+ if (!revoked_certificates_parser.ReadSequence(&crl_entry_parser)) {
return CRLRevocationStatus::UNKNOWN;
+ }
der::Input revoked_cert_serial_number;
// userCertificate CertificateSerialNumber,
- if (!crl_entry_parser.ReadTag(der::kInteger, &revoked_cert_serial_number))
+ if (!crl_entry_parser.ReadTag(der::kInteger, &revoked_cert_serial_number)) {
return CRLRevocationStatus::UNKNOWN;
+ }
// revocationDate Time,
der::GeneralizedTime unused_revocation_date;
- if (!ReadUTCOrGeneralizedTime(&crl_entry_parser, &unused_revocation_date))
+ if (!ReadUTCOrGeneralizedTime(&crl_entry_parser, &unused_revocation_date)) {
return CRLRevocationStatus::UNKNOWN;
+ }
// crlEntryExtensions Extensions OPTIONAL
if (crl_entry_parser.HasMore()) {
// -- if present, version MUST be v2
- if (crl_version != CrlVersion::V2)
+ if (crl_version != CrlVersion::V2) {
return CRLRevocationStatus::UNKNOWN;
+ }
der::Input crl_entry_extensions_tlv;
- if (!crl_entry_parser.ReadRawTLV(&crl_entry_extensions_tlv))
+ if (!crl_entry_parser.ReadRawTLV(&crl_entry_extensions_tlv)) {
return CRLRevocationStatus::UNKNOWN;
+ }
std::map<der::Input, ParsedExtension> extensions;
- if (!ParseExtensions(crl_entry_extensions_tlv, &extensions))
+ if (!ParseExtensions(crl_entry_extensions_tlv, &extensions)) {
return CRLRevocationStatus::UNKNOWN;
+ }
// RFC 5280 Section 5.3: "If a CRL contains a critical CRL entry
// extension that the application cannot process, then the application
// MUST NOT use that CRL to determine the status of any certificates."
for (const auto &ext : extensions) {
- if (ext.second.critical)
+ if (ext.second.critical) {
return CRLRevocationStatus::UNKNOWN;
+ }
}
}
- if (crl_entry_parser.HasMore())
+ if (crl_entry_parser.HasMore()) {
return CRLRevocationStatus::UNKNOWN;
+ }
if (revoked_cert_serial_number == cert_serial) {
// Cert is revoked, but can't return yet since there might be critical
@@ -355,8 +391,9 @@
}
}
- if (found_matching_serial)
+ if (found_matching_serial) {
return CRLRevocationStatus::REVOKED;
+ }
// |cert| is not present in the revokedCertificates list.
return CRLRevocationStatus::GOOD;
@@ -401,8 +438,9 @@
}
ParsedCrlTbsCertList tbs_cert_list;
- if (!ParseCrlTbsCertList(tbs_cert_list_tlv, &tbs_cert_list))
+ if (!ParseCrlTbsCertList(tbs_cert_list_tlv, &tbs_cert_list)) {
return CRLRevocationStatus::UNKNOWN;
+ }
// 5.1.1.2 signatureAlgorithm
//
@@ -466,16 +504,18 @@
// TODO(https://crbug.com/749276): could do exact comparison first and only
// fall back to normalizing if that fails.
std::string normalized_crl_issuer;
- if (!NormalizeNameTLV(tbs_cert_list.issuer_tlv, &normalized_crl_issuer))
+ if (!NormalizeNameTLV(tbs_cert_list.issuer_tlv, &normalized_crl_issuer)) {
return CRLRevocationStatus::UNKNOWN;
+ }
if (der::Input(normalized_crl_issuer) != target_cert->normalized_issuer()) {
return CRLRevocationStatus::UNKNOWN;
}
if (tbs_cert_list.crl_extensions_tlv.has_value()) {
std::map<der::Input, ParsedExtension> extensions;
- if (!ParseExtensions(*tbs_cert_list.crl_extensions_tlv, &extensions))
+ if (!ParseExtensions(*tbs_cert_list.crl_extensions_tlv, &extensions)) {
return CRLRevocationStatus::UNKNOWN;
+ }
// 6.3.3 (b) (2) If the complete CRL includes an issuing distribution point
// (IDP) CRL extension, check the following:
@@ -559,8 +599,9 @@
for (const auto &ext : extensions) {
// Fail if any unhandled critical CRL extensions are present.
- if (ext.second.critical)
+ if (ext.second.critical) {
return CRLRevocationStatus::UNKNOWN;
+ }
}
}
diff --git a/pki/encode_values.cc b/pki/encode_values.cc
index 4d4a0e9..25dc677 100644
--- a/pki/encode_values.cc
+++ b/pki/encode_values.cc
@@ -13,8 +13,9 @@
namespace {
bool WriteFourDigit(uint16_t value, uint8_t out[4]) {
- if (value >= 10000)
+ if (value >= 10000) {
return false;
+ }
out[3] = '0' + (value % 10);
value /= 10;
out[2] = '0' + (value % 10);
@@ -26,8 +27,9 @@
}
bool WriteTwoDigit(uint8_t value, uint8_t out[2]) {
- if (value >= 100)
+ if (value >= 100) {
return false;
+ }
out[0] = '0' + (value / 10);
out[1] = '0' + (value % 10);
return true;
@@ -82,12 +84,14 @@
}
bool EncodeUTCTime(const GeneralizedTime &time, uint8_t out[kUTCTimeLength]) {
- if (!time.InUTCTimeRange())
+ if (!time.InUTCTimeRange()) {
return false;
+ }
uint16_t year = time.year - 1900;
- if (year >= 100)
+ if (year >= 100) {
year -= 100;
+ }
if (!WriteTwoDigit(year, out) || !WriteTwoDigit(time.month, out + 2) ||
!WriteTwoDigit(time.day, out + 4) ||
diff --git a/pki/extended_key_usage.cc b/pki/extended_key_usage.cc
index d81599a..caf2514 100644
--- a/pki/extended_key_usage.cc
+++ b/pki/extended_key_usage.cc
@@ -14,26 +14,30 @@
std::vector<der::Input> *eku_oids) {
der::Parser extension_parser(extension_value);
der::Parser sequence_parser;
- if (!extension_parser.ReadSequence(&sequence_parser))
+ if (!extension_parser.ReadSequence(&sequence_parser)) {
return false;
+ }
// Section 4.2.1.12 of RFC 5280 defines ExtKeyUsageSyntax as:
// ExtKeyUsageSyntax ::= SEQUENCE SIZE (1..MAX) OF KeyPurposeId
//
// Therefore, the sequence must contain at least one KeyPurposeId.
- if (!sequence_parser.HasMore())
+ if (!sequence_parser.HasMore()) {
return false;
+ }
while (sequence_parser.HasMore()) {
der::Input eku_oid;
- if (!sequence_parser.ReadTag(der::kOid, &eku_oid))
+ if (!sequence_parser.ReadTag(der::kOid, &eku_oid)) {
// The SEQUENCE OF must contain only KeyPurposeIds (OIDs).
return false;
+ }
eku_oids->push_back(eku_oid);
}
- if (extension_parser.HasMore())
+ if (extension_parser.HasMore()) {
// The extension value must follow ExtKeyUsageSyntax - there is no way that
// it could be extended to allow for something after the SEQUENCE OF.
return false;
+ }
return true;
}
diff --git a/pki/extended_key_usage_unittest.cc b/pki/extended_key_usage_unittest.cc
index f764e52..e617dce 100644
--- a/pki/extended_key_usage_unittest.cc
+++ b/pki/extended_key_usage_unittest.cc
@@ -15,8 +15,9 @@
// Helper method to check if an EKU is present in a std::vector of EKUs.
bool HasEKU(const std::vector<der::Input> &list, const der::Input &eku) {
for (const auto &oid : list) {
- if (oid == eku)
+ if (oid == eku) {
return true;
+ }
}
return false;
}
diff --git a/pki/general_names.cc b/pki/general_names.cc
index 253e54e..a881c11 100644
--- a/pki/general_names.cc
+++ b/pki/general_names.cc
@@ -103,8 +103,9 @@
der::Parser parser(input);
der::Tag tag;
der::Input value;
- if (!parser.ReadTagAndValue(&tag, &value))
+ if (!parser.ReadTagAndValue(&tag, &value)) {
return false;
+ }
GeneralNameTypes name_type = GENERAL_NAME_NONE;
if (tag == der::ContextSpecificConstructed(0)) {
// otherName [0] OtherName,
@@ -140,8 +141,9 @@
// only the value portion.
der::Parser name_parser(value);
der::Input name_value;
- if (!name_parser.ReadTag(der::kSequence, &name_value) || parser.HasMore())
+ if (!name_parser.ReadTag(der::kSequence, &name_value) || parser.HasMore()) {
return false;
+ }
subtrees->directory_names.push_back(name_value);
} else if (tag == der::ContextSpecificConstructed(5)) {
// ediPartyName [5] EDIPartyName,
diff --git a/pki/input.cc b/pki/input.cc
index 274055b..a0d8af6 100644
--- a/pki/input.cc
+++ b/pki/input.cc
@@ -34,16 +34,18 @@
: data_(in.UnsafeData()), len_(in.Length()) {}
bool ByteReader::ReadByte(uint8_t *byte_p) {
- if (!HasMore())
+ if (!HasMore()) {
return false;
+ }
*byte_p = *data_;
Advance(1);
return true;
}
bool ByteReader::ReadBytes(size_t len, Input *out) {
- if (len > len_)
+ if (len > len_) {
return false;
+ }
*out = Input(data_, len);
Advance(len);
return true;
diff --git a/pki/name_constraints.cc b/pki/name_constraints.cc
index 6dea412..6de5b50 100644
--- a/pki/name_constraints.cc
+++ b/pki/name_constraints.cc
@@ -56,14 +56,17 @@
bool DNSNameMatches(std::string_view name, std::string_view dns_constraint,
WildcardMatchType wildcard_matching) {
// Everything matches the empty DNS name constraint.
- if (dns_constraint.empty())
+ if (dns_constraint.empty()) {
return true;
+ }
// Normalize absolute DNS names by removing the trailing dot, if any.
- if (!name.empty() && *name.rbegin() == '.')
+ if (!name.empty() && *name.rbegin() == '.') {
name.remove_suffix(1);
- if (!dns_constraint.empty() && *dns_constraint.rbegin() == '.')
+ }
+ if (!dns_constraint.empty() && *dns_constraint.rbegin() == '.') {
dns_constraint.remove_suffix(1);
+ }
// Wildcard partial-match handling ("*.bar.com" matching name constraint
// "foo.bar.com"). This only handles the case where the the dnsname and the
@@ -89,13 +92,15 @@
}
// Exact match.
- if (name.size() == dns_constraint.size())
+ if (name.size() == dns_constraint.size()) {
return true;
+ }
// If dNSName constraint starts with a dot, only subdomains should match.
// (e.g., "foo.bar.com" matches constraint ".bar.com", but "bar.com" doesn't.)
// RFC 5280 is ambiguous, but this matches the behavior of other platforms.
- if (!dns_constraint.empty() && dns_constraint[0] == '.')
+ if (!dns_constraint.empty() && dns_constraint[0] == '.') {
dns_constraint.remove_prefix(1);
+ }
// Subtree match.
if (name.size() > dns_constraint.size() &&
name[name.size() - dns_constraint.size() - 1] == '.') {
@@ -127,16 +132,19 @@
// BaseDistance ::= INTEGER (0..MAX)
der::Parser sequence_parser(value);
// The GeneralSubtrees sequence should have at least 1 element.
- if (!sequence_parser.HasMore())
+ if (!sequence_parser.HasMore()) {
return false;
+ }
while (sequence_parser.HasMore()) {
der::Parser subtree_sequence;
- if (!sequence_parser.ReadSequence(&subtree_sequence))
+ if (!sequence_parser.ReadSequence(&subtree_sequence)) {
return false;
+ }
der::Input raw_general_name;
- if (!subtree_sequence.ReadRawTLV(&raw_general_name))
+ if (!subtree_sequence.ReadRawTLV(&raw_general_name)) {
return false;
+ }
if (!ParseGeneralName(raw_general_name,
GeneralNames::IP_ADDRESS_AND_NETMASK, subtrees,
@@ -157,8 +165,9 @@
// fail if a name of this type actually appears in a subsequent cert and
// this extension was marked critical. However the minimum and maximum
// fields appear uncommon enough that implementing that isn't useful.
- if (subtree_sequence.HasMore())
+ if (subtree_sequence.HasMore()) {
return false;
+ }
}
return true;
}
@@ -273,8 +282,9 @@
BSSL_CHECK(errors);
auto name_constraints = std::make_unique<NameConstraints>();
- if (!name_constraints->Parse(extension_value, is_critical, errors))
+ if (!name_constraints->Parse(extension_value, is_critical, errors)) {
return nullptr;
+ }
return name_constraints;
}
@@ -288,10 +298,12 @@
// NameConstraints ::= SEQUENCE {
// permittedSubtrees [0] GeneralSubtrees OPTIONAL,
// excludedSubtrees [1] GeneralSubtrees OPTIONAL }
- if (!extension_parser.ReadSequence(&sequence_parser))
+ if (!extension_parser.ReadSequence(&sequence_parser)) {
return false;
- if (extension_parser.HasMore())
+ }
+ if (extension_parser.HasMore()) {
return false;
+ }
std::optional<der::Input> permitted_subtrees_value;
if (!sequence_parser.ReadOptionalTag(der::ContextSpecificConstructed(0),
@@ -325,11 +337,13 @@
// Conforming CAs MUST NOT issue certificates where name constraints is an
// empty sequence. That is, either the permittedSubtrees field or the
// excludedSubtrees MUST be present.
- if (!permitted_subtrees_value && !excluded_subtrees_value)
+ if (!permitted_subtrees_value && !excluded_subtrees_value) {
return false;
+ }
- if (sequence_parser.HasMore())
+ if (sequence_parser.HasMore()) {
return false;
+ }
return true;
}
@@ -488,8 +502,9 @@
// This code assumes that criticality condition is checked by the caller, and
// therefore only needs to avoid the IsPermittedDirectoryName check against an
// empty subject in such a case.
- if (subject_alt_names && subject_rdn_sequence.Length() == 0)
+ if (subject_alt_names && subject_rdn_sequence.Length() == 0) {
return;
+ }
if (!IsPermittedDirectoryName(subject_rdn_sequence)) {
errors->AddError(cert_errors::kNotPermittedByNameConstraints);
@@ -608,21 +623,24 @@
// When matching wildcard hosts against excluded subtrees, consider it a
// match if the constraint would match any expansion of the wildcard. Eg,
// *.bar.com should match a constraint of foo.bar.com.
- if (DNSNameMatches(name, excluded_name, WILDCARD_PARTIAL_MATCH))
+ if (DNSNameMatches(name, excluded_name, WILDCARD_PARTIAL_MATCH)) {
return false;
+ }
}
// If permitted subtrees are not constrained, any name that is not excluded is
// allowed.
- if (!(permitted_subtrees_.present_name_types & GENERAL_NAME_DNS_NAME))
+ if (!(permitted_subtrees_.present_name_types & GENERAL_NAME_DNS_NAME)) {
return true;
+ }
for (const auto &permitted_name : permitted_subtrees_.dns_names) {
// When matching wildcard hosts against permitted subtrees, consider it a
// match only if the constraint would match all expansions of the wildcard.
// Eg, *.bar.com should match a constraint of bar.com, but not foo.bar.com.
- if (DNSNameMatches(name, permitted_name, WILDCARD_FULL_MATCH))
+ if (DNSNameMatches(name, permitted_name, WILDCARD_FULL_MATCH)) {
return true;
+ }
}
return false;
@@ -631,18 +649,21 @@
bool NameConstraints::IsPermittedDirectoryName(
const der::Input &name_rdn_sequence) const {
for (const auto &excluded_name : excluded_subtrees_.directory_names) {
- if (VerifyNameInSubtree(name_rdn_sequence, excluded_name))
+ if (VerifyNameInSubtree(name_rdn_sequence, excluded_name)) {
return false;
+ }
}
// If permitted subtrees are not constrained, any name that is not excluded is
// allowed.
- if (!(permitted_subtrees_.present_name_types & GENERAL_NAME_DIRECTORY_NAME))
+ if (!(permitted_subtrees_.present_name_types & GENERAL_NAME_DIRECTORY_NAME)) {
return true;
+ }
for (const auto &permitted_name : permitted_subtrees_.directory_names) {
- if (VerifyNameInSubtree(name_rdn_sequence, permitted_name))
+ if (VerifyNameInSubtree(name_rdn_sequence, permitted_name)) {
return true;
+ }
}
return false;
diff --git a/pki/name_constraints_unittest.cc b/pki/name_constraints_unittest.cc
index 4121b6f..569bbec 100644
--- a/pki/name_constraints_unittest.cc
+++ b/pki/name_constraints_unittest.cc
@@ -64,10 +64,12 @@
CertErrors errors;
name_constraints->IsPermittedCert(subject_rdn_sequence, subject_alt_names,
&errors);
- if (!errors.ContainsAnyErrorWithSeverity(CertError::SEVERITY_HIGH))
+ if (!errors.ContainsAnyErrorWithSeverity(CertError::SEVERITY_HIGH)) {
return ::testing::AssertionSuccess();
- if (!errors.ContainsError(cert_errors::kNotPermittedByNameConstraints))
+ }
+ if (!errors.ContainsError(cert_errors::kNotPermittedByNameConstraints)) {
ADD_FAILURE() << "unexpected error " << errors.ToDebugString();
+ }
return ::testing::AssertionFailure();
}
diff --git a/pki/ocsp.cc b/pki/ocsp.cc
index ed0d2ce..54afab5 100644
--- a/pki/ocsp.cc
+++ b/pki/ocsp.cc
@@ -40,25 +40,34 @@
bool ParseOCSPCertID(const der::Input &raw_tlv, OCSPCertID *out) {
der::Parser outer_parser(raw_tlv);
der::Parser parser;
- if (!outer_parser.ReadSequence(&parser))
+ if (!outer_parser.ReadSequence(&parser)) {
return false;
- if (outer_parser.HasMore())
+ }
+ if (outer_parser.HasMore()) {
return false;
+ }
der::Input sigalg_tlv;
- if (!parser.ReadRawTLV(&sigalg_tlv))
+ if (!parser.ReadRawTLV(&sigalg_tlv)) {
return false;
- if (!ParseHashAlgorithm(sigalg_tlv, &(out->hash_algorithm)))
+ }
+ if (!ParseHashAlgorithm(sigalg_tlv, &(out->hash_algorithm))) {
return false;
- if (!parser.ReadTag(der::kOctetString, &(out->issuer_name_hash)))
+ }
+ if (!parser.ReadTag(der::kOctetString, &(out->issuer_name_hash))) {
return false;
- if (!parser.ReadTag(der::kOctetString, &(out->issuer_key_hash)))
+ }
+ if (!parser.ReadTag(der::kOctetString, &(out->issuer_key_hash))) {
return false;
- if (!parser.ReadTag(der::kInteger, &(out->serial_number)))
+ }
+ if (!parser.ReadTag(der::kInteger, &(out->serial_number))) {
return false;
+ }
CertErrors errors;
- if (!VerifySerialNumber(out->serial_number, false /*warnings_only*/, &errors))
+ if (!VerifySerialNumber(out->serial_number, false /*warnings_only*/,
+ &errors)) {
return false;
+ }
return !parser.HasMore();
}
@@ -75,8 +84,9 @@
// }
bool ParseRevokedInfo(const der::Input &raw_tlv, OCSPCertStatus *out) {
der::Parser parser(raw_tlv);
- if (!parser.ReadGeneralizedTime(&(out->revocation_time)))
+ if (!parser.ReadGeneralizedTime(&(out->revocation_time))) {
return false;
+ }
der::Input reason_input;
if (!parser.ReadOptionalTag(der::ContextSpecificConstructed(0), &reason_input,
@@ -87,20 +97,24 @@
der::Parser reason_parser(reason_input);
der::Input reason_value_input;
uint8_t reason_value;
- if (!reason_parser.ReadTag(der::kEnumerated, &reason_value_input))
+ if (!reason_parser.ReadTag(der::kEnumerated, &reason_value_input)) {
return false;
- if (!der::ParseUint8(reason_value_input, &reason_value))
+ }
+ if (!der::ParseUint8(reason_value_input, &reason_value)) {
return false;
+ }
if (reason_value >
static_cast<uint8_t>(OCSPCertStatus::RevocationReason::LAST)) {
return false;
}
out->revocation_reason =
static_cast<OCSPCertStatus::RevocationReason>(reason_value);
- if (out->revocation_reason == OCSPCertStatus::RevocationReason::UNUSED)
+ if (out->revocation_reason == OCSPCertStatus::RevocationReason::UNUSED) {
return false;
- if (reason_parser.HasMore())
+ }
+ if (reason_parser.HasMore()) {
return false;
+ }
}
return !parser.HasMore();
}
@@ -120,16 +134,18 @@
der::Parser parser(raw_tlv);
der::Tag status_tag;
der::Input status;
- if (!parser.ReadTagAndValue(&status_tag, &status))
+ if (!parser.ReadTagAndValue(&status_tag, &status)) {
return false;
+ }
out->has_reason = false;
if (status_tag == der::ContextSpecificPrimitive(0)) {
out->status = OCSPRevocationStatus::GOOD;
} else if (status_tag == der::ContextSpecificConstructed(1)) {
out->status = OCSPRevocationStatus::REVOKED;
- if (!ParseRevokedInfo(status, out))
+ if (!ParseRevokedInfo(status, out)) {
return false;
+ }
} else if (status_tag == der::ContextSpecificPrimitive(2)) {
out->status = OCSPRevocationStatus::UNKNOWN;
} else {
@@ -166,20 +182,26 @@
OCSPSingleResponse *out) {
der::Parser outer_parser(raw_tlv);
der::Parser parser;
- if (!outer_parser.ReadSequence(&parser))
+ if (!outer_parser.ReadSequence(&parser)) {
return false;
- if (outer_parser.HasMore())
+ }
+ if (outer_parser.HasMore()) {
return false;
+ }
- if (!parser.ReadRawTLV(&(out->cert_id_tlv)))
+ if (!parser.ReadRawTLV(&(out->cert_id_tlv))) {
return false;
+ }
der::Input status_tlv;
- if (!parser.ReadRawTLV(&status_tlv))
+ if (!parser.ReadRawTLV(&status_tlv)) {
return false;
- if (!ParseCertStatus(status_tlv, &(out->cert_status)))
+ }
+ if (!ParseCertStatus(status_tlv, &(out->cert_status))) {
return false;
- if (!parser.ReadGeneralizedTime(&(out->this_update)))
+ }
+ if (!parser.ReadGeneralizedTime(&(out->this_update))) {
return false;
+ }
der::Input next_update_input;
if (!parser.ReadOptionalTag(der::ContextSpecificConstructed(0),
@@ -188,10 +210,12 @@
}
if (out->has_next_update) {
der::Parser next_update_parser(next_update_input);
- if (!next_update_parser.ReadGeneralizedTime(&(out->next_update)))
+ if (!next_update_parser.ReadGeneralizedTime(&(out->next_update))) {
return false;
- if (next_update_parser.HasMore())
+ }
+ if (next_update_parser.HasMore()) {
return false;
+ }
}
if (!parser.ReadOptionalTag(der::ContextSpecificConstructed(1),
@@ -216,8 +240,9 @@
der::Parser parser(raw_tlv);
der::Tag id_tag;
der::Input id_input;
- if (!parser.ReadTagAndValue(&id_tag, &id_input))
+ if (!parser.ReadTagAndValue(&id_tag, &id_input)) {
return false;
+ }
if (id_tag == der::ContextSpecificConstructed(1)) {
out->type = OCSPResponseData::ResponderType::NAME;
@@ -225,12 +250,15 @@
} else if (id_tag == der::ContextSpecificConstructed(2)) {
der::Parser key_parser(id_input);
der::Input key_hash;
- if (!key_parser.ReadTag(der::kOctetString, &key_hash))
+ if (!key_parser.ReadTag(der::kOctetString, &key_hash)) {
return false;
- if (key_parser.HasMore())
+ }
+ if (key_parser.HasMore()) {
return false;
- if (key_hash.Length() != SHA_DIGEST_LENGTH)
+ }
+ if (key_hash.Length() != SHA_DIGEST_LENGTH) {
return false;
+ }
out->type = OCSPResponseData::ResponderType::KEY_HASH;
out->key_hash = key_hash;
@@ -252,10 +280,12 @@
bool ParseOCSPResponseData(const der::Input &raw_tlv, OCSPResponseData *out) {
der::Parser outer_parser(raw_tlv);
der::Parser parser;
- if (!outer_parser.ReadSequence(&parser))
+ if (!outer_parser.ReadSequence(&parser)) {
return false;
- if (outer_parser.HasMore())
+ }
+ if (outer_parser.HasMore()) {
return false;
+ }
der::Input version_input;
bool version_present;
@@ -269,33 +299,41 @@
// TODO: Add warning about non-strict parsing.
if (version_present) {
der::Parser version_parser(version_input);
- if (!version_parser.ReadUint8(&(out->version)))
+ if (!version_parser.ReadUint8(&(out->version))) {
return false;
- if (version_parser.HasMore())
+ }
+ if (version_parser.HasMore()) {
return false;
+ }
} else {
out->version = 0;
}
- if (out->version != 0)
+ if (out->version != 0) {
return false;
+ }
der::Input responder_input;
- if (!parser.ReadRawTLV(&responder_input))
+ if (!parser.ReadRawTLV(&responder_input)) {
return false;
- if (!ParseResponderID(responder_input, &(out->responder_id)))
+ }
+ if (!ParseResponderID(responder_input, &(out->responder_id))) {
return false;
- if (!parser.ReadGeneralizedTime(&(out->produced_at)))
+ }
+ if (!parser.ReadGeneralizedTime(&(out->produced_at))) {
return false;
+ }
der::Parser responses_parser;
- if (!parser.ReadSequence(&responses_parser))
+ if (!parser.ReadSequence(&responses_parser)) {
return false;
+ }
out->responses.clear();
while (responses_parser.HasMore()) {
der::Input single_response;
- if (!responses_parser.ReadRawTLV(&single_response))
+ if (!responses_parser.ReadRawTLV(&single_response)) {
return false;
+ }
out->responses.push_back(single_response);
}
@@ -322,25 +360,31 @@
bool ParseBasicOCSPResponse(const der::Input &raw_tlv, OCSPResponse *out) {
der::Parser outer_parser(raw_tlv);
der::Parser parser;
- if (!outer_parser.ReadSequence(&parser))
+ if (!outer_parser.ReadSequence(&parser)) {
return false;
- if (outer_parser.HasMore())
+ }
+ if (outer_parser.HasMore()) {
return false;
+ }
- if (!parser.ReadRawTLV(&(out->data)))
+ if (!parser.ReadRawTLV(&(out->data))) {
return false;
+ }
der::Input sigalg_tlv;
- if (!parser.ReadRawTLV(&sigalg_tlv))
+ if (!parser.ReadRawTLV(&sigalg_tlv)) {
return false;
+ }
// TODO(crbug.com/634443): Propagate the errors.
std::optional<SignatureAlgorithm> sigalg =
ParseSignatureAlgorithm(sigalg_tlv);
- if (!sigalg)
+ if (!sigalg) {
return false;
+ }
out->signature_algorithm = sigalg.value();
std::optional<der::BitString> signature = parser.ReadBitString();
- if (!signature)
+ if (!signature) {
return false;
+ }
out->signature = signature.value();
der::Input certs_input;
if (!parser.ReadOptionalTag(der::ContextSpecificConstructed(0), &certs_input,
@@ -352,14 +396,17 @@
if (out->has_certs) {
der::Parser certs_seq_parser(certs_input);
der::Parser certs_parser;
- if (!certs_seq_parser.ReadSequence(&certs_parser))
+ if (!certs_seq_parser.ReadSequence(&certs_parser)) {
return false;
- if (certs_seq_parser.HasMore())
+ }
+ if (certs_seq_parser.HasMore()) {
return false;
+ }
while (certs_parser.HasMore()) {
der::Input cert_tlv;
- if (!certs_parser.ReadRawTLV(&cert_tlv))
+ if (!certs_parser.ReadRawTLV(&cert_tlv)) {
return false;
+ }
out->certs.push_back(cert_tlv);
}
}
@@ -381,24 +428,29 @@
bool ParseOCSPResponse(const der::Input &raw_tlv, OCSPResponse *out) {
der::Parser outer_parser(raw_tlv);
der::Parser parser;
- if (!outer_parser.ReadSequence(&parser))
+ if (!outer_parser.ReadSequence(&parser)) {
return false;
- if (outer_parser.HasMore())
+ }
+ if (outer_parser.HasMore()) {
return false;
+ }
der::Input response_status_input;
uint8_t response_status;
- if (!parser.ReadTag(der::kEnumerated, &response_status_input))
+ if (!parser.ReadTag(der::kEnumerated, &response_status_input)) {
return false;
- if (!der::ParseUint8(response_status_input, &response_status))
+ }
+ if (!der::ParseUint8(response_status_input, &response_status)) {
return false;
+ }
if (response_status >
static_cast<uint8_t>(OCSPResponse::ResponseStatus::LAST)) {
return false;
}
out->status = static_cast<OCSPResponse::ResponseStatus>(response_status);
- if (out->status == OCSPResponse::ResponseStatus::UNUSED)
+ if (out->status == OCSPResponse::ResponseStatus::UNUSED) {
return false;
+ }
if (out->status == OCSPResponse::ResponseStatus::SUCCESSFUL) {
der::Parser outer_bytes_parser;
@@ -407,26 +459,33 @@
&outer_bytes_parser)) {
return false;
}
- if (!outer_bytes_parser.ReadSequence(&bytes_parser))
+ if (!outer_bytes_parser.ReadSequence(&bytes_parser)) {
return false;
- if (outer_bytes_parser.HasMore())
+ }
+ if (outer_bytes_parser.HasMore()) {
return false;
+ }
der::Input type_oid;
- if (!bytes_parser.ReadTag(der::kOid, &type_oid))
+ if (!bytes_parser.ReadTag(der::kOid, &type_oid)) {
return false;
- if (type_oid != der::Input(kBasicOCSPResponseOid))
+ }
+ if (type_oid != der::Input(kBasicOCSPResponseOid)) {
return false;
+ }
// As per RFC 6960 Section 4.2.1, the value of |response| SHALL be the DER
// encoding of BasicOCSPResponse.
der::Input response;
- if (!bytes_parser.ReadTag(der::kOctetString, &response))
+ if (!bytes_parser.ReadTag(der::kOctetString, &response)) {
return false;
- if (!ParseBasicOCSPResponse(response, out))
+ }
+ if (!ParseBasicOCSPResponse(response, out)) {
return false;
- if (bytes_parser.HasMore())
+ }
+ if (bytes_parser.HasMore()) {
return false;
+ }
}
return !parser.HasMore();
@@ -505,15 +564,18 @@
break;
}
- if (!VerifyHash(type, id.issuer_name_hash, certificate->tbs().issuer_tlv))
+ if (!VerifyHash(type, id.issuer_name_hash, certificate->tbs().issuer_tlv)) {
return false;
+ }
der::Input key_tlv;
- if (!GetSubjectPublicKeyBytes(issuer_certificate->tbs().spki_tlv, &key_tlv))
+ if (!GetSubjectPublicKeyBytes(issuer_certificate->tbs().spki_tlv, &key_tlv)) {
return false;
+ }
- if (!VerifyHash(type, id.issuer_key_hash, key_tlv))
+ if (!VerifyHash(type, id.issuer_key_hash, key_tlv)) {
return false;
+ }
return id.serial_number == certificate->tbs().serial_number;
}
@@ -550,14 +612,16 @@
der::Input cert_rdn;
if (!der::Parser(id.name).ReadTag(der::kSequence, &name_rdn) ||
!der::Parser(cert->tbs().subject_tlv)
- .ReadTag(der::kSequence, &cert_rdn))
+ .ReadTag(der::kSequence, &cert_rdn)) {
return false;
+ }
return VerifyNameMatch(name_rdn, cert_rdn);
}
case OCSPResponseData::ResponderType::KEY_HASH: {
der::Input key;
- if (!GetSubjectPublicKeyBytes(cert->tbs().spki_tlv, &key))
+ if (!GetSubjectPublicKeyBytes(cert->tbs().spki_tlv, &key)) {
return false;
+ }
return VerifyHash(EVP_sha1(), id.key_hash, key);
}
}
@@ -591,13 +655,15 @@
// The Authorized Responder must include the value id-kp-OCSPSigning as
// part of the extended key usage extension.
- if (!responder_certificate->has_extended_key_usage())
+ if (!responder_certificate->has_extended_key_usage()) {
return false;
+ }
for (const auto &key_purpose_oid :
responder_certificate->extended_key_usage()) {
- if (key_purpose_oid == der::Input(kOCSPSigning))
+ if (key_purpose_oid == der::Input(kOCSPSigning)) {
return true;
+ }
}
return false;
}
@@ -636,8 +702,9 @@
OCSPParseCertificate(responder_cert_tlv.AsStringView());
// If failed parsing the certificate, keep looking.
- if (!cur_responder_certificate)
+ if (!cur_responder_certificate) {
continue;
+ }
// If the certificate doesn't match the OCSP's responder ID, keep looking.
if (!CheckResponderIDMatchesCertificate(response_data.responder_id,
@@ -710,8 +777,9 @@
for (const auto &ext : extensions) {
// The CT OCSP extension is handled in ct::ExtractSCTListFromOCSPResponse
- if (ext.second.oid == ct_ext_oid)
+ if (ext.second.oid == ct_ext_oid) {
continue;
+ }
// TODO: handle SingleResponse extensions
@@ -742,8 +810,9 @@
// certificate. A SingleResponse matches a certificate if it has the same
// serial number, issuer name (hash), and issuer public key (hash).
OCSPSingleResponse single_response;
- if (!ParseOCSPSingleResponse(single_response_der, &single_response))
+ if (!ParseOCSPSingleResponse(single_response_der, &single_response)) {
return OCSPRevocationStatus::UNKNOWN;
+ }
// Reject unhandled critical extensions in SingleResponse
if (single_response.has_extensions &&
@@ -753,10 +822,12 @@
}
OCSPCertID cert_id;
- if (!ParseOCSPCertID(single_response.cert_id_tlv, &cert_id))
+ if (!ParseOCSPCertID(single_response.cert_id_tlv, &cert_id)) {
return OCSPRevocationStatus::UNKNOWN;
- if (!CheckCertIDMatchesCertificate(cert_id, cert, issuer_certificate))
+ }
+ if (!CheckCertIDMatchesCertificate(cert_id, cert, issuer_certificate)) {
continue;
+ }
// The SingleResponse matches the certificate, but may be out of date. Out
// of date responses are noted seperate from responses with mismatched
@@ -768,8 +839,9 @@
? &single_response.next_update
: nullptr,
verify_time_epoch_seconds, max_age_seconds)) {
- if (*response_details != OCSPVerifyResult::PROVIDED)
+ if (*response_details != OCSPVerifyResult::PROVIDED) {
*response_details = OCSPVerifyResult::INVALID_DATE;
+ }
continue;
}
@@ -921,8 +993,9 @@
// number doesn't matter for correctness.
const size_t kInitialBufferSize = 100;
- if (!CBB_init(cbb.get(), kInitialBufferSize))
+ if (!CBB_init(cbb.get(), kInitialBufferSize)) {
return false;
+ }
// OCSPRequest ::= SEQUENCE {
// tbsRequest TBSRequest,
@@ -934,29 +1007,34 @@
// requestList SEQUENCE OF Request,
// requestExtensions [2] EXPLICIT Extensions OPTIONAL }
CBB ocsp_request;
- if (!CBB_add_asn1(cbb.get(), &ocsp_request, CBS_ASN1_SEQUENCE))
+ if (!CBB_add_asn1(cbb.get(), &ocsp_request, CBS_ASN1_SEQUENCE)) {
return false;
+ }
CBB tbs_request;
- if (!CBB_add_asn1(&ocsp_request, &tbs_request, CBS_ASN1_SEQUENCE))
+ if (!CBB_add_asn1(&ocsp_request, &tbs_request, CBS_ASN1_SEQUENCE)) {
return false;
+ }
// "version", "requestorName", and "requestExtensions" are omitted.
CBB request_list;
- if (!CBB_add_asn1(&tbs_request, &request_list, CBS_ASN1_SEQUENCE))
+ if (!CBB_add_asn1(&tbs_request, &request_list, CBS_ASN1_SEQUENCE)) {
return false;
+ }
CBB request;
- if (!CBB_add_asn1(&request_list, &request, CBS_ASN1_SEQUENCE))
+ if (!CBB_add_asn1(&request_list, &request, CBS_ASN1_SEQUENCE)) {
return false;
+ }
// Request ::= SEQUENCE {
// reqCert CertID,
// singleRequestExtensions [0] EXPLICIT Extensions OPTIONAL }
CBB req_cert;
- if (!CBB_add_asn1(&request, &req_cert, CBS_ASN1_SEQUENCE))
+ if (!CBB_add_asn1(&request, &req_cert, CBS_ASN1_SEQUENCE)) {
return false;
+ }
// CertID ::= SEQUENCE {
// hashAlgorithm AlgorithmIdentifier,
@@ -966,19 +1044,22 @@
// TODO(eroman): Don't use SHA1.
const EVP_MD *md = EVP_sha1();
- if (!EVP_marshal_digest_algorithm(&req_cert, md))
+ if (!EVP_marshal_digest_algorithm(&req_cert, md)) {
return false;
+ }
AppendHashAsOctetString(md, &req_cert, issuer->tbs().subject_tlv);
der::Input key_tlv;
- if (!GetSubjectPublicKeyBytes(issuer->tbs().spki_tlv, &key_tlv))
+ if (!GetSubjectPublicKeyBytes(issuer->tbs().spki_tlv, &key_tlv)) {
return false;
+ }
AppendHashAsOctetString(md, &req_cert, key_tlv);
CBB serial_number;
- if (!CBB_add_asn1(&req_cert, &serial_number, CBS_ASN1_INTEGER))
+ if (!CBB_add_asn1(&req_cert, &serial_number, CBS_ASN1_INTEGER)) {
return false;
+ }
if (!CBB_add_bytes(&serial_number, cert->tbs().serial_number.UnsafeData(),
cert->tbs().serial_number.Length())) {
return false;
@@ -986,8 +1067,9 @@
uint8_t *result_bytes;
size_t result_bytes_length;
- if (!CBB_finish(cbb.get(), &result_bytes, &result_bytes_length))
+ if (!CBB_finish(cbb.get(), &result_bytes, &result_bytes_length)) {
return false;
+ }
bssl::UniquePtr<uint8_t> delete_tbs_cert_bytes(result_bytes);
request_der->assign(result_bytes, result_bytes + result_bytes_length);
diff --git a/pki/ocsp_verify_result.cc b/pki/ocsp_verify_result.cc
index 30ab6ca..f343f8e 100644
--- a/pki/ocsp_verify_result.cc
+++ b/pki/ocsp_verify_result.cc
@@ -11,8 +11,9 @@
OCSPVerifyResult::~OCSPVerifyResult() = default;
bool OCSPVerifyResult::operator==(const OCSPVerifyResult &other) const {
- if (response_status != other.response_status)
+ if (response_status != other.response_status) {
return false;
+ }
if (response_status == PROVIDED) {
// |revocation_status| is only defined when |response_status| is PROVIDED.
diff --git a/pki/parse_certificate.cc b/pki/parse_certificate.cc
index 6894b23..ab06c93 100644
--- a/pki/parse_certificate.cc
+++ b/pki/parse_certificate.cc
@@ -76,8 +76,9 @@
[[nodiscard]] bool IsSequenceTLV(const der::Input &input) {
der::Parser parser(input);
der::Parser unused_sequence_parser;
- if (!parser.ReadSequence(&unused_sequence_parser))
+ if (!parser.ReadSequence(&unused_sequence_parser)) {
return false;
+ }
// Should by a single SEQUENCE by definition of the function.
return !parser.HasMore();
}
@@ -103,8 +104,9 @@
CertificateVersion *version) {
der::Parser parser(in);
uint64_t version64;
- if (!parser.ReadUint64(&version64))
+ if (!parser.ReadUint64(&version64)) {
return false;
+ }
switch (version64) {
case 0:
@@ -159,8 +161,9 @@
CertErrors errors;
distribution_point->distribution_point_fullname =
GeneralNames::CreateFromValue(*der_full_name, &errors);
- if (!distribution_point->distribution_point_fullname)
+ if (!distribution_point->distribution_point_fullname) {
return false;
+ }
return !parser.HasMore();
}
@@ -191,8 +194,9 @@
// DistributionPoint ::= SEQUENCE {
der::Parser distrib_point_parser;
- if (!parser->ReadSequence(&distrib_point_parser))
+ if (!parser->ReadSequence(&distrib_point_parser)) {
return false;
+ }
// distributionPoint [0] DistributionPointName OPTIONAL,
std::optional<der::Input> distribution_point_name;
@@ -224,11 +228,13 @@
// RFC 5280, section 4.2.1.13:
// either distributionPoint or cRLIssuer MUST be present.
- if (!distribution_point_name && !distribution_point.crl_issuer)
+ if (!distribution_point_name && !distribution_point.crl_issuer) {
return false;
+ }
- if (distrib_point_parser.HasMore())
+ if (distrib_point_parser.HasMore()) {
return false;
+ }
distribution_points->push_back(std::move(distribution_point));
return true;
@@ -261,8 +267,9 @@
// Note: Non-conforming CAs may issue certificates with serial numbers
// that are negative or zero. Certificate users SHOULD be prepared to
// gracefully handle such certificates.
- if (negative)
+ if (negative) {
errors->AddWarning(kSerialNumberIsNegative);
+ }
if (value.Length() == 1 && value[0] == 0) {
errors->AddWarning(kSerialNumberIsZero);
}
@@ -285,14 +292,17 @@
der::Input value;
der::Tag tag;
- if (!parser->ReadTagAndValue(&tag, &value))
+ if (!parser->ReadTagAndValue(&tag, &value)) {
return false;
+ }
- if (tag == der::kUtcTime)
+ if (tag == der::kUtcTime) {
return der::ParseUTCTime(value, out);
+ }
- if (tag == der::kGeneralizedTime)
+ if (tag == der::kGeneralizedTime) {
return der::ParseGeneralizedTime(value, out);
+ }
// Unrecognized tag.
return false;
@@ -305,25 +315,30 @@
// Validity ::= SEQUENCE {
der::Parser validity_parser;
- if (!parser.ReadSequence(&validity_parser))
+ if (!parser.ReadSequence(&validity_parser)) {
return false;
+ }
// notBefore Time,
- if (!ReadUTCOrGeneralizedTime(&validity_parser, not_before))
+ if (!ReadUTCOrGeneralizedTime(&validity_parser, not_before)) {
return false;
+ }
// notAfter Time }
- if (!ReadUTCOrGeneralizedTime(&validity_parser, not_after))
+ if (!ReadUTCOrGeneralizedTime(&validity_parser, not_after)) {
return false;
+ }
// By definition the input was a single Validity sequence, so there shouldn't
// be unconsumed data.
- if (parser.HasMore())
+ if (parser.HasMore()) {
return false;
+ }
// The Validity type does not have an extension point.
- if (validity_parser.HasMore())
+ if (validity_parser.HasMore()) {
return false;
+ }
// Note that RFC 5280 doesn't require notBefore to be <=
// notAfter, so that will not be considered a "parsing" error here. Instead it
@@ -340,8 +355,9 @@
// |out_errors| is optional. But ensure it is non-null for the remainder of
// this function.
CertErrors unused_errors;
- if (!out_errors)
+ if (!out_errors) {
out_errors = &unused_errors;
+ }
der::Parser parser(certificate_tlv);
@@ -411,8 +427,9 @@
ParsedTbsCertificate *out, CertErrors *errors) {
// The rest of this function assumes that |errors| is non-null.
CertErrors unused_errors;
- if (!errors)
+ if (!errors) {
errors = &unused_errors;
+ }
// TODO(crbug.com/634443): Add useful error information to |errors|.
@@ -456,8 +473,9 @@
options.allow_invalid_serial_numbers, errors)) {
// Invalid serial numbers are only considered fatal failures if
// |!allow_invalid_serial_numbers|.
- if (!options.allow_invalid_serial_numbers)
+ if (!options.allow_invalid_serial_numbers) {
return false;
+ }
}
// signature AlgorithmIdentifier,
@@ -572,8 +590,9 @@
// By definition the input was a single TBSCertificate, so there shouldn't be
// unconsumed data.
- if (parser.HasMore())
+ if (parser.HasMore()) {
return false;
+ }
return true;
}
@@ -593,39 +612,47 @@
// Extension ::= SEQUENCE {
der::Parser extension_parser;
- if (!parser.ReadSequence(&extension_parser))
+ if (!parser.ReadSequence(&extension_parser)) {
return false;
+ }
// extnID OBJECT IDENTIFIER,
- if (!extension_parser.ReadTag(der::kOid, &out->oid))
+ if (!extension_parser.ReadTag(der::kOid, &out->oid)) {
return false;
+ }
// critical BOOLEAN DEFAULT FALSE,
out->critical = false;
bool has_critical;
der::Input critical;
- if (!extension_parser.ReadOptionalTag(der::kBool, &critical, &has_critical))
+ if (!extension_parser.ReadOptionalTag(der::kBool, &critical, &has_critical)) {
return false;
+ }
if (has_critical) {
- if (!der::ParseBool(critical, &out->critical))
+ if (!der::ParseBool(critical, &out->critical)) {
return false;
- if (!out->critical)
+ }
+ if (!out->critical) {
return false; // DER-encoding requires DEFAULT values be omitted.
+ }
}
// extnValue OCTET STRING
- if (!extension_parser.ReadTag(der::kOctetString, &out->value))
+ if (!extension_parser.ReadTag(der::kOctetString, &out->value)) {
return false;
+ }
// The Extension type does not have an extension point (everything goes in
// extnValue).
- if (extension_parser.HasMore())
+ if (extension_parser.HasMore()) {
return false;
+ }
// By definition the input was a single Extension sequence, so there shouldn't
// be unconsumed data.
- if (parser.HasMore())
+ if (parser.HasMore()) {
return false;
+ }
return true;
}
@@ -637,13 +664,15 @@
// Extensions ::= SEQUENCE SIZE (1..MAX) OF Extension
der::Parser extensions_parser;
- if (!parser.ReadSequence(&extensions_parser))
+ if (!parser.ReadSequence(&extensions_parser)) {
return false;
+ }
// The Extensions SEQUENCE must contains at least 1 element (otherwise it
// should have been omitted).
- if (!extensions_parser.HasMore())
+ if (!extensions_parser.HasMore()) {
return false;
+ }
extensions->clear();
@@ -651,24 +680,28 @@
ParsedExtension extension;
der::Input extension_tlv;
- if (!extensions_parser.ReadRawTLV(&extension_tlv))
+ if (!extensions_parser.ReadRawTLV(&extension_tlv)) {
return false;
+ }
- if (!ParseExtension(extension_tlv, &extension))
+ if (!ParseExtension(extension_tlv, &extension)) {
return false;
+ }
bool is_duplicate =
!extensions->insert(std::make_pair(extension.oid, extension)).second;
// RFC 5280 says that an extension should not appear more than once.
- if (is_duplicate)
+ if (is_duplicate) {
return false;
+ }
}
// By definition the input was a single Extensions sequence, so there
// shouldn't be unconsumed data.
- if (parser.HasMore())
+ if (parser.HasMore()) {
return false;
+ }
return true;
}
@@ -678,8 +711,9 @@
std::map<der::Input, ParsedExtension> *unconsumed_extensions,
ParsedExtension *extension) {
auto it = unconsumed_extensions->find(oid);
- if (it == unconsumed_extensions->end())
+ if (it == unconsumed_extensions->end()) {
return false;
+ }
*extension = it->second;
unconsumed_extensions->erase(it);
@@ -692,18 +726,21 @@
// BasicConstraints ::= SEQUENCE {
der::Parser sequence_parser;
- if (!parser.ReadSequence(&sequence_parser))
+ if (!parser.ReadSequence(&sequence_parser)) {
return false;
+ }
// cA BOOLEAN DEFAULT FALSE,
out->is_ca = false;
bool has_ca;
der::Input ca;
- if (!sequence_parser.ReadOptionalTag(der::kBool, &ca, &has_ca))
+ if (!sequence_parser.ReadOptionalTag(der::kBool, &ca, &has_ca)) {
return false;
+ }
if (has_ca) {
- if (!der::ParseBool(ca, &out->is_ca))
+ if (!der::ParseBool(ca, &out->is_ca)) {
return false;
+ }
// TODO(eroman): Should reject if CA was set to false, since
// DER-encoding requires DEFAULT values be omitted. In
// practice however there are a lot of certificates that use
@@ -718,21 +755,24 @@
}
if (out->has_path_len) {
// TODO(eroman): Surface reason for failure if length was longer than uint8.
- if (!der::ParseUint8(encoded_path_len, &out->path_len))
+ if (!der::ParseUint8(encoded_path_len, &out->path_len)) {
return false;
+ }
} else {
// Default initialize to 0 as a precaution.
out->path_len = 0;
}
// There shouldn't be any unconsumed data in the extension.
- if (sequence_parser.HasMore())
+ if (sequence_parser.HasMore()) {
return false;
+ }
// By definition the input was a single BasicConstraints sequence, so there
// shouldn't be unconsumed data.
- if (parser.HasMore())
+ if (parser.HasMore()) {
return false;
+ }
return true;
}
@@ -742,19 +782,22 @@
bool ParseKeyUsage(const der::Input &key_usage_tlv, der::BitString *key_usage) {
der::Parser parser(key_usage_tlv);
std::optional<der::BitString> key_usage_internal = parser.ReadBitString();
- if (!key_usage_internal)
+ if (!key_usage_internal) {
return false;
+ }
// By definition the input was a single BIT STRING.
- if (parser.HasMore())
+ if (parser.HasMore()) {
return false;
+ }
// RFC 5280 section 4.2.1.3:
//
// When the keyUsage extension appears in a certificate, at least
// one of the bits MUST be set to 1.
- if (BitStringIsAllZeros(key_usage_internal.value()))
+ if (BitStringIsAllZeros(key_usage_internal.value())) {
return false;
+ }
*key_usage = key_usage_internal.value();
return true;
@@ -770,18 +813,21 @@
// AuthorityInfoAccessSyntax ::=
// SEQUENCE SIZE (1..MAX) OF AccessDescription
der::Parser sequence_parser;
- if (!parser.ReadSequence(&sequence_parser))
+ if (!parser.ReadSequence(&sequence_parser)) {
return false;
- if (!sequence_parser.HasMore())
+ }
+ if (!sequence_parser.HasMore()) {
return false;
+ }
while (sequence_parser.HasMore()) {
AuthorityInfoAccessDescription access_description;
// AccessDescription ::= SEQUENCE {
der::Parser access_description_sequence_parser;
- if (!sequence_parser.ReadSequence(&access_description_sequence_parser))
+ if (!sequence_parser.ReadSequence(&access_description_sequence_parser)) {
return false;
+ }
// accessMethod OBJECT IDENTIFIER,
if (!access_description_sequence_parser.ReadTag(
@@ -795,8 +841,9 @@
return false;
}
- if (access_description_sequence_parser.HasMore())
+ if (access_description_sequence_parser.HasMore()) {
return false;
+ }
out_access_descriptions->push_back(access_description);
}
@@ -827,13 +874,16 @@
if (access_location_tag == der::ContextSpecificPrimitive(6)) {
// uniformResourceIdentifier [6] IA5String,
std::string_view uri = access_location_value.AsStringView();
- if (!bssl::string_util::IsAscii(uri))
+ if (!bssl::string_util::IsAscii(uri)) {
return false;
+ }
- if (access_description.access_method_oid == der::Input(kAdCaIssuersOid))
+ if (access_description.access_method_oid == der::Input(kAdCaIssuersOid)) {
out_ca_issuers_uris->push_back(uri);
- else if (access_description.access_method_oid == der::Input(kAdOcspOid))
+ } else if (access_description.access_method_oid ==
+ der::Input(kAdOcspOid)) {
out_ocsp_uris->push_back(uri);
+ }
}
}
return true;
@@ -854,19 +904,23 @@
// CRLDistributionPoints ::= SEQUENCE SIZE (1..MAX) OF DistributionPoint
der::Parser extension_value_parser(extension_value);
der::Parser distribution_points_parser;
- if (!extension_value_parser.ReadSequence(&distribution_points_parser))
+ if (!extension_value_parser.ReadSequence(&distribution_points_parser)) {
return false;
- if (extension_value_parser.HasMore())
+ }
+ if (extension_value_parser.HasMore()) {
return false;
+ }
// Sequence must have a minimum of 1 item.
- if (!distribution_points_parser.HasMore())
+ if (!distribution_points_parser.HasMore()) {
return false;
+ }
while (distribution_points_parser.HasMore()) {
if (!ParseAndAddDistributionPoint(&distribution_points_parser,
- distribution_points))
+ distribution_points)) {
return false;
+ }
}
return true;
@@ -892,10 +946,12 @@
der::Parser extension_value_parser(extension_value);
der::Parser aki_parser;
- if (!extension_value_parser.ReadSequence(&aki_parser))
+ if (!extension_value_parser.ReadSequence(&aki_parser)) {
return false;
- if (extension_value_parser.HasMore())
+ }
+ if (extension_value_parser.HasMore()) {
return false;
+ }
// TODO(mattm): Should having an empty AuthorityKeyIdentifier SEQUENCE be an
// error? RFC 5280 doesn't explicitly say it.
@@ -929,8 +985,9 @@
// There shouldn't be any unconsumed data in the AuthorityKeyIdentifier
// SEQUENCE.
- if (aki_parser.HasMore())
+ if (aki_parser.HasMore()) {
return false;
+ }
return true;
}
@@ -947,8 +1004,9 @@
}
// There shouldn't be any unconsumed data in the extension SEQUENCE.
- if (extension_value_parser.HasMore())
+ if (extension_value_parser.HasMore()) {
return false;
+ }
return true;
}
diff --git a/pki/parse_certificate_unittest.cc b/pki/parse_certificate_unittest.cc
index 6537e76..ec3c642 100644
--- a/pki/parse_certificate_unittest.cc
+++ b/pki/parse_certificate_unittest.cc
@@ -172,8 +172,9 @@
EXPECT_EQ(expected_result, actual_result);
VerifyCertErrors(expected_errors, errors, test_file_path);
- if (!expected_result || !actual_result)
+ if (!expected_result || !actual_result) {
return;
+ }
// Ensure that the ParsedTbsCertificate matches expectations.
EXPECT_EQ(expected_version, parsed.version);
@@ -673,12 +674,14 @@
cert_bytes.size(), nullptr)),
{}, &errors);
- if (!cert)
+ if (!cert) {
return false;
+ }
auto it = cert->extensions().find(der::Input(kCrlDistributionPointsOid));
- if (it == cert->extensions().end())
+ if (it == cert->extensions().end()) {
return false;
+ }
der::Input crl_dp_tlv = it->second.value;
diff --git a/pki/parse_name.cc b/pki/parse_name.cc
index c961867..443bc09 100644
--- a/pki/parse_name.cc
+++ b/pki/parse_name.cc
@@ -21,8 +21,9 @@
CBS cbs;
CBS_init(&cbs, oid.UnsafeData(), oid.Length());
bssl::UniquePtr<char> text(CBS_asn1_oid_to_text(&cbs));
- if (!text)
+ if (!text) {
return std::string();
+ }
return text.get();
}
@@ -100,16 +101,18 @@
type_string = "emailAddress";
} else {
type_string = OidToString(type);
- if (type_string.empty())
+ if (type_string.empty()) {
return false;
+ }
value_string =
"#" + bssl::string_util::HexEncode(value.UnsafeData(), value.Length());
}
if (value_string.empty()) {
std::string unescaped;
- if (!ValueAsStringUnsafe(&unescaped))
+ if (!ValueAsStringUnsafe(&unescaped)) {
return false;
+ }
bool nonprintable = false;
for (unsigned int i = 0; i < unescaped.length(); ++i) {
@@ -138,9 +141,10 @@
// If we have non-printable characters in a TeletexString, we hex encode
// since we don't handle Teletex control codes.
- if (nonprintable && value_tag == der::kTeletexString)
+ if (nonprintable && value_tag == der::kTeletexString) {
value_string = "#" + bssl::string_util::HexEncode(value.UnsafeData(),
value.Length());
+ }
}
*out = type_string + "=" + value_string;
@@ -150,23 +154,27 @@
bool ReadRdn(der::Parser *parser, RelativeDistinguishedName *out) {
while (parser->HasMore()) {
der::Parser attr_type_and_value;
- if (!parser->ReadSequence(&attr_type_and_value))
+ if (!parser->ReadSequence(&attr_type_and_value)) {
return false;
+ }
// Read the attribute type, which must be an OBJECT IDENTIFIER.
der::Input type;
- if (!attr_type_and_value.ReadTag(der::kOid, &type))
+ if (!attr_type_and_value.ReadTag(der::kOid, &type)) {
return false;
+ }
// Read the attribute value.
der::Tag tag;
der::Input value;
- if (!attr_type_and_value.ReadTagAndValue(&tag, &value))
+ if (!attr_type_and_value.ReadTagAndValue(&tag, &value)) {
return false;
+ }
// There should be no more elements in the sequence after reading the
// attribute type and value.
- if (attr_type_and_value.HasMore())
+ if (attr_type_and_value.HasMore()) {
return false;
+ }
out->push_back(X509NameAttribute(type, tag, value));
}
@@ -179,8 +187,9 @@
bool ParseName(const der::Input &name_tlv, RDNSequence *out) {
der::Parser name_parser(name_tlv);
der::Input name_value;
- if (!name_parser.ReadTag(der::kSequence, &name_value))
+ if (!name_parser.ReadTag(der::kSequence, &name_value)) {
return false;
+ }
return ParseNameValue(name_value, out);
}
@@ -188,11 +197,13 @@
der::Parser rdn_sequence_parser(name_value);
while (rdn_sequence_parser.HasMore()) {
der::Parser rdn_parser;
- if (!rdn_sequence_parser.ReadConstructed(der::kSet, &rdn_parser))
+ if (!rdn_sequence_parser.ReadConstructed(der::kSet, &rdn_parser)) {
return false;
+ }
RelativeDistinguishedName type_and_values;
- if (!ReadRdn(&rdn_parser, &type_and_values))
+ if (!ReadRdn(&rdn_parser, &type_and_values)) {
return false;
+ }
out->push_back(type_and_values);
}
@@ -206,15 +217,18 @@
RelativeDistinguishedName rdn = rdn_sequence[size - i - 1];
std::string rdn_string;
for (const auto &atv : rdn) {
- if (!rdn_string.empty())
+ if (!rdn_string.empty()) {
rdn_string += "+";
+ }
std::string atv_string;
- if (!atv.AsRFC2253String(&atv_string))
+ if (!atv.AsRFC2253String(&atv_string)) {
return false;
+ }
rdn_string += atv_string;
}
- if (!rdns_string.empty())
+ if (!rdns_string.empty()) {
rdns_string += ",";
+ }
rdns_string += rdn_string;
}
diff --git a/pki/parse_values.cc b/pki/parse_values.cc
index 13e2c20..cc7a072 100644
--- a/pki/parse_values.cc
+++ b/pki/parse_values.cc
@@ -20,12 +20,14 @@
// According to ITU-T X.690 section 8.2, a bool is encoded as a single octet
// where the octet of all zeroes is FALSE and a non-zero value for the octet
// is TRUE.
- if (in.Length() != 1)
+ if (in.Length() != 1) {
return false;
+ }
ByteReader data(in);
uint8_t byte;
- if (!data.ReadByte(&byte))
+ if (!data.ReadByte(&byte)) {
return false;
+ }
if (byte == 0) {
*out = false;
return true;
@@ -67,10 +69,12 @@
// 0 and 60 (to allow for leap seconds; no validation is done that a leap
// second is on a day that could be a leap second).
bool ValidateGeneralizedTime(const GeneralizedTime &time) {
- if (time.month < 1 || time.month > 12)
+ if (time.month < 1 || time.month > 12) {
return false;
- if (time.day < 1)
+ }
+ if (time.day < 1) {
return false;
+ }
if (time.hours > 23) {
return false;
}
@@ -88,8 +92,9 @@
case 6:
case 9:
case 11:
- if (time.day > 30)
+ if (time.day > 30) {
return false;
+ }
break;
case 1:
case 3:
@@ -98,17 +103,20 @@
case 8:
case 10:
case 12:
- if (time.day > 31)
+ if (time.day > 31) {
return false;
+ }
break;
case 2:
if (time.year % 4 == 0 &&
(time.year % 100 != 0 || time.year % 400 == 0)) {
- if (time.day > 29)
+ if (time.day > 29) {
return false;
+ }
} else {
- if (time.day > 28)
+ if (time.day > 28) {
return false;
+ }
}
break;
default:
@@ -129,11 +137,13 @@
size_t GetUnsignedIntegerLength(const Input &in) {
der::ByteReader reader(in);
uint8_t first_byte;
- if (!reader.ReadByte(&first_byte))
+ if (!reader.ReadByte(&first_byte)) {
return 0; // Not valid DER as |in| was empty.
+ }
- if (first_byte == 0 && in.Length() > 1)
+ if (first_byte == 0 && in.Length() > 1) {
return in.Length() - 1;
+ }
return in.Length();
}
@@ -170,12 +180,14 @@
bool ParseUint64(const Input &in, uint64_t *out) {
// Reject non-minimally encoded numbers and negative numbers.
bool negative;
- if (!IsValidInteger(in, &negative) || negative)
+ if (!IsValidInteger(in, &negative) || negative) {
return false;
+ }
// Reject (non-negative) integers whose value would overflow the output type.
- if (GetUnsignedIntegerLength(in) > sizeof(*out))
+ if (GetUnsignedIntegerLength(in) > sizeof(*out)) {
return false;
+ }
ByteReader reader(in);
uint8_t data;
@@ -192,11 +204,13 @@
bool ParseUint8(const Input &in, uint8_t *out) {
// TODO(eroman): Implement this more directly.
uint64_t value;
- if (!ParseUint64(in, &value))
+ if (!ParseUint64(in, &value)) {
return false;
+ }
- if (value > 0xFF)
+ if (value > 0xFF) {
return false;
+ }
*out = static_cast<uint8_t>(value);
return true;
@@ -217,8 +231,9 @@
// If the bit is outside of the bitstring, by definition it is not
// asserted.
- if (byte_index >= bytes_.Length())
+ if (byte_index >= bytes_.Length()) {
return false;
+ }
// Within a byte, bits are ordered from most significant to least significant.
// Convert |bit_index| to an index within the |byte_index| byte, measured from
@@ -241,14 +256,17 @@
// bit 1 as the least significant bit, the number of unused bits in the final
// subsequent octet. The number shall be in the range zero to seven.
uint8_t unused_bits;
- if (!reader.ReadByte(&unused_bits))
+ if (!reader.ReadByte(&unused_bits)) {
return std::nullopt;
- if (unused_bits > 7)
+ }
+ if (unused_bits > 7) {
return std::nullopt;
+ }
Input bytes;
- if (!reader.ReadBytes(reader.BytesLeft(), &bytes))
+ if (!reader.ReadBytes(reader.BytesLeft(), &bytes)) {
return std::nullopt; // Not reachable.
+ }
// Ensure that unused bits in the last byte are set to 0.
if (unused_bits > 0) {
@@ -256,8 +274,9 @@
//
// If the bitstring is empty, there shall be no subsequent octets,
// and the initial octet shall be zero.
- if (bytes.Length() == 0)
+ if (bytes.Length() == 0) {
return std::nullopt;
+ }
uint8_t last_byte = bytes[bytes.Length() - 1];
// From ITU-T X.690, section 11.2.1 (applies to CER and DER, but not BER):
@@ -265,8 +284,9 @@
// Each unused bit in the final octet of the encoding of a bit string value
// shall be set to zero.
uint8_t mask = 0xFF >> (8 - unused_bits);
- if ((mask & last_byte) != 0)
+ if ((mask & last_byte) != 0) {
return std::nullopt;
+ }
}
return BitString(bytes, unused_bits);
@@ -306,15 +326,17 @@
return false;
}
uint8_t zulu;
- if (!reader.ReadByte(&zulu) || zulu != 'Z' || reader.HasMore())
+ if (!reader.ReadByte(&zulu) || zulu != 'Z' || reader.HasMore()) {
return false;
+ }
if (time.year < 50) {
time.year += 2000;
} else {
time.year += 1900;
}
- if (!ValidateGeneralizedTime(time))
+ if (!ValidateGeneralizedTime(time)) {
return false;
+ }
*value = time;
return true;
}
@@ -331,18 +353,21 @@
return false;
}
uint8_t zulu;
- if (!reader.ReadByte(&zulu) || zulu != 'Z' || reader.HasMore())
+ if (!reader.ReadByte(&zulu) || zulu != 'Z' || reader.HasMore()) {
return false;
- if (!ValidateGeneralizedTime(time))
+ }
+ if (!ValidateGeneralizedTime(time)) {
return false;
+ }
*value = time;
return true;
}
bool ParseIA5String(Input in, std::string *out) {
for (char c : in.AsStringView()) {
- if (static_cast<uint8_t>(c) > 127)
+ if (static_cast<uint8_t>(c) > 127) {
return false;
+ }
}
*out = in.AsString();
return true;
@@ -356,8 +381,9 @@
// "for VisibleString [the range] is 32 to 126 ... For VisibleString .. all
// the values in the range are present."
for (char c : in.AsStringView()) {
- if (static_cast<uint8_t>(c) < 32 || static_cast<uint8_t>(c) > 126)
+ if (static_cast<uint8_t>(c) < 32 || static_cast<uint8_t>(c) > 126) {
return false;
+ }
}
*out = in.AsString();
return true;
diff --git a/pki/parsed_certificate.cc b/pki/parsed_certificate.cc
index 5f2406d..0e8146c 100644
--- a/pki/parsed_certificate.cc
+++ b/pki/parsed_certificate.cc
@@ -58,8 +58,9 @@
bool ParsedCertificate::GetExtension(const der::Input &extension_oid,
ParsedExtension *parsed_extension) const {
- if (!tbs_.extensions_tlv)
+ if (!tbs_.extensions_tlv) {
return false;
+ }
auto it = extensions_.find(extension_oid);
if (it == extensions_.end()) {
@@ -81,8 +82,9 @@
// |errors| is an optional parameter, but to keep the code simpler, use a
// dummy object when one wasn't provided.
CertErrors unused_errors;
- if (!errors)
+ if (!errors) {
errors = &unused_errors;
+ }
auto result = std::make_shared<ParsedCertificate>(PrivateConstructor{});
result->cert_data_ = std::move(backing_data);
@@ -284,8 +286,9 @@
CertErrors *errors) {
std::shared_ptr<const ParsedCertificate> cert(
Create(std::move(cert_data), options, errors));
- if (!cert)
+ if (!cert) {
return false;
+ }
chain->push_back(std::move(cert));
return true;
}
diff --git a/pki/parsed_certificate_unittest.cc b/pki/parsed_certificate_unittest.cc
index 95f4433..33cd078 100644
--- a/pki/parsed_certificate_unittest.cc
+++ b/pki/parsed_certificate_unittest.cc
@@ -48,8 +48,9 @@
// The errors are baselined for |!allow_invalid_serial_numbers|. So if
// requesting a non-default option skip the error checks.
// TODO(eroman): This is ugly.
- if (!options.allow_invalid_serial_numbers)
+ if (!options.allow_invalid_serial_numbers) {
VerifyCertErrors(expected_errors, errors, test_file_path);
+ }
// Every parse failure being tested should emit error information.
if (!cert) {
diff --git a/pki/parser.cc b/pki/parser.cc
index ae6a201..327c239 100644
--- a/pki/parser.cc
+++ b/pki/parser.cc
@@ -31,8 +31,9 @@
}
bool Parser::Advance() {
- if (advance_len_ == 0)
+ if (advance_len_ == 0) {
return false;
+ }
bool ret = !!CBS_skip(&cbs_, advance_len_);
advance_len_ = 0;
return ret;
@@ -42,15 +43,17 @@
bool Parser::ReadRawTLV(Input *out) {
CBS tmp_out;
- if (!CBS_get_any_asn1_element(&cbs_, &tmp_out, nullptr, nullptr))
+ if (!CBS_get_any_asn1_element(&cbs_, &tmp_out, nullptr, nullptr)) {
return false;
+ }
*out = Input(CBS_data(&tmp_out), CBS_len(&tmp_out));
return true;
}
bool Parser::ReadTagAndValue(Tag *tag, Input *out) {
- if (!PeekTagAndValue(tag, out))
+ if (!PeekTagAndValue(tag, out)) {
return false;
+ }
BSSL_CHECK(Advance());
return true;
}
@@ -77,8 +80,9 @@
bool Parser::ReadOptionalTag(Tag tag, Input *out, bool *present) {
std::optional<Input> tmp_out;
- if (!ReadOptionalTag(tag, &tmp_out))
+ if (!ReadOptionalTag(tag, &tmp_out)) {
return false;
+ }
*present = tmp_out.has_value();
*out = tmp_out.value_or(der::Input());
return true;
@@ -108,11 +112,13 @@
// Type-specific variants of ReadTag
bool Parser::ReadConstructed(Tag tag, Parser *out) {
- if (!IsConstructed(tag))
+ if (!IsConstructed(tag)) {
return false;
+ }
Input data;
- if (!ReadTag(tag, &data))
+ if (!ReadTag(tag, &data)) {
return false;
+ }
*out = Parser(data);
return true;
}
@@ -123,29 +129,33 @@
bool Parser::ReadUint8(uint8_t *out) {
Input encoded_int;
- if (!ReadTag(kInteger, &encoded_int))
+ if (!ReadTag(kInteger, &encoded_int)) {
return false;
+ }
return ParseUint8(encoded_int, out);
}
bool Parser::ReadUint64(uint64_t *out) {
Input encoded_int;
- if (!ReadTag(kInteger, &encoded_int))
+ if (!ReadTag(kInteger, &encoded_int)) {
return false;
+ }
return ParseUint64(encoded_int, out);
}
std::optional<BitString> Parser::ReadBitString() {
Input value;
- if (!ReadTag(kBitString, &value))
+ if (!ReadTag(kBitString, &value)) {
return std::nullopt;
+ }
return ParseBitString(value);
}
bool Parser::ReadGeneralizedTime(GeneralizedTime *out) {
Input value;
- if (!ReadTag(kGeneralizedTime, &value))
+ if (!ReadTag(kGeneralizedTime, &value)) {
return false;
+ }
return ParseGeneralizedTime(value, out);
}
diff --git a/pki/path_builder.cc b/pki/path_builder.cc
index f0e74e8..ad69ecd 100644
--- a/pki/path_builder.cc
+++ b/pki/path_builder.cc
@@ -43,8 +43,9 @@
RDNSequence subject;
std::string subject_str;
if (!ParseName(cert->tbs().subject_tlv, &subject) ||
- !ConvertToRFC2253(subject, &subject_str))
+ !ConvertToRFC2253(subject, &subject_str)) {
subject_str = "???";
+ }
return FingerPrintParsedCertificate(cert) + " " + subject_str;
}
@@ -52,8 +53,9 @@
std::string PathDebugString(const ParsedCertificateList &certs) {
std::string s;
for (const auto &cert : certs) {
- if (!s.empty())
+ if (!s.empty()) {
s += "\n";
+ }
s += " " + CertDebugString(cert.get());
}
return s;
@@ -84,8 +86,9 @@
// values indicate higer priority.
KeyIdentifierMatch CalculateKeyIdentifierMatch(
const ParsedCertificate *target, const ParsedCertificate *issuer) {
- if (!target->authority_key_identifier())
+ if (!target->authority_key_identifier()) {
return kNoData;
+ }
// TODO(crbug.com/635205): If issuer does not have a subjectKeyIdentifier,
// could try synthesizing one using the standard SHA-1 method. Ideally in a
@@ -301,8 +304,9 @@
void CertIssuersIter::AddIssuers(ParsedCertificateList new_issuers) {
for (std::shared_ptr<const ParsedCertificate> &issuer : new_issuers) {
if (present_issuers_.find(issuer->der_cert().AsStringView()) !=
- present_issuers_.end())
+ present_issuers_.end()) {
continue;
+ }
present_issuers_.insert(issuer->der_cert().AsStringView());
// Look up the trust for this issuer.
@@ -332,8 +336,9 @@
}
void CertIssuersIter::SortRemainingIssuers() {
- if (!issuers_needs_sort_)
+ if (!issuers_needs_sort_) {
return;
+ }
std::stable_sort(
issuers_.begin() + cur_issuer_, issuers_.end(),
@@ -392,8 +397,9 @@
// Copies the ParsedCertificate elements of the current path to |*out_path|.
void CopyPath(ParsedCertificateList *out_path) {
out_path->clear();
- for (const auto &node : cur_path_)
+ for (const auto &node : cur_path_) {
out_path->push_back(node->reference_cert());
+ }
}
// Returns true if the path is empty.
@@ -408,8 +414,9 @@
std::string PathDebugString() {
std::string s;
for (const auto &node : cur_path_) {
- if (!s.empty())
+ if (!s.empty()) {
s += "\n";
+ }
s += " " + CertDebugString(node->cert());
}
return s;
@@ -440,8 +447,9 @@
} // namespace
const ParsedCertificate *CertPathBuilderResultPath::GetTrustedCert() const {
- if (certs.empty())
+ if (certs.empty()) {
return nullptr;
+ }
switch (last_cert_trust.type) {
case CertificateTrustType::TRUSTED_ANCHOR:
@@ -531,8 +539,9 @@
// If the deadline is already expired before the first call to
// GetNextPath, cur_path_ will be empty. Return the leaf cert in that
// case.
- if (next_issuer_.cert)
+ if (next_issuer_.cert) {
out_certs->push_back(next_issuer_.cert);
+ }
} else {
cur_path_.CopyPath(out_certs);
}
@@ -698,8 +707,9 @@
bool CertPathBuilder::Result::AnyPathContainsError(CertErrorId error_id) const {
for (const auto &path : paths) {
- if (path->errors.ContainsError(error_id))
+ if (path->errors.ContainsError(error_id)) {
return true;
+ }
}
return false;
@@ -709,8 +719,9 @@
const {
const CertPathBuilderResultPath *result_path = GetBestPathPossiblyInvalid();
- if (result_path && result_path->IsValid())
+ if (result_path && result_path->IsValid()) {
return result_path;
+ }
return nullptr;
}
@@ -720,8 +731,9 @@
BSSL_CHECK((paths.empty() && best_result_index == 0) ||
best_result_index < paths.size());
- if (best_result_index >= paths.size())
+ if (best_result_index >= paths.size()) {
return nullptr;
+ }
return paths[best_result_index].get();
}
diff --git a/pki/path_builder_pkits_unittest.cc b/pki/path_builder_pkits_unittest.cc
index 6f3573f..e912e46 100644
--- a/pki/path_builder_pkits_unittest.cc
+++ b/pki/path_builder_pkits_unittest.cc
@@ -43,8 +43,9 @@
CertPathBuilderResultPath *path) override {
SimplePathBuilderDelegate::CheckPathAfterVerification(path_builder, path);
- if (!path->IsValid())
+ if (!path->IsValid()) {
return;
+ }
// It would be preferable if this test could use
// CheckValidatedChainRevocation somehow, but that only supports getting
@@ -56,8 +57,9 @@
// Trust anchors bypass OCSP/CRL revocation checks. (The only way to
// revoke trust anchors is via CRLSet or the built-in SPKI block list).
- if (reverse_i == 0 && path->last_cert_trust.IsTrustAnchor())
+ if (reverse_i == 0 && path->last_cert_trust.IsTrustAnchor()) {
continue;
+ }
// RFC 5280 6.3.3. [If the CRL was not specified in a distribution
// point], assume a DP with both the reasons and the
@@ -91,8 +93,9 @@
}
// If there were only DistributionPoints with reasons, just use the
// first one.
- if (cert_dp == &fake_cert_dp && !distribution_points.empty())
+ if (cert_dp == &fake_cert_dp && !distribution_points.empty()) {
cert_dp = &distribution_points[0];
+ }
}
bool cert_good = false;
@@ -154,8 +157,9 @@
// TODO(mattm): test with other irrelevant certs in cert_issuer_sources?
CertIssuerSourceStatic cert_issuer_source;
- for (size_t i = 1; i < cert_ders.size() - 1; ++i)
+ for (size_t i = 1; i < cert_ders.size() - 1; ++i) {
cert_issuer_source.AddCert(certs[i]);
+ }
std::shared_ptr<const ParsedCertificate> target_cert(certs.back());
diff --git a/pki/path_builder_unittest.cc b/pki/path_builder_unittest.cc
index dfe5a50..ebb940a 100644
--- a/pki/path_builder_unittest.cc
+++ b/pki/path_builder_unittest.cc
@@ -86,8 +86,9 @@
~StaticAsyncRequest() override = default;
void GetNext(ParsedCertificateList *out_certs) override {
- if (issuers_iter_ != issuers_.end())
+ if (issuers_iter_ != issuers_.end()) {
out_certs->push_back(std::move(*issuers_iter_++));
+ }
}
ParsedCertificateList issuers_;
@@ -142,8 +143,9 @@
std::string der;
::testing::AssertionResult r = ReadTestPem(
"testdata/ssl/certificates/" + file_name, "CERTIFICATE", &der);
- if (!r)
+ if (!r) {
return r;
+ }
CertErrors errors;
*result = ParsedCertificate::Create(
bssl::UniquePtr<CRYPTO_BUFFER>(CRYPTO_BUFFER_new(
@@ -594,11 +596,13 @@
b_by_c_, b_by_f_, f_by_e_, c_by_d_, c_by_e_};
CertIssuerSourceStatic sync_certs;
if (reverse_order) {
- for (auto it = certs.rbegin(); it != certs.rend(); ++it)
+ for (auto it = certs.rbegin(); it != certs.rend(); ++it) {
sync_certs.AddCert(*it);
+ }
} else {
- for (const auto &cert : certs)
+ for (const auto &cert : certs) {
sync_certs.AddCert(cert);
+ }
}
CertPathBuilder path_builder(
@@ -1756,10 +1760,12 @@
// Set up the trust store such that |distrusted_cert| is blocked, and
// the root is trusted (except if it was |distrusted_cert|).
TrustStoreInMemory trust_store;
- if (distrusted_cert != test_.chain.back())
+ if (distrusted_cert != test_.chain.back()) {
trust_store.AddTrustAnchor(test_.chain.back());
- if (distrusted_cert)
+ }
+ if (distrusted_cert) {
trust_store.AddDistrustedCertificateForTest(distrusted_cert);
+ }
// Add the single intermediate.
CertIssuerSourceStatic intermediates;
@@ -1817,8 +1823,9 @@
// The built path should be identical the the one read from disk.
const auto &path = *result.GetBestValidPath();
ASSERT_EQ(test_.chain.size(), path.certs.size());
- for (size_t i = 0; i < test_.chain.size(); ++i)
+ for (size_t i = 0; i < test_.chain.size(); ++i) {
EXPECT_EQ(test_.chain[i], path.certs[i]);
+ }
}
// Try path building when only the target is blocked - should fail.
diff --git a/pki/path_builder_verify_certificate_chain_unittest.cc b/pki/path_builder_verify_certificate_chain_unittest.cc
index bb0cab9..a5aeef7 100644
--- a/pki/path_builder_verify_certificate_chain_unittest.cc
+++ b/pki/path_builder_verify_certificate_chain_unittest.cc
@@ -24,8 +24,9 @@
trust_store.AddCertificate(test.chain.back(), test.last_cert_trust);
CertIssuerSourceStatic intermediate_cert_issuer_source;
- for (size_t i = 1; i < test.chain.size(); ++i)
+ for (size_t i = 1; i < test.chain.size(); ++i) {
intermediate_cert_issuer_source.AddCert(test.chain[i]);
+ }
// First cert in the |chain| is the target.
CertPathBuilder path_builder(
diff --git a/pki/pem.cc b/pki/pem.cc
index 57068d4..0bff537 100644
--- a/pki/pem.cc
+++ b/pki/pem.cc
@@ -39,14 +39,16 @@
while (pos_ != std::string_view::npos) {
// Scan for the beginning of the next PEM encoded block.
pos_ = str_.find(kPEMHeaderBeginBlock, pos_);
- if (pos_ == std::string_view::npos)
+ if (pos_ == std::string_view::npos) {
return false; // No more PEM blocks
+ }
std::vector<PEMType>::const_iterator it;
// Check to see if it is of an acceptable block type.
for (it = block_types_.begin(); it != block_types_.end(); ++it) {
- if (!bssl::string_util::StartsWith(str_.substr(pos_), it->header))
+ if (!bssl::string_util::StartsWith(str_.substr(pos_), it->header)) {
continue;
+ }
// Look for a footer matching the header. If none is found, then all
// data following this point is invalid and should not be parsed.
@@ -77,8 +79,9 @@
// continue the search. Otherwise, |pos_| has been updated to the most
// appropriate search position to continue searching from and should not
// be adjusted.
- if (it == block_types_.end())
+ if (it == block_types_.end()) {
pos_ += kPEMHeaderBeginBlock.size();
+ }
}
return false;
diff --git a/pki/signature_algorithm.cc b/pki/signature_algorithm.cc
index 83f65d1..341cae5 100644
--- a/pki/signature_algorithm.cc
+++ b/pki/signature_algorithm.cc
@@ -131,12 +131,14 @@
[[nodiscard]] bool IsNull(const der::Input &input) {
der::Parser parser(input);
der::Input null_value;
- if (!parser.ReadTag(der::kNull, &null_value))
+ if (!parser.ReadTag(der::kNull, &null_value)) {
return false;
+ }
// NULL values are TLV encoded; the value is expected to be empty.
- if (!IsEmpty(null_value))
+ if (!IsEmpty(null_value)) {
return false;
+ }
// By definition of this function, the entire input must be a NULL.
return !parser.HasMore();
@@ -175,12 +177,14 @@
DigestAlgorithm *mgf1_hash) {
der::Input oid;
der::Input params;
- if (!ParseAlgorithmIdentifier(input, &oid, ¶ms))
+ if (!ParseAlgorithmIdentifier(input, &oid, ¶ms)) {
return false;
+ }
// MGF1 is the only supported mask generation algorithm.
- if (oid != der::Input(kOidMgf1))
+ if (oid != der::Input(kOidMgf1)) {
return false;
+ }
return ParseHashAlgorithm(params, mgf1_hash);
}
@@ -275,17 +279,20 @@
der::Parser parser(input);
der::Parser algorithm_identifier_parser;
- if (!parser.ReadSequence(&algorithm_identifier_parser))
+ if (!parser.ReadSequence(&algorithm_identifier_parser)) {
return false;
+ }
// There shouldn't be anything after the sequence. This is by definition,
// as the input to this function is expected to be a single
// AlgorithmIdentifier.
- if (parser.HasMore())
+ if (parser.HasMore()) {
return false;
+ }
- if (!algorithm_identifier_parser.ReadTag(der::kOid, algorithm))
+ if (!algorithm_identifier_parser.ReadTag(der::kOid, algorithm)) {
return false;
+ }
// Read the optional parameters to a der::Input. The parameters can be at
// most one TLV (for instance NULL or a sequence).
diff --git a/pki/simple_path_builder_delegate.cc b/pki/simple_path_builder_delegate.cc
index 4dfaea8..248d640 100644
--- a/pki/simple_path_builder_delegate.cc
+++ b/pki/simple_path_builder_delegate.cc
@@ -84,8 +84,9 @@
if (pkey_id == EVP_PKEY_RSA) {
// Extract the modulus length from the key.
RSA *rsa = EVP_PKEY_get0_RSA(public_key);
- if (!rsa)
+ if (!rsa) {
return false;
+ }
unsigned int modulus_length_bits = RSA_bits(rsa);
if (modulus_length_bits < min_rsa_modulus_length_bits_) {
@@ -102,8 +103,9 @@
if (pkey_id == EVP_PKEY_EC) {
// Extract the curve name.
EC_KEY *ec = EVP_PKEY_get0_EC_KEY(public_key);
- if (!ec)
+ if (!ec) {
return false; // Unexpected.
+ }
int curve_nid = EC_GROUP_get_curve_name(EC_KEY_get0_group(ec));
if (!IsAcceptableCurveForEcdsa(curve_nid)) {
diff --git a/pki/test_helpers.cc b/pki/test_helpers.cc
index f149d32..b7369d9 100644
--- a/pki/test_helpers.cc
+++ b/pki/test_helpers.cc
@@ -28,8 +28,9 @@
bool GetValue(std::string_view prefix, std::string_view line,
std::string *value, bool *has_value) {
- if (!bssl::string_util::StartsWith(line, prefix))
+ if (!bssl::string_util::StartsWith(line, prefix)) {
return false;
+ }
if (*has_value) {
ADD_FAILURE() << "Duplicated " << prefix;
@@ -195,8 +196,9 @@
*chain = ParsedCertificateList();
std::string file_data = ReadTestFileToString(file_path_ascii);
- if (file_data.empty())
+ if (file_data.empty()) {
return false;
+ }
std::vector<std::string> pem_headers = {"CERTIFICATE"};
@@ -221,10 +223,12 @@
std::shared_ptr<const ParsedCertificate> ReadCertFromFile(
const std::string &file_path_ascii) {
ParsedCertificateList chain;
- if (!ReadCertChainFromFile(file_path_ascii, &chain))
+ if (!ReadCertChainFromFile(file_path_ascii, &chain)) {
return nullptr;
- if (chain.size() != 1)
+ }
+ if (chain.size() != 1) {
return nullptr;
+ }
return chain[0];
}
@@ -234,8 +238,9 @@
*test = {};
std::string file_data = ReadTestFileToString(file_path_ascii);
- if (file_data.empty())
+ if (file_data.empty()) {
return false;
+ }
bool has_chain = false;
bool has_trust = false;
diff --git a/pki/trust_store_collection.cc b/pki/trust_store_collection.cc
index 804b1c8..4c607b5 100644
--- a/pki/trust_store_collection.cc
+++ b/pki/trust_store_collection.cc
@@ -35,8 +35,9 @@
// last one
if (!cur_trust.HasUnspecifiedTrust()) {
result = cur_trust;
- if (result.IsDistrusted())
+ if (result.IsDistrusted()) {
break;
+ }
}
}
diff --git a/pki/trust_store_in_memory.cc b/pki/trust_store_in_memory.cc
index 40aaa1f..47fe6d4 100644
--- a/pki/trust_store_in_memory.cc
+++ b/pki/trust_store_in_memory.cc
@@ -44,8 +44,9 @@
void TrustStoreInMemory::SyncGetIssuersOf(const ParsedCertificate *cert,
ParsedCertificateList *issuers) {
auto range = entries_.equal_range(cert->normalized_issuer().AsStringView());
- for (auto it = range.first; it != range.second; ++it)
+ for (auto it = range.first; it != range.second; ++it) {
issuers->push_back(it->second.cert);
+ }
}
CertificateTrust TrustStoreInMemory::GetTrust(const ParsedCertificate *cert) {
diff --git a/pki/verify_certificate_chain.cc b/pki/verify_certificate_chain.cc
index e1d8d3f..f504ef0 100644
--- a/pki/verify_certificate_chain.cc
+++ b/pki/verify_certificate_chain.cc
@@ -25,19 +25,24 @@
bool IsHandledCriticalExtension(const ParsedExtension &extension,
const ParsedCertificate &cert) {
- if (extension.oid == der::Input(kBasicConstraintsOid))
+ if (extension.oid == der::Input(kBasicConstraintsOid)) {
return true;
+ }
// Key Usage is NOT processed for end-entity certificates (this is the
// responsibility of callers), however it is considered "handled" here in
// order to allow being marked as critical.
- if (extension.oid == der::Input(kKeyUsageOid))
+ if (extension.oid == der::Input(kKeyUsageOid)) {
return true;
- if (extension.oid == der::Input(kExtKeyUsageOid))
+ }
+ if (extension.oid == der::Input(kExtKeyUsageOid)) {
return true;
- if (extension.oid == der::Input(kNameConstraintsOid))
+ }
+ if (extension.oid == der::Input(kNameConstraintsOid)) {
return true;
- if (extension.oid == der::Input(kSubjectAltNameOid))
+ }
+ if (extension.oid == der::Input(kSubjectAltNameOid)) {
return true;
+ }
if (extension.oid == der::Input(kCertificatePoliciesOid)) {
// Policy qualifiers are skipped during processing, so if the
// extension is marked critical need to ensure there weren't any
@@ -56,12 +61,15 @@
// TODO(eroman): Give a better error message.
}
- if (extension.oid == der::Input(kPolicyMappingsOid))
+ if (extension.oid == der::Input(kPolicyMappingsOid)) {
return true;
- if (extension.oid == der::Input(kPolicyConstraintsOid))
+ }
+ if (extension.oid == der::Input(kPolicyConstraintsOid)) {
return true;
- if (extension.oid == der::Input(kInhibitAnyPolicyOid))
+ }
+ if (extension.oid == der::Input(kInhibitAnyPolicyOid)) {
return true;
+ }
if (extension.oid == der::Input(kMSApplicationPoliciesOid)) {
// Per https://crbug.com/1439638 and
// https://learn.microsoft.com/en-us/windows/win32/seccertenroll/supported-extensions#msapplicationpolicies
@@ -111,11 +119,13 @@
// notBefore through notAfter, inclusive.
void VerifyTimeValidity(const ParsedCertificate &cert,
const der::GeneralizedTime &time, CertErrors *errors) {
- if (time < cert.tbs().validity_not_before)
+ if (time < cert.tbs().validity_not_before) {
errors->AddError(cert_errors::kValidityFailedNotBefore);
+ }
- if (cert.tbs().validity_not_after < time)
+ if (cert.tbs().validity_not_after < time) {
errors->AddError(cert_errors::kValidityFailedNotAfter);
+ }
}
// Adds errors to |errors| if |cert| has internally inconsistent signature
@@ -145,8 +155,9 @@
// Ensure that the two DER-encoded signature algorithms are byte-for-byte
// equal.
- if (alg1_tlv == alg2_tlv)
+ if (alg1_tlv == alg2_tlv) {
return true;
+ }
// But make a compatibility concession if alternate encodings are used
// TODO(eroman): Turn this warning into an error.
@@ -874,19 +885,22 @@
// (e) If the certificate policies extension is not present, set the
// valid_policy_tree to NULL.
- if (!cert.has_policy_oids())
+ if (!cert.has_policy_oids()) {
valid_policy_graph_.SetNull();
+ }
// (f) Verify that either explicit_policy is greater than 0 or the
// valid_policy_tree is not equal to NULL;
- if (!((explicit_policy_ > 0) || !valid_policy_graph_.IsNull()))
+ if (!((explicit_policy_ > 0) || !valid_policy_graph_.IsNull())) {
errors->AddError(cert_errors::kNoValidPolicy);
+ }
}
void PathVerifier::VerifyPolicyMappings(const ParsedCertificate &cert,
CertErrors *errors) {
- if (!cert.has_policy_mappings())
+ if (!cert.has_policy_mappings()) {
return;
+ }
// From RFC 5280 section 6.1.4:
//
@@ -1030,8 +1044,9 @@
errors->AddError(cert_errors::kVerifySignedDataFailed);
}
}
- if (*shortcircuit_chain_validation)
+ if (*shortcircuit_chain_validation) {
return;
+ }
// Check the time range for the certificate's validity, ensuring it is valid
// at |time|.
@@ -1044,8 +1059,9 @@
// Verify the certificate's issuer name matches the issuing certificate's
// subject name. (RFC 5280 section 6.1.3 step a.4)
- if (cert.normalized_issuer() != working_normalized_issuer_name_)
+ if (cert.normalized_issuer() != working_normalized_issuer_name_) {
errors->AddError(cert_errors::kSubjectDoesNotMatchIssuer);
+ }
// Name constraints (RFC 5280 section 6.1.3 step b & c)
// If certificate i is self-issued and it is not the final certificate in the
@@ -1089,24 +1105,28 @@
// of |working_spki|.
// From RFC 5280 section 6.1.4 step g:
- if (cert.has_name_constraints())
+ if (cert.has_name_constraints()) {
name_constraints_list_.push_back(&cert.name_constraints());
+ }
// (h) If certificate i is not self-issued:
if (!IsSelfIssued(cert)) {
// (1) If explicit_policy is not 0, decrement explicit_policy by
// 1.
- if (explicit_policy_ > 0)
+ if (explicit_policy_ > 0) {
explicit_policy_ -= 1;
+ }
// (2) If policy_mapping is not 0, decrement policy_mapping by 1.
- if (policy_mapping_ > 0)
+ if (policy_mapping_ > 0) {
policy_mapping_ -= 1;
+ }
// (3) If inhibit_anyPolicy is not 0, decrement inhibit_anyPolicy
// by 1.
- if (inhibit_any_policy_ > 0)
+ if (inhibit_any_policy_ > 0) {
inhibit_any_policy_ -= 1;
+ }
}
// RFC 5280 section 6.1.4 step i-j:
@@ -1203,8 +1223,9 @@
CertErrors *errors) {
// From RFC 5280 section 6.1.5:
// (a) If explicit_policy is not 0, decrement explicit_policy by 1.
- if (explicit_policy_ > 0)
+ if (explicit_policy_ > 0) {
explicit_policy_ -= 1;
+ }
// (b) If a policy constraints extension is included in the
// certificate and requireExplicitPolicy is present and has a
@@ -1299,8 +1320,9 @@
// The following enforcements follow from RFC 5937 (primarily section 3.2):
// Initialize name constraints initial-permitted/excluded-subtrees.
- if (cert.has_name_constraints())
+ if (cert.has_name_constraints()) {
name_constraints_list_.push_back(&cert.name_constraints());
+ }
if (cert.has_basic_constraints()) {
// Enforce CA=true if basicConstraints is present. This matches behavior of
@@ -1353,8 +1375,9 @@
case CertificateTrustType::TRUSTED_ANCHOR_OR_LEAF:
break;
}
- if (*shortcircuit_chain_validation)
+ if (*shortcircuit_chain_validation) {
return;
+ }
if (trust.enforce_anchor_expiry) {
VerifyTimeValidity(cert, time, errors);
@@ -1441,8 +1464,9 @@
}
// Check if the key is acceptable by the delegate.
- if (!delegate_->IsPublicKeyAcceptable(pkey.get(), errors))
+ if (!delegate_->IsPublicKeyAcceptable(pkey.get(), errors)) {
errors->AddError(cert_errors::kUnacceptablePublicKey);
+ }
return pkey;
}
diff --git a/pki/verify_name_match.cc b/pki/verify_name_match.cc
index 5952fff..10297b0 100644
--- a/pki/verify_name_match.cc
+++ b/pki/verify_name_match.cc
@@ -72,8 +72,9 @@
// multiple whitespace chars to a single space, otherwise ignore trailing
// whitespace.
std::string::const_iterator next_iter = read_iter + 1;
- if (next_iter != output->end() && *next_iter != ' ')
+ if (next_iter != output->end() && *next_iter != ' ') {
*(write_iter++) = ' ';
+ }
} else if (c >= 'A' && c <= 'Z') {
// Fold case.
*(write_iter++) = c + ('a' - 'A');
@@ -85,12 +86,14 @@
// See NormalizePrintableStringValue comment for the acceptable list
// of characters.
if (!((c >= 'a' && c <= 'z') || (c >= '\'' && c <= ':') || c == '=' ||
- c == '?'))
+ c == '?')) {
return false;
+ }
break;
case ENFORCE_ASCII:
- if (c > 0x7F)
+ if (c > 0x7F) {
return false;
+ }
break;
case NO_ENFORCEMENT:
break;
@@ -98,8 +101,9 @@
*(write_iter++) = c;
}
}
- if (write_iter != output->end())
+ if (write_iter != output->end()) {
output->erase(write_iter, output->end());
+ }
return true;
}
@@ -177,13 +181,15 @@
// TODO(eroman): Plumb this down.
CertErrors unused_errors;
if (!NormalizeValue(a, &a_normalized, &unused_errors) ||
- !NormalizeValue(b, &b_normalized, &unused_errors))
+ !NormalizeValue(b, &b_normalized, &unused_errors)) {
return false;
+ }
return a_normalized == b_normalized;
}
// Attributes encoded with different types may be assumed to be unequal.
- if (a.value_tag != b.value_tag)
+ if (a.value_tag != b.value_tag) {
return false;
+ }
// All other types use binary comparison.
return a.value == b.value;
}
@@ -194,15 +200,17 @@
bool VerifyRdnMatch(der::Parser *a_parser, der::Parser *b_parser) {
RelativeDistinguishedName a_type_and_values, b_type_and_values;
if (!ReadRdn(a_parser, &a_type_and_values) ||
- !ReadRdn(b_parser, &b_type_and_values))
+ !ReadRdn(b_parser, &b_type_and_values)) {
return false;
+ }
// RFC 5280 section 7.1:
// Two relative distinguished names RDN1 and RDN2 match if they have the same
// number of naming attributes and for each naming attribute in RDN1 there is
// a matching naming attribute in RDN2.
- if (a_type_and_values.size() != b_type_and_values.size())
+ if (a_type_and_values.size() != b_type_and_values.size()) {
return false;
+ }
// The ordering of elements may differ due to denormalized values sorting
// differently in the DER encoding. Since the number of elements should be
@@ -216,8 +224,9 @@
break;
}
}
- if (b_iter == b_type_and_values.end())
+ if (b_iter == b_type_and_values.end()) {
return false;
+ }
// Remove the matched element from b_type_and_values to ensure duplicate
// elements in a_type_and_values can't match the same element in
// b_type_and_values multiple times.
@@ -273,10 +282,12 @@
// If doing exact match and either of the sequences has more elements than the
// other, not a match. If doing a subtree match, the first Name may have more
// RDNs than the second.
- if (b_rdn_sequence_counter.HasMore())
+ if (b_rdn_sequence_counter.HasMore()) {
return false;
- if (match_type == EXACT_MATCH && a_rdn_sequence_counter.HasMore())
+ }
+ if (match_type == EXACT_MATCH && a_rdn_sequence_counter.HasMore()) {
return false;
+ }
// Verify that RDNs in |a| and |b| match.
der::Parser a_rdn_sequence(a);
@@ -287,8 +298,9 @@
!b_rdn_sequence.ReadConstructed(der::kSet, &b_rdn)) {
return false;
}
- if (!VerifyRdnMatch(&a_rdn, &b_rdn))
+ if (!VerifyRdnMatch(&a_rdn, &b_rdn)) {
return false;
+ }
}
return true;
@@ -305,21 +317,25 @@
der::Parser rdn_sequence_parser(name_rdn_sequence);
bssl::ScopedCBB cbb;
- if (!CBB_init(cbb.get(), 0))
+ if (!CBB_init(cbb.get(), 0)) {
return false;
+ }
while (rdn_sequence_parser.HasMore()) {
// RelativeDistinguishedName ::= SET SIZE (1..MAX) OF AttributeTypeAndValue
der::Parser rdn_parser;
- if (!rdn_sequence_parser.ReadConstructed(der::kSet, &rdn_parser))
+ if (!rdn_sequence_parser.ReadConstructed(der::kSet, &rdn_parser)) {
return false;
+ }
RelativeDistinguishedName type_and_values;
- if (!ReadRdn(&rdn_parser, &type_and_values))
+ if (!ReadRdn(&rdn_parser, &type_and_values)) {
return false;
+ }
CBB rdn_cbb;
- if (!CBB_add_asn1(cbb.get(), &rdn_cbb, CBS_ASN1_SET))
+ if (!CBB_add_asn1(cbb.get(), &rdn_cbb, CBS_ASN1_SET)) {
return false;
+ }
for (const auto &type_and_value : type_and_values) {
// AttributeTypeAndValue ::= SEQUENCE {
@@ -342,30 +358,35 @@
// AttributeValue ::= ANY -- DEFINED BY AttributeType
if (IsNormalizableDirectoryString(type_and_value.value_tag)) {
std::string normalized_value;
- if (!NormalizeValue(type_and_value, &normalized_value, errors))
+ if (!NormalizeValue(type_and_value, &normalized_value, errors)) {
return false;
+ }
if (!CBB_add_asn1(&attribute_type_and_value_cbb, &value_cbb,
CBS_ASN1_UTF8STRING) ||
!CBB_add_bytes(
&value_cbb,
reinterpret_cast<const uint8_t *>(normalized_value.data()),
- normalized_value.size()))
+ normalized_value.size())) {
return false;
+ }
} else {
if (!CBB_add_asn1(&attribute_type_and_value_cbb, &value_cbb,
type_and_value.value_tag) ||
!CBB_add_bytes(&value_cbb, type_and_value.value.UnsafeData(),
- type_and_value.value.Length()))
+ type_and_value.value.Length())) {
return false;
+ }
}
- if (!CBB_flush(&rdn_cbb))
+ if (!CBB_flush(&rdn_cbb)) {
return false;
+ }
}
// Ensure the encoded AttributeTypeAndValue values in the SET OF are sorted.
- if (!CBB_flush_asn1_set_of(&rdn_cbb) || !CBB_flush(cbb.get()))
+ if (!CBB_flush_asn1_set_of(&rdn_cbb) || !CBB_flush(cbb.get())) {
return false;
+ }
}
normalized_rdn_sequence->assign(CBB_data(cbb.get()),
@@ -392,12 +413,14 @@
der::Parser rdn_sequence_parser(name_rdn_sequence);
while (rdn_sequence_parser.HasMore()) {
der::Parser rdn_parser;
- if (!rdn_sequence_parser.ReadConstructed(der::kSet, &rdn_parser))
+ if (!rdn_sequence_parser.ReadConstructed(der::kSet, &rdn_parser)) {
return false;
+ }
RelativeDistinguishedName type_and_values;
- if (!ReadRdn(&rdn_parser, &type_and_values))
+ if (!ReadRdn(&rdn_parser, &type_and_values)) {
return false;
+ }
for (const auto &type_and_value : type_and_values) {
if (type_and_value.type == der::Input(kTypeEmailAddressOid)) {
diff --git a/pki/verify_name_match_unittest.cc b/pki/verify_name_match_unittest.cc
index 29fc844..53cd012 100644
--- a/pki/verify_name_match_unittest.cc
+++ b/pki/verify_name_match_unittest.cc
@@ -32,8 +32,9 @@
}
bool TypesAreComparable(const std::string &type_1, const std::string &type_2) {
- if (type_1 == type_2)
+ if (type_1 == type_2) {
return true;
+ }
if ((type_1 == "PRINTABLESTRING" || type_1 == "UTF8" ||
type_1 == "BMPSTRING" || type_1 == "UNIVERSALSTRING") &&
(type_2 == "PRINTABLESTRING" || type_2 == "UTF8" ||
diff --git a/pki/verify_signed_data.cc b/pki/verify_signed_data.cc
index f477178..9bf8414 100644
--- a/pki/verify_signed_data.cc
+++ b/pki/verify_signed_data.cc
@@ -214,13 +214,15 @@
break;
}
- if (expected_pkey_id != EVP_PKEY_id(public_key))
+ if (expected_pkey_id != EVP_PKEY_id(public_key)) {
return false;
+ }
// For the supported algorithms the signature value must be a whole
// number of bytes.
- if (signature_value.unused_bits() != 0)
+ if (signature_value.unused_bits() != 0) {
return false;
+ }
const der::Input &signature_value_bytes = signature_value.bytes();
std::string cache_key;
@@ -244,8 +246,9 @@
bssl::ScopedEVP_MD_CTX ctx;
EVP_PKEY_CTX *pctx = nullptr; // Owned by |ctx|.
- if (!EVP_DigestVerifyInit(ctx.get(), &pctx, digest, nullptr, public_key))
+ if (!EVP_DigestVerifyInit(ctx.get(), &pctx, digest, nullptr, public_key)) {
return false;
+ }
if (is_rsa_pss) {
// All supported RSASSA-PSS algorithms match signing and MGF-1 digest. They
@@ -279,8 +282,9 @@
const der::Input &public_key_spki,
SignatureVerifyCache *cache) {
bssl::UniquePtr<EVP_PKEY> public_key;
- if (!ParsePublicKey(public_key_spki, &public_key))
+ if (!ParsePublicKey(public_key_spki, &public_key)) {
return false;
+ }
return VerifySignedData(algorithm, signed_data, signature_value,
public_key.get(), cache);
}
