Skip to content

Commit

Permalink
certstore: make TrustStore thread safe
Browse files Browse the repository at this point in the history
Because multiple threads can access the TrustStore to update/add/rm
certificates, introduce a mutex to protect the maps.

Because a lot of methods only access the maps in read-only, the
mutex is mutable. Moreover, because isAllowed will check the whole
chain, to avoid multiple lock/unlocks, the mutex is a recursive one.

Change-Id: Iec197221e2eefba4a7192f36f1a9a952f2533778
GitLab: #690
  • Loading branch information
Sébastien Blin committed Jan 24, 2022
1 parent 8ca7dd3 commit 90a5ea2
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/security/certstore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -531,6 +531,7 @@ TrustStore::setCertificateStatus(std::shared_ptr<crypto::Certificate> cert,
{
if (cert)
CertificateStore::instance().pinCertificate(cert, local);
std::lock_guard<std::recursive_mutex> lk(mutex_);
updateKnownCerts();
bool dirty {false};
if (status == PermissionStatus::UNDEFINED) {
Expand Down Expand Up @@ -573,6 +574,7 @@ TrustStore::setCertificateStatus(std::shared_ptr<crypto::Certificate> cert,
TrustStore::PermissionStatus
TrustStore::getCertificateStatus(const std::string& cert_id) const
{
std::lock_guard<std::recursive_mutex> lk(mutex_);
auto s = certStatus_.find(cert_id);
if (s == std::end(certStatus_)) {
auto us = unknownCertStatus_.find(cert_id);
Expand All @@ -586,6 +588,7 @@ TrustStore::getCertificateStatus(const std::string& cert_id) const
std::vector<std::string>
TrustStore::getCertificatesByStatus(TrustStore::PermissionStatus status) const
{
std::lock_guard<std::recursive_mutex> lk(mutex_);
std::vector<std::string> ret;
for (const auto& i : certStatus_)
if (i.second.second.allowed == (status == TrustStore::PermissionStatus::ALLOWED))
Expand All @@ -600,9 +603,10 @@ bool
TrustStore::isAllowed(const crypto::Certificate& crt, bool allowPublic)
{
// Match by certificate pinning
std::lock_guard<std::recursive_mutex> lk(mutex_);
bool allowed {allowPublic};
for (auto c = &crt; c; c = c->issuer.get()) {
auto status = getCertificateStatus(c->getId().toString());
auto status = getCertificateStatus(c->getId().toString()); // lock mutex_
if (status == PermissionStatus::ALLOWED)
allowed = true;
else if (status == PermissionStatus::BANNED)
Expand Down
1 change: 1 addition & 0 deletions src/security/certstore.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ class TrustStore
};

// unknown certificates with known status
mutable std::recursive_mutex mutex_;
std::map<std::string, Status> unknownCertStatus_;
std::map<std::string, std::pair<std::shared_ptr<crypto::Certificate>, Status>> certStatus_;
dht::crypto::TrustList allowed_;
Expand Down

0 comments on commit 90a5ea2

Please sign in to comment.