Skip to content

Commit

Permalink
add debug_data for TrustStoreNSS and pipe it through trial verifier code
Browse files Browse the repository at this point in the history
When running the dual verifier trial on linux & chromeos, this will
record in any trial reports the NSS library version as well as the
TrustStoreNSS parameters (whether system trust settings are being
ignored, and whether a slot filter is being used).

Bug: 1340420
Change-Id: I10c665b2e474a93a7588dfccd8a8ffa4b75134c7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4296558
Reviewed-by: Alex Gough <ajgo@chromium.org>
Commit-Queue: Matt Mueller <mattm@chromium.org>
Reviewed-by: Hubert Chao <hchao@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1112413}
  • Loading branch information
matt-mueller authored and Chromium LUCI CQ committed Mar 2, 2023
1 parent 4a35466 commit 19b3ebf
Show file tree
Hide file tree
Showing 10 changed files with 311 additions and 5 deletions.
22 changes: 22 additions & 0 deletions components/security_interstitials/content/cert_logger.proto
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,17 @@ message ChromeRootStoreDebugInfo {
optional int64 chrome_root_store_version = 1;
}

message TrustStoreNSSDebugInfo {
optional bool ignore_system_trust_settings = 1;

enum SlotFilterType {
DONT_FILTER = 0;
DO_NOT_ALLOW_USER_SLOTS = 1;
ALLOW_SPECIFIED_USER_SLOT = 2;
};
optional SlotFilterType slot_filter_type = 2;
}

