Add functions to convert from Span<const uint8> and std::string_view

These can replace the current der::Input-specific string_view methods. I
converted many of the uses within the library, but we should take a pass
over the library and turn many of the std::strings into
std::vector<uint8_t>.

On MSVC, we have to pass /Zc:__cplusplus for __cplusplus to be set
correctly. For now, I'm going to try just adding the flag, but if pki
gets used in more places before we bump our minimum to C++17, we may
want to start looking at _MSVC_LANG instead.

There are actually a few places outside of pki that could use this, but
we sadly can't use them until we require C++17 across the board.

Bug: 661
Change-Id: I1002d9f2e1e4a2592760e8938560894d42a9b58c
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65908
Reviewed-by: Bob Beck <bbe@google.com>
Reviewed-by: Matt Mueller <mattm@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
chromium-stable
David Benjamin 10 months ago committed by Boringssl LUCI CQ
parent 10605c0d1e
commit 71c589682f
  1. 6
      CMakeLists.txt
  2. 14
      include/openssl/span.h
  3. 4
      pki/cert_error_params.cc
  4. 4
      pki/cert_issuer_source_static.cc
  5. 6
      pki/general_names.cc
  6. 10
      pki/input.cc
  7. 8
      pki/input.h
  8. 2
      pki/ocsp.cc
  9. 2
      pki/parse_certificate.cc
  10. 6
      pki/parse_name.cc
  11. 6
      pki/parse_name_unittest.cc
  12. 16
      pki/parse_values.cc
  13. 10
      pki/path_builder.cc
  14. 11
      pki/trust_store_in_memory.cc
  15. 2
      pki/trust_store_in_memory.h
  16. 6
      pki/trust_store_in_memory_unittest.cc

@ -203,7 +203,11 @@ elseif(MSVC)
string(REPLACE "C" " -wd" MSVC_DISABLED_WARNINGS_STR
${MSVC_DISABLED_WARNINGS_LIST})
set(CMAKE_C_FLAGS "-utf-8 -W4 -WX ${MSVC_DISABLED_WARNINGS_STR}")
set(CMAKE_CXX_FLAGS "-utf-8 -W4 -WX ${MSVC_DISABLED_WARNINGS_STR}")
# Without /Zc:__cplusplus, MSVC does not define the right value for
# __cplusplus. See https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/
# If this becomes too problematic for downstream code, we can look at
# _MSVC_LANG.
set(CMAKE_CXX_FLAGS "-utf-8 -W4 -WX ${MSVC_DISABLED_WARNINGS_STR} -Zc:__cplusplus")
endif()
if(WIN32)

@ -26,6 +26,10 @@ extern "C++" {
#include <algorithm>
#include <type_traits>
#if __cplusplus >= 201703L
#include <string_view>
#endif
BSSL_NAMESPACE_BEGIN
template <typename T>
@ -210,6 +214,16 @@ constexpr Span<const T> MakeConstSpan(T (&array)[size]) {
return array;
}
#if __cplusplus >= 201703L
inline Span<const uint8_t> StringAsBytes(std::string_view s) {
return MakeConstSpan(reinterpret_cast<const uint8_t *>(s.data()), s.size());
}
inline std::string_view BytesAsStringView(bssl::Span<const uint8_t> b) {
return std::string_view(reinterpret_cast<const char *>(b.data()), b.size());
}
#endif
BSSL_NAMESPACE_END
} // extern C++