// Contains the results of verification by the trial verifier. All fields
// have the same meaning as those of the same name in CertLoggerRequest.
message TrialVerificationInfo {
Expand Down Expand Up @@ -332,4 +343,15 @@ message TrialVerificationInfo {

// Debug information pertaining to the Chrome Root Store (if it was used).
optional ChromeRootStoreDebugInfo chrome_root_store_debug_info = 15;

// The NSS library version (if NSS is used).
optional string nss_version = 16;

// Debug information pertaining to the TrustStoreNSS instance used by the
// primary verifier (if NSS is used).
optional TrustStoreNSSDebugInfo primary_nss_debug_info = 17;

// Debug information pertaining to the TrustStoreNSS instance used by the
// trial verifier (if NSS is used).
optional TrustStoreNSSDebugInfo trial_nss_debug_info = 18;
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/time/time.h"
#include "build/build_config.h"
#include "components/network_time/network_time_tracker.h"
#include "crypto/crypto_buildflags.h"
#include "net/cert/cert_verifier.h"
#include "net/cert/x509_certificate.h"
#include "net/ssl/ssl_info.h"
Expand Down Expand Up @@ -185,6 +186,37 @@ void AddWinPlatformDebugInfoToReport(
}
#endif // BUILDFLAG(IS_WIN)

#if BUILDFLAG(USE_NSS_CERTS)
chrome_browser_ssl::TrustStoreNSSDebugInfo::SlotFilterType
SlotFilterTypeFromMojom(
cert_verifier::mojom::TrustStoreNSSDebugInfo::SlotFilterType input) {
switch (input) {
case cert_verifier::mojom::TrustStoreNSSDebugInfo::SlotFilterType::
kDontFilter:
return chrome_browser_ssl::TrustStoreNSSDebugInfo::DONT_FILTER;
case cert_verifier::mojom::TrustStoreNSSDebugInfo::SlotFilterType::
kDoNotAllowUserSlots:
return chrome_browser_ssl::TrustStoreNSSDebugInfo::
DO_NOT_ALLOW_USER_SLOTS;
case cert_verifier::mojom::TrustStoreNSSDebugInfo::SlotFilterType::
kAllowSpecifiedUserSlot:
return chrome_browser_ssl::TrustStoreNSSDebugInfo::
ALLOW_SPECIFIED_USER_SLOT;
}
}
void AddNSSDebugInfoToReport(
const cert_verifier::mojom::TrustStoreNSSDebugInfoPtr& nss_debug_info,
chrome_browser_ssl::TrustStoreNSSDebugInfo* report_info) {
if (!nss_debug_info) {
return;
}
report_info->set_ignore_system_trust_settings(
nss_debug_info->ignore_system_trust_settings);
report_info->set_slot_filter_type(
SlotFilterTypeFromMojom(nss_debug_info->slot_filter_type));
}
#endif // BUILDFLAG(USE_NSS_CERTS)

#if BUILDFLAG(CHROME_ROOT_STORE_SUPPORTED)
void AddChromeRootStoreDebugInfoToReport(
const cert_verifier::mojom::ChromeRootStoreDebugInfoPtr&
Expand Down Expand Up @@ -282,6 +314,13 @@ CertificateErrorReport::CertificateErrorReport(
AddWinPlatformDebugInfoToReport(debug_info->win_platform_debug_info,
trial_report);
#endif
#if BUILDFLAG(USE_NSS_CERTS)
trial_report->set_nss_version(debug_info->nss_version);
AddNSSDebugInfoToReport(debug_info->primary_nss_debug_info,
trial_report->mutable_primary_nss_debug_info());
AddNSSDebugInfoToReport(debug_info->trial_nss_debug_info,
trial_report->mutable_trial_nss_debug_info());
#endif
#if BUILDFLAG(CHROME_ROOT_STORE_SUPPORTED)
AddChromeRootStoreDebugInfoToReport(debug_info->chrome_root_store_debug_info,
trial_report);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,21 @@ TEST(ErrorReportTest, TrialDebugInfo) {
debug_info->win_platform_debug_info->authroot_sequence_number = {
'J', 'E', 'N', 'N', 'Y'};
#endif
#if BUILDFLAG(USE_NSS_CERTS)
debug_info->nss_version = "aoeu";

debug_info->primary_nss_debug_info =
cert_verifier::mojom::TrustStoreNSSDebugInfo::New();
debug_info->primary_nss_debug_info->ignore_system_trust_settings = false;
debug_info->primary_nss_debug_info->slot_filter_type =
cert_verifier::mojom::TrustStoreNSSDebugInfo::SlotFilterType::kDontFilter,

debug_info->trial_nss_debug_info =
cert_verifier::mojom::TrustStoreNSSDebugInfo::New();
debug_info->trial_nss_debug_info->ignore_system_trust_settings = true;
debug_info->trial_nss_debug_info->slot_filter_type = cert_verifier::mojom::
TrustStoreNSSDebugInfo::SlotFilterType::kAllowSpecifiedUserSlot;
#endif
#if BUILDFLAG(CHROME_ROOT_STORE_SUPPORTED)
debug_info->chrome_root_store_debug_info =
cert_verifier::mojom::ChromeRootStoreDebugInfo::New();
Expand Down Expand Up @@ -491,6 +506,28 @@ TEST(ErrorReportTest, TrialDebugInfo) {
EXPECT_FALSE(trial_info.has_win_platform_debug_info());
#endif

#if BUILDFLAG(USE_NSS_CERTS)
ASSERT_TRUE(trial_info.has_nss_version());
EXPECT_EQ("aoeu", trial_info.nss_version());

ASSERT_TRUE(trial_info.has_primary_nss_debug_info());
EXPECT_EQ(false,
trial_info.primary_nss_debug_info().ignore_system_trust_settings());
EXPECT_EQ(chrome_browser_ssl::TrustStoreNSSDebugInfo::DONT_FILTER,
trial_info.primary_nss_debug_info().slot_filter_type());

ASSERT_TRUE(trial_info.has_trial_nss_debug_info());
EXPECT_EQ(true,
trial_info.trial_nss_debug_info().ignore_system_trust_settings());
EXPECT_EQ(
chrome_browser_ssl::TrustStoreNSSDebugInfo::ALLOW_SPECIFIED_USER_SLOT,
trial_info.trial_nss_debug_info().slot_filter_type());
#else
EXPECT_FALSE(trial_info.has_nss_version());
EXPECT_FALSE(trial_info.has_primary_nss_debug_info());
EXPECT_FALSE(trial_info.has_trial_nss_debug_info());
#endif

#if BUILDFLAG(CHROME_ROOT_STORE_SUPPORTED)
ASSERT_TRUE(trial_info.has_chrome_root_store_debug_info());
EXPECT_EQ(
Expand Down
48 changes: 48 additions & 0 deletions net/cert/internal/trust_store_nss.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,21 @@ namespace net {

namespace {

const void* kResultDebugDataKey = &kResultDebugDataKey;

TrustStoreNSS::ResultDebugData::SlotFilterType GetSlotFilterType(
const TrustStoreNSS::UserSlotTrustSetting& user_slot_trust_setting) {
if (absl::holds_alternative<TrustStoreNSS::UseTrustFromAllUserSlots>(
user_slot_trust_setting)) {
return TrustStoreNSS::ResultDebugData::SlotFilterType::kDontFilter;
}
if (absl::get<crypto::ScopedPK11Slot>(user_slot_trust_setting) == nullptr) {
return TrustStoreNSS::ResultDebugData::SlotFilterType::kDoNotAllowUserSlots;
}
return TrustStoreNSS::ResultDebugData::SlotFilterType::
kAllowSpecifiedUserSlot;
}

struct FreePK11GenericObjects {
void operator()(PK11GenericObject* x) const {
if (x) {
Expand Down Expand Up @@ -72,6 +87,34 @@ GetAllSlotsAndHandlesForCert(CERTCertificate* nss_cert) {

} // namespace

TrustStoreNSS::ResultDebugData::ResultDebugData(
bool ignore_system_trust_settings,
SlotFilterType slot_filter_type)
: ignore_system_trust_settings_(ignore_system_trust_settings),
slot_filter_type_(slot_filter_type) {}

// static
const TrustStoreNSS::ResultDebugData* TrustStoreNSS::ResultDebugData::Get(
const base::SupportsUserData* debug_data) {
return static_cast<ResultDebugData*>(
debug_data->GetUserData(kResultDebugDataKey));
}

// static
void TrustStoreNSS::ResultDebugData::Create(
bool ignore_system_trust_settings,
SlotFilterType slot_filter_type,
base::SupportsUserData* debug_data) {
debug_data->SetUserData(kResultDebugDataKey,
std::make_unique<ResultDebugData>(
ignore_system_trust_settings, slot_filter_type));
}

std::unique_ptr<base::SupportsUserData::Data>
TrustStoreNSS::ResultDebugData::Clone() {
return std::make_unique<ResultDebugData>(*this);
}

TrustStoreNSS::TrustStoreNSS(SystemTrustSetting system_trust_setting,
UserSlotTrustSetting user_slot_trust_setting)
: ignore_system_trust_settings_(system_trust_setting == kIgnoreSystemTrust),
Expand Down Expand Up @@ -121,6 +164,11 @@ void TrustStoreNSS::SyncGetIssuersOf(const ParsedCertificate* cert,
CertificateTrust TrustStoreNSS::GetTrust(const ParsedCertificate* cert,
base::SupportsUserData* debug_data) {
crypto::EnsureNSSInit();
if (debug_data) {
ResultDebugData::Create(ignore_system_trust_settings_,
GetSlotFilterType(user_slot_trust_setting_),
debug_data);
}
// In theory we could also do better multi-profile slot filtering using a
// similar approach as GetTrustIgnoringSystemTrust, however it makes the
// logic more complicated and isn't really worth doing since we'll be
Expand Down
30 changes: 30 additions & 0 deletions net/cert/internal/trust_store_nss.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,36 @@ class NET_EXPORT TrustStoreNSS : public TrustStore {
using UserSlotTrustSetting =
absl::variant<UseTrustFromAllUserSlots, crypto::ScopedPK11Slot>;

class ResultDebugData : public base::SupportsUserData::Data {
public:
enum class SlotFilterType {
kDontFilter,
kDoNotAllowUserSlots,
kAllowSpecifiedUserSlot
};

explicit ResultDebugData(bool ignore_system_trust_settings,
SlotFilterType slot_filter_type);

static const ResultDebugData* Get(const base::SupportsUserData* debug_data);
static void Create(bool ignore_system_trust_settings,
SlotFilterType slot_filter_type,
base::SupportsUserData* debug_data);

// base::SupportsUserData::Data implementation:
std::unique_ptr<Data> Clone() override;

bool ignore_system_trust_settings() const {
return ignore_system_trust_settings_;
}

SlotFilterType slot_filter_type() const { return slot_filter_type_; }

private:
const bool ignore_system_trust_settings_;
const SlotFilterType slot_filter_type_;
};

// Creates a TrustStoreNSS which will find anchors that are trusted for
// SSL server auth.
//
Expand Down
29 changes: 24 additions & 5 deletions net/cert/internal/trust_store_nss_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

#include "base/strings/strcat.h"
#include "base/strings/string_number_conversions.h"
#include "base/supports_user_data.h"
#include "base/test/scoped_feature_list.h"
#include "crypto/nss_util_internal.h"
#include "crypto/scoped_test_nss_db.h"
Expand Down Expand Up @@ -451,11 +452,12 @@ class TrustStoreNSSTestBase : public ::testing::Test {

// Specifies which kind of per-slot filtering the TrustStoreNSS is supposed to
// perform in the parametrized TrustStoreNSSTestWithSlotFilterType.
enum class SlotFilterType {
kDontFilter,
kDoNotAllowUserSlots,
kAllowSpecifiedUserSlot
};
// TODO(https://crbug.com/1412591): The SlotFilterType enum is shared with
// TrustStoreNSS::ResultDebugData::SlotFilterType for convenience. Once the old
// code path and trial code is cleaned up, the type definition can be moved
// back to here.
using SlotFilterType = TrustStoreNSS::ResultDebugData::SlotFilterType;

std::string SlotFilterTypeToString(SlotFilterType slot_filter_type) {
switch (slot_filter_type) {
case SlotFilterType::kDontFilter:
Expand Down Expand Up @@ -500,6 +502,23 @@ class TrustStoreNSSTestWithSlotFilterType
}
};

class DebugData : public base::SupportsUserData {
public:
~DebugData() override = default;
};

TEST_P(TrustStoreNSSTestWithSlotFilterType, DebugData) {
DebugData debug_data;
trust_store_nss_->GetTrust(target_.get(), &debug_data);
const TrustStoreNSS::ResultDebugData* trust_debug_data =
TrustStoreNSS::ResultDebugData::Get(&debug_data);
ASSERT_TRUE(trust_debug_data);
EXPECT_EQ(system_trust_setting() ==
TrustStoreNSS::SystemTrustSetting::kIgnoreSystemTrust,
trust_debug_data->ignore_system_trust_settings());
EXPECT_EQ(slot_filter_type(), trust_debug_data->slot_filter_type());
}

// Without adding any certs to the NSS DB, should get no anchor results for
// any of the test certs.
TEST_P(TrustStoreNSSTestWithSlotFilterType, CertsNotPresent) {
Expand Down
5 changes: 5 additions & 0 deletions services/cert_verifier/public/mojom/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

import("//crypto/features.gni")
import("//mojo/public/tools/bindings/mojom.gni")
import("//net/features.gni")

Expand All @@ -22,4 +23,8 @@ mojom("mojom") {
if (chrome_root_store_supported) {
enabled_features += [ "is_chrome_root_store_supported" ]
}

if (use_nss_certs) {
enabled_features += [ "use_nss_certs" ]
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,21 @@ struct WinPlatformVerifierDebugInfo {
array<uint8> authroot_sequence_number;
};

[EnableIf=use_nss_certs]
struct TrustStoreNSSDebugInfo {
// Whether the TrustStoreNSS instance is ignoring the NSS builtin trust
// records or not.
bool ignore_system_trust_settings;

// Which user slots are allowed by the TrustStoreNSS instance.
enum SlotFilterType {
kDontFilter = 0,
kDoNotAllowUserSlots = 1,
kAllowSpecifiedUserSlot = 2,
};
SlotFilterType slot_filter_type;
};

[EnableIf=is_chrome_root_store_supported]
struct ChromeRootStoreDebugInfo {
int64 chrome_root_store_version;
Expand Down Expand Up @@ -83,6 +98,15 @@ struct CertVerifierDebugInfo {
[EnableIf=is_win]
WinPlatformVerifierDebugInfo? win_platform_debug_info;

[EnableIf=use_nss_certs]
string nss_version;

[EnableIf=use_nss_certs]
TrustStoreNSSDebugInfo? primary_nss_debug_info;

[EnableIf=use_nss_certs]
TrustStoreNSSDebugInfo? trial_nss_debug_info;

[EnableIf=is_chrome_root_store_supported]
ChromeRootStoreDebugInfo? chrome_root_store_debug_info;

Expand Down
Loading

0 comments on commit 19b3ebf

Please sign in to comment.