@ -22,9 +22,9 @@ class CertErrorParams2Der : public CertErrorParams {
CertErrorParams2Der(const char *name1, der::Input der1, const char *name2,
der::Input der2)
: name1_(name1),
der1_(der1.AsString()),
der1_(BytesAsStringView(der1)),
name2_(name2),
der2_(der2.AsString()) {}
der2_(BytesAsStringView(der2)) {}
CertErrorParams2Der(const CertErrorParams2Der &) = delete;
CertErrorParams2Der &operator=(const CertErrorParams2Der &) = delete;

@ -12,7 +12,7 @@ CertIssuerSourceStatic::~CertIssuerSourceStatic() = default;
void CertIssuerSourceStatic::AddCert(
std::shared_ptr<const ParsedCertificate> cert) {
intermediates_.insert(std::make_pair(
cert->normalized_subject().AsStringView(), std::move(cert)));
BytesAsStringView(cert->normalized_subject()), std::move(cert)));
}
void CertIssuerSourceStatic::Clear() { intermediates_.clear(); }
@ -20,7 +20,7 @@ void CertIssuerSourceStatic::Clear() { intermediates_.clear(); }
void CertIssuerSourceStatic::SyncGetIssuersOf(const ParsedCertificate *cert,
ParsedCertificateList *issuers) {
auto range =
intermediates_.equal_range(cert->normalized_issuer().AsStringView());
intermediates_.equal_range(BytesAsStringView(cert->normalized_issuer()));
for (auto it = range.first; it != range.second; ++it) {
issuers->push_back(it->second);
}

@ -114,7 +114,7 @@ std::unique_ptr<GeneralNames> GeneralNames::CreateFromValue(
} else if (tag == der::ContextSpecificPrimitive(1)) {
// rfc822Name [1] IA5String,
name_type = GENERAL_NAME_RFC822_NAME;
const std::string_view s = value.AsStringView();
const std::string_view s = BytesAsStringView(value);
if (!bssl::string_util::IsAscii(s)) {
errors->AddError(kRFC822NameNotAscii);
return false;
@ -123,7 +123,7 @@ std::unique_ptr<GeneralNames> GeneralNames::CreateFromValue(
} else if (tag == der::ContextSpecificPrimitive(2)) {
// dNSName [2] IA5String,
name_type = GENERAL_NAME_DNS_NAME;
const std::string_view s = value.AsStringView();
const std::string_view s = BytesAsStringView(value);
if (!bssl::string_util::IsAscii(s)) {
errors->AddError(kDnsNameNotAscii);
return false;
@ -152,7 +152,7 @@ std::unique_ptr<GeneralNames> GeneralNames::CreateFromValue(
} else if (tag == der::ContextSpecificPrimitive(6)) {
// uniformResourceIdentifier [6] IA5String,
name_type = GENERAL_NAME_UNIFORM_RESOURCE_IDENTIFIER;
const std::string_view s = value.AsStringView();
const std::string_view s = BytesAsStringView(value);
if (!bssl::string_util::IsAscii(s)) {
errors->AddError(kURINotAscii);
return false;

@ -8,15 +8,7 @@
namespace bssl::der {
std::string Input::AsString() const {
return std::string(reinterpret_cast<const char *>(data_.data()),
data_.size());
}
std::string_view Input::AsStringView() const {
return std::string_view(reinterpret_cast<const char *>(data_.data()),
data_.size());
}
std::string Input::AsString() const { return std::string(AsStringView()); }
bool operator==(Input lhs, Input rhs) {
return MakeConstSpan(lhs) == MakeConstSpan(rhs);

@ -44,6 +44,8 @@ class OPENSSL_EXPORT Input {
constexpr explicit Input(const uint8_t *data, size_t len)
: data_(MakeConstSpan(data, len)) {}
// Deprecated: Use StringAsBytes.
//
// Creates an Input from a std::string_view. The constructed Input is only
// valid as long as |data| points to live memory. If constructed from, say, a
// |std::string|, mutating the vector will invalidate the Input.
@ -69,13 +71,17 @@ class OPENSSL_EXPORT Input {
constexpr Input first(size_t len) const { return Input(data_.first(len)); }
constexpr Input last(size_t len) const { return Input(data_.last(len)); }
// Deprecated: use BytesAsStringView and convert to std::string.
//
// Returns a copy of the data represented by this object as a std::string.
std::string AsString() const;
// Deprecated: Use ByteAsString.
//
// Returns a std::string_view pointing to the same data as the Input. The
// resulting string_view must not outlive the data that was used to construct
// this Input.
std::string_view AsStringView() const;
std::string_view AsStringView() const { return BytesAsStringView(data_); }
// Deprecated: This class implicitly converts to bssl::Span<const uint8_t>.
//

@ -696,7 +696,7 @@ std::shared_ptr<const ParsedCertificate> OCSPParseCertificate(
// (3) Has signed the OCSP response using its public key.
for (const auto &responder_cert_tlv : response.certs) {
std::shared_ptr<const ParsedCertificate> cur_responder_certificate =
OCSPParseCertificate(responder_cert_tlv.AsStringView());
OCSPParseCertificate(BytesAsStringView(responder_cert_tlv));
// If failed parsing the certificate, keep looking.
if (!cur_responder_certificate) {

@ -872,7 +872,7 @@ bool ParseAuthorityInfoAccessURIs(
// GeneralName ::= CHOICE {
if (access_location_tag == der::ContextSpecificPrimitive(6)) {
// uniformResourceIdentifier [6] IA5String,
std::string_view uri = access_location_value.AsStringView();
std::string_view uri = BytesAsStringView(access_location_value);
if (!bssl::string_util::IsAscii(uri)) {
return false;
}

@ -38,7 +38,7 @@ bool X509NameAttribute::ValueAsString(std::string *out) const {
case der::kPrintableString:
return der::ParsePrintableString(value, out);
case der::kUtf8String:
*out = value.AsString();
*out = BytesAsStringView(value);
return true;
case der::kUniversalString:
return der::ParseUniversalString(value, out);
@ -53,7 +53,7 @@ bool X509NameAttribute::ValueAsStringWithUnsafeOptions(
PrintableStringHandling printable_string_handling, std::string *out) const {
if (printable_string_handling == PrintableStringHandling::kAsUTF8Hack &&
value_tag == der::kPrintableString) {
*out = value.AsString();
*out = BytesAsStringView(value);
return true;
}
return ValueAsString(out);
@ -65,7 +65,7 @@ bool X509NameAttribute::ValueAsStringUnsafe(std::string *out) const {
case der::kPrintableString:
case der::kTeletexString:
case der::kUtf8String:
*out = value.AsString();
*out = BytesAsStringView(value);
return true;
case der::kUniversalString:
return der::ParseUniversalString(value, out);

@ -187,13 +187,13 @@ TEST(ParseNameTest, ValidName) {
ASSERT_EQ(3u, atv.size());
ASSERT_EQ(1u, atv[0].size());
ASSERT_EQ(der::Input(kTypeCountryNameOid), atv[0][0].type);
ASSERT_EQ("US", atv[0][0].value.AsString());
ASSERT_EQ("US", BytesAsStringView(atv[0][0].value));
ASSERT_EQ(1u, atv[1].size());
ASSERT_EQ(der::Input(kTypeOrganizationNameOid), atv[1][0].type);
ASSERT_EQ("Google Inc.", atv[1][0].value.AsString());
ASSERT_EQ("Google Inc.", BytesAsStringView(atv[1][0].value));
ASSERT_EQ(1u, atv[2].size());
ASSERT_EQ(der::Input(kTypeCommonNameOid), atv[2][0].type);
ASSERT_EQ("Google Test CA", atv[2][0].value.AsString());
ASSERT_EQ("Google Test CA", BytesAsStringView(atv[2][0].value));
}
TEST(ParseNameTest, InvalidNameExtraData) {

@ -363,12 +363,12 @@ bool ParseGeneralizedTime(Input in, GeneralizedTime *value) {
}
bool ParseIA5String(Input in, std::string *out) {
for (char c : in.AsStringView()) {
if (static_cast<uint8_t>(c) > 127) {
for (uint8_t c : in) {
if (c > 127) {
return false;
}
}
*out = in.AsString();
*out = BytesAsStringView(in);
return true;
}
@ -379,23 +379,23 @@ bool ParseVisibleString(Input in, std::string *out) {
// Also ITU-T X.691 says it much more clearly:
// "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) {
for (uint8_t c : in) {
if (c < 32 || c > 126) {
return false;
}
}
*out = in.AsString();
*out = BytesAsStringView(in);
return true;
}
bool ParsePrintableString(Input in, std::string *out) {
for (char c : in.AsStringView()) {
for (uint8_t c : in) {
if (!(OPENSSL_isalpha(c) || c == ' ' || (c >= '\'' && c <= ':') ||
c == '=' || c == '?')) {
return false;
}
}
*out = in.AsString();
*out = BytesAsStringView(in);
return true;
}

@ -295,11 +295,11 @@ void CertIssuersIter::GetNextIssuer(IssuerEntry *out) {
void CertIssuersIter::AddIssuers(ParsedCertificateList new_issuers) {
for (std::shared_ptr<const ParsedCertificate> &issuer : new_issuers) {
if (present_issuers_.find(issuer->der_cert().AsStringView()) !=
if (present_issuers_.find(BytesAsStringView(issuer->der_cert())) !=
present_issuers_.end()) {
continue;
}
present_issuers_.insert(issuer->der_cert().AsStringView());
present_issuers_.insert(BytesAsStringView(issuer->der_cert()));
// Look up the trust for this issuer.
IssuerEntry entry;
@ -423,9 +423,9 @@ class CertIssuerIterPath {
// Note that subject_alt_names_extension().value will be empty if the cert
// had no SubjectAltName extension, so there is no need for a condition on
// has_subject_alt_names().
return Key(cert->normalized_subject().AsStringView(),
cert->subject_alt_names_extension().value.AsStringView(),
cert->tbs().spki_tlv.AsStringView());
return Key(BytesAsStringView(cert->normalized_subject()),
BytesAsStringView(cert->subject_alt_names_extension().value),
BytesAsStringView(cert->tbs().spki_tlv));
}
std::vector<std::unique_ptr<CertIssuersIter>> cur_path_;

@ -47,7 +47,8 @@ void TrustStoreInMemory::AddCertificateWithUnspecifiedTrust(
void TrustStoreInMemory::SyncGetIssuersOf(const ParsedCertificate *cert,
ParsedCertificateList *issuers) {
auto range = entries_.equal_range(cert->normalized_issuer().AsStringView());
auto range =
entries_.equal_range(BytesAsStringView(cert->normalized_issuer()));
for (auto it = range.first; it != range.second; ++it) {
issuers->push_back(it->second.cert);
}
@ -55,7 +56,7 @@ void TrustStoreInMemory::SyncGetIssuersOf(const ParsedCertificate *cert,
CertificateTrust TrustStoreInMemory::GetTrust(const ParsedCertificate *cert) {
// Check SPKI distrust first.
if (distrusted_spkis_.find(cert->tbs().spki_tlv.AsString()) !=
if (distrusted_spkis_.find(BytesAsStringView(cert->tbs().spki_tlv)) !=
distrusted_spkis_.end()) {
return CertificateTrust::ForDistrusted();
}
@ -80,13 +81,13 @@ void TrustStoreInMemory::AddCertificate(
entry.trust = trust;
// TODO(mattm): should this check for duplicate certificates?
entries_.insert(
std::make_pair(entry.cert->normalized_subject().AsStringView(), entry));
entries_.emplace(BytesAsStringView(entry.cert->normalized_subject()), entry);
}
const TrustStoreInMemory::Entry *TrustStoreInMemory::GetEntry(
const ParsedCertificate *cert) const {
auto range = entries_.equal_range(cert->normalized_subject().AsStringView());
auto range =
entries_.equal_range(BytesAsStringView(cert->normalized_subject()));
for (auto it = range.first; it != range.second; ++it) {
if (cert == it->second.cert.get() ||
cert->der_cert() == it->second.cert->der_cert()) {

@ -89,7 +89,7 @@ class OPENSSL_EXPORT TrustStoreInMemory : public TrustStore {
std::unordered_multimap<std::string_view, Entry> entries_;
// Set of distrusted SPKIs.
std::set<std::string> distrusted_spkis_;
std::set<std::string, std::less<>> distrusted_spkis_;
// Returns the `Entry` matching `cert`, or `nullptr` if not in the trust
// store.

@ -77,7 +77,8 @@ TEST_F(TrustStoreInMemoryTest, OneRootTrusted) {
TEST_F(TrustStoreInMemoryTest, DistrustBySPKI) {
TrustStoreInMemory in_memory;
in_memory.AddDistrustedCertificateBySPKI(newroot_->tbs().spki_tlv.AsString());
in_memory.AddDistrustedCertificateBySPKI(
std::string(BytesAsStringView(newroot_->tbs().spki_tlv)));
// newroot_ is distrusted.
CertificateTrust trust = in_memory.GetTrust(newroot_.get());
@ -98,7 +99,8 @@ TEST_F(TrustStoreInMemoryTest, DistrustBySPKI) {
TEST_F(TrustStoreInMemoryTest, DistrustBySPKIOverridesTrust) {
TrustStoreInMemory in_memory;
in_memory.AddTrustAnchor(newroot_);
in_memory.AddDistrustedCertificateBySPKI(newroot_->tbs().spki_tlv.AsString());
in_memory.AddDistrustedCertificateBySPKI(
std::string(BytesAsStringView(newroot_->tbs().spki_tlv)));
// newroot_ is distrusted.
CertificateTrust trust = in_memory.GetTrust(newroot_.get());

Loading…
Cancel
Save