From 33447cc6bb3363aa3756c96900edde7c2287da91 Mon Sep 17 00:00:00 2001
From: Adrien Beraud <adrien.beraud@savoirfairelinux.com>
Date: Fri, 26 May 2017 15:16:01 +0200
Subject: [PATCH] certstore: re-factor peer certificate authorization routine
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* cleanup CertStore to remove unused features
* ensure that at least one certificate in the chain is allowed
if public calls are disabled and that no certificate in the chain
is banned, additionally to the chain match.
This prevent allowing any certificate signed by the CA of a contact,
while still supporting authorization through a CA (with
setCertificateStatus).
* use dht::crypto::TrustList when relevant (requires OpenDHT bump)
* rebuild trust list when appropriate as GnuTLS' trust list
doesn't allow "unbanning". This fix issues when banning then allowing
a certificate during the same session
Change-Id: I8ea28f3a673eebf2174e8fab3a413c20630b28ca
Reviewed-by: Nicolas Jäger <nicolas.jager@savoirfairelinux.com>
---
src/client/configurationmanager.cpp | 9 +-
src/ringdht/ringaccount.cpp | 68 +++----
src/ringdht/ringaccount.h | 3 +-
src/security/certstore.cpp | 269 ++++++++--------------------
src/security/certstore.h | 55 +++---
src/security/tlsvalidator.cpp | 5 +-
6 files changed, 125 insertions(+), 284 deletions(-)
diff --git a/src/client/configurationmanager.cpp b/src/client/configurationmanager.cpp
index a26184b704..a80ed05074 100644
--- a/src/client/configurationmanager.cpp
+++ b/src/client/configurationmanager.cpp
@@ -220,13 +220,8 @@ setCertificateStatus(const std::string& accountId, const std::string& certId, co
if (accountId.empty()) {
ring::tls::CertificateStore::instance().setTrustedCertificate(certId, ring::tls::trustStatusFromStr(ststr.c_str()));
} else if (auto acc = ring::Manager::instance().getAccount<ring::RingAccount>(accountId)) {
- try {
- auto status = ring::tls::TrustStore::statusFromStr(ststr.c_str());
- return acc->setCertificateStatus(certId, status);
- } catch (const std::out_of_range&) {
- auto status = ring::tls::trustStatusFromStr(ststr.c_str());
- return acc->setCertificateStatus(certId, status);
- }
+ auto status = ring::tls::TrustStore::statusFromStr(ststr.c_str());
+ return acc->setCertificateStatus(certId, status);
}
} catch (const std::out_of_range&) {}
return false;
diff --git a/src/ringdht/ringaccount.cpp b/src/ringdht/ringaccount.cpp
index 3d902e48bf..ebb2b339fd 100644
--- a/src/ringdht/ringaccount.cpp
+++ b/src/ringdht/ringaccount.cpp
@@ -191,9 +191,9 @@ struct RingAccount::Contact
if (c.confirmed != confirmed) {
confirmed = c.confirmed or confirmed;
}
- return hasSameState(copy);
+ return hasDifferentState(copy);
}
- bool hasSameState(const Contact& other) const {
+ bool hasDifferentState(const Contact& other) const {
return other.isActive() != isActive()
or other.isBanned() != isBanned()
or other.confirmed != confirmed;
@@ -798,8 +798,8 @@ RingAccount::createRingDevice(const dht::crypto::Identity& id)
std::tie(tlsPrivateKeyFile_, tlsCertificateFile_) = saveIdentity(dev_id, idPath_, "ring_device");
tlsPassword_ = {};
identity_ = dev_id;
- accountTrust_ = tls::TrustStore{};
- accountTrust_.setCertificateStatus(id.second, tls::TrustStore::PermissionStatus::ALLOWED, false);
+ accountTrust_ = dht::crypto::TrustList{};
+ accountTrust_.add(*id.second);
ringDeviceId_ = dev_id.first->getPublicKey().getId().toString();
ringDeviceName_ = ip_utils::getHostname();
if (ringDeviceName_.empty())
@@ -865,9 +865,9 @@ RingAccount::useIdentity(const dht::crypto::Identity& identity)
}
// match certificate chain
- tls::TrustStore account_trust;
- account_trust.setCertificateStatus(accountCertificate, tls::TrustStore::PermissionStatus::ALLOWED, false);
- if (not account_trust.isAllowed(*identity.second)) {
+ dht::crypto::TrustList account_trust;
+ account_trust.add(*accountCertificate);
+ if (not account_trust.verify(*identity.second)) {
RING_ERR("[Account %s] can't use identity: device certificate chain can't be verified", getAccountID().c_str());
return false;
}
@@ -2304,6 +2304,9 @@ RingAccount::onTrustRequest(const dht::InfoHash& peer_account, const dht::InfoHa
// Check existing contact
auto contact = contacts_.find(peer_account);
if (contact != contacts_.end()) {
+ // Banned contact: discard request
+ if (contact->second.isBanned())
+ return;
// Send confirmation
if (not confirm)
sendTrustRequestConfirm(peer_account);
@@ -2343,16 +2346,16 @@ RingAccount::onTrustRequest(const dht::InfoHash& peer_account, const dht::InfoHa
void
RingAccount::onPeerMessage(const dht::InfoHash& peer_device, std::function<void(const std::shared_ptr<dht::crypto::Certificate>& crt, const dht::InfoHash& peer_account)> cb)
{
- // quick check in case we already explicilty banned this public key
+ // quick check in case we already explicilty banned this device
auto trustStatus = trust_.getCertificateStatus(peer_device.toString());
if (trustStatus == tls::TrustStore::PermissionStatus::BANNED) {
- RING_WARN("[Account %s] Discarding message from banned peer %s", getAccountID().c_str(), peer_device.toString().c_str());
+ RING_WARN("[Account %s] Discarding message from banned device %s", getAccountID().c_str(), peer_device.toString().c_str());
return;
}
auto shared = std::static_pointer_cast<RingAccount>(shared_from_this());
findCertificate(peer_device,
- [shared, peer_device, trustStatus, cb](const std::shared_ptr<dht::crypto::Certificate>& cert) {
+ [shared, peer_device, cb](const std::shared_ptr<dht::crypto::Certificate>& cert) {
auto& this_ = *shared;
dht::InfoHash peer_account_id;
@@ -2361,19 +2364,8 @@ RingAccount::onPeerMessage(const dht::InfoHash& peer_device, std::function<void(
return;
}
- if (not this_.dhtPublicInCalls_ and trustStatus != tls::TrustStore::PermissionStatus::ALLOWED) {
- if (!cert or cert->getId() != peer_device) {
- RING_WARN("[Account %s] Can't find certificate of %s for incoming message.", this_.getAccountID().c_str(), peer_device.toString().c_str());
- return;
- }
-
- auto& this_ = *shared;
- if (!this_.trust_.isAllowed(*cert)) {
- RING_WARN("[Account %s] Discarding message from untrusted peer %s.", this_.getAccountID().c_str(), peer_device.toString().c_str());
- return;
- }
- } else if (not this_.dhtPublicInCalls_ or trustStatus == tls::TrustStore::PermissionStatus::BANNED) {
- RING_WARN("[Account %s] Discarding message from untrusted or banned peer %s.", this_.getAccountID().c_str(), peer_device.toString().c_str());
+ if (not this_.trust_.isAllowed(*cert, this_.dhtPublicInCalls_)) {
+ RING_WARN("[Account %s] Discarding message from unauthorized peer %s.", this_.getAccountID().c_str(), peer_device.toString().c_str());
return;
}
@@ -2421,7 +2413,7 @@ RingAccount::foundAccountDevice(const std::shared_ptr<dht::crypto::Certificate>&
return false;
// match certificate chain
- if (not accountTrust_.isAllowed(*crt)) {
+ if (not accountTrust_.verify(*crt)) {
RING_WARN("[Account %s] Found invalid account device: %s", getAccountID().c_str(), crt->getId().toString().c_str());
return false;
}
@@ -2465,19 +2457,17 @@ RingAccount::foundPeerDevice(const std::shared_ptr<dht::crypto::Certificate>& cr
return false;
}
- RING_WARN("foundPeerDevice dev:%s CA:%s", crt->getId().toString().c_str(), top_issuer->getId().toString().c_str());
-
// Check peer certificate chain
// Trust store with top issuer as the only CA
- tls::TrustStore peer_trust;
- peer_trust.setCertificateStatus(top_issuer, tls::TrustStore::PermissionStatus::ALLOWED, false);
-
- if (not peer_trust.isAllowed(*crt)) {
+ dht::crypto::TrustList peer_trust;
+ peer_trust.add(*top_issuer);
+ if (not peer_trust.verify(*crt)) {
RING_WARN("[Account %s] Found invalid peer device: %s", getAccountID().c_str(), crt->getId().toString().c_str());
return false;
}
account_id = crt->issuer->getId();
+ RING_WARN("[Account %s] found peer device: %s account:%s CA:%s", getAccountID().c_str(), crt->getId().toString().c_str(), account_id.toString().c_str(), top_issuer->getId().toString().c_str());
return true;
}
@@ -2622,16 +2612,6 @@ RingAccount::setCertificateStatus(const std::string& cert_id, tls::TrustStore::P
return done;
}
-bool
-RingAccount::setCertificateStatus(const std::string& cert_id, tls::TrustStatus status)
-{
- findCertificate(cert_id);
- bool done = trust_.setCertificateStatus(cert_id, status);
- if (done)
- emitSignal<DRing::ConfigurationSignal::CertificateStateChanged>(getAccountID(), cert_id, tls::statusToStr(status));
- return done;
-}
-
std::vector<std::string>
RingAccount::getCertificatesByStatus(tls::TrustStore::PermissionStatus status)
{
@@ -2917,6 +2897,7 @@ RingAccount::getContactHeader(pjsip_transport* t)
void
RingAccount::addContact(const std::string& uri, bool confirmed)
{
+ RING_WARN("[Account %s] addContact: %s", getAccountID().c_str(), uri.c_str());
dht::InfoHash h (uri);
auto c = contacts_.find(h);
if (c == contacts_.end())
@@ -2934,6 +2915,7 @@ RingAccount::addContact(const std::string& uri, bool confirmed)
void
RingAccount::removeContact(const std::string& uri, bool ban)
{
+ RING_WARN("[Account %s] removeContact: %s", getAccountID().c_str(), uri.c_str());
dht::InfoHash h (uri);
auto c = contacts_.find(h);
if (c == contacts_.end())
@@ -2942,8 +2924,10 @@ RingAccount::removeContact(const std::string& uri, bool ban)
return;
c->second.removed = std::time(nullptr);
c->second.banned = ban;
- if (ban)
- trust_.setCertificateStatus(uri, tls::TrustStore::PermissionStatus::BANNED);
+ trust_.setCertificateStatus(uri, ban ? tls::TrustStore::PermissionStatus::BANNED
+ : tls::TrustStore::PermissionStatus::UNDEFINED);
+ if (ban and trustRequests_.erase(h) > 0)
+ saveTrustRequests();
saveContacts();
emitSignal<DRing::ConfigurationSignal::ContactRemoved>(getAccountID(), uri, ban);
syncDevices();
diff --git a/src/ringdht/ringaccount.h b/src/ringdht/ringaccount.h
index f8a3dd9b82..694f4894c6 100644
--- a/src/ringdht/ringaccount.h
+++ b/src/ringdht/ringaccount.h
@@ -279,7 +279,6 @@ class RingAccount : public SIPAccountBase {
}
bool setCertificateStatus(const std::string& cert_id, tls::TrustStore::PermissionStatus status);
- bool setCertificateStatus(const std::string& cert_id, tls::TrustStatus status);
std::vector<std::string> getCertificatesByStatus(tls::TrustStore::PermissionStatus status);
@@ -477,7 +476,7 @@ class RingAccount : public SIPAccountBase {
void updateContact(const dht::InfoHash&, const Contact&);
// Trust store with Ring account main certificate as the only CA
- tls::TrustStore accountTrust_;
+ dht::crypto::TrustList accountTrust_;
// Trust store for to match peer certificates
tls::TrustStore trust_;
diff --git a/src/security/certstore.cpp b/src/security/certstore.cpp
index f4e89ddab9..9985bc7239 100644
--- a/src/security/certstore.cpp
+++ b/src/security/certstore.cpp
@@ -402,101 +402,70 @@ statusToStr(TrustStatus s)
}
}
-TrustStore::TrustStore()
-{
- gnutls_x509_trust_list_init(&allowed_, 0);
-}
-
-TrustStore::~TrustStore()
-{
- gnutls_x509_trust_list_deinit(allowed_, true);
-}
-
-TrustStore&
-TrustStore::operator=(TrustStore&& o)
-{
- unknownCertStatus_ = std::move(o.unknownCertStatus_);
- certStatus_ = std::move(o.certStatus_);
- revokedList_ = std::move(o.revokedList_);
- if (allowed_)
- gnutls_x509_trust_list_deinit(allowed_, true);
- allowed_ = std::move(o.allowed_);
- o.allowed_ = nullptr;
- return *this;
-}
-
bool
TrustStore::addRevocationList(dht::crypto::RevocationList&& crl)
{
- auto packed = crl.getPacked();
- revokedList_.emplace_back(std::forward<dht::crypto::RevocationList>(crl));
- auto crlp = crl.get();
- return gnutls_x509_trust_list_add_crls(allowed_, &crlp, 1, GNUTLS_TL_VERIFY_CRL, 0) > 0;
+ allowed_.add(crl);
+ return true;
}
bool
TrustStore::setCertificateStatus(const std::string& cert_id,
const TrustStore::PermissionStatus status)
{
- updateKnownCerts();
- auto s = certStatus_.find(cert_id);
- if (s == std::end(certStatus_)) {
- if (auto cert = CertificateStore::instance().getCertificate(cert_id)) {
- auto& crt_status = certStatus_[cert->getId().toString()];
- if (not crt_status.first)
- crt_status.first = cert;
- crt_status.second.allowed = (status == PermissionStatus::ALLOWED);
- setStoreCertStatus(*cert, status);
- } else
- unknownCertStatus_[cert_id].allowed = (status == PermissionStatus::ALLOWED);
- } else {
- s->second.second.allowed = (status == PermissionStatus::ALLOWED);
- setStoreCertStatus(*s->second.first, status);
- }
- return true;
+ return setCertificateStatus(nullptr, cert_id, status, false);
}
bool
TrustStore::setCertificateStatus(const std::shared_ptr<crypto::Certificate>& cert,
const TrustStore::PermissionStatus status, bool local)
{
- CertificateStore::instance().pinCertificate(cert, local);
- auto& crt_status = certStatus_[cert->getId().toString()];
- if (not crt_status.first)
- crt_status.first = cert;
- crt_status.second.allowed = (status == PermissionStatus::ALLOWED);
- setStoreCertStatus(*cert, status);
- return true;
+ return setCertificateStatus(cert, cert->getId().toString(), status, local);
}
bool
-TrustStore::setCertificateStatus(const std::string& cert_id, const TrustStatus status)
+TrustStore::setCertificateStatus(std::shared_ptr<crypto::Certificate> cert,
+ const std::string& cert_id,
+ const TrustStore::PermissionStatus status, bool local)
{
+ if (cert)
+ CertificateStore::instance().pinCertificate(cert, local);
updateKnownCerts();
- auto s = certStatus_.find(cert_id);
- if (s == std::end(certStatus_)) {
- if (auto cert = CertificateStore::instance().getCertificate(cert_id)) {
- auto& crt_status = certStatus_[cert->getId().toString()];
- if (not crt_status.first)
- crt_status.first = cert;
- crt_status.second.trusted = (status == TrustStatus::TRUSTED);
- } else
- unknownCertStatus_[cert_id].trusted = (status == TrustStatus::TRUSTED);
+ bool dirty {false};
+ if (status == PermissionStatus::UNDEFINED) {
+ unknownCertStatus_.erase(cert_id);
+ dirty = certStatus_.erase(cert_id);
} else {
- s->second.second.trusted = (status == TrustStatus::TRUSTED);
+ bool allowed = (status == PermissionStatus::ALLOWED);
+ auto s = certStatus_.find(cert_id);
+ if (s == std::end(certStatus_)) {
+ // Certificate state is currently undefined
+ if (not cert)
+ cert = CertificateStore::instance().getCertificate(cert_id);
+ if (cert) {
+ unknownCertStatus_.erase(cert_id);
+ auto& crt_status = certStatus_[cert_id];
+ if (not crt_status.first)
+ crt_status.first = cert;
+ crt_status.second.allowed = allowed;
+ setStoreCertStatus(*cert, allowed);
+ } else {
+ // Can't find certificate
+ unknownCertStatus_[cert_id].allowed = allowed;
+ }
+ } else {
+ // Certificate is already allowed or banned
+ if (s->second.second.allowed != allowed) {
+ s->second.second.allowed = allowed;
+ if (allowed) // Certificate is re-added after ban, rebuld needed
+ dirty = true;
+ else
+ allowed_.remove(*s->second.first);
+ }
+ }
}
- return true;
-}
-
-bool
-TrustStore::setCertificateStatus(const std::shared_ptr<crypto::Certificate>& cert,
- const TrustStatus status, bool local)
-{
- CertificateStore::instance().pinCertificate(cert, local);
- auto& crt_status = certStatus_[cert->getId().toString()];
- if (not crt_status.first)
- crt_status.first = cert;
- crt_status.second.trusted = (status == TrustStatus::TRUSTED);
+ if (dirty)
+ rebuildTrust();
return true;
}
@@ -513,19 +482,6 @@ TrustStore::getCertificateStatus(const std::string& cert_id) const
return s->second.second.allowed ? PermissionStatus::ALLOWED : PermissionStatus::BANNED;
}
-TrustStatus
-TrustStore::getCertificateTrustStatus(const std::string& cert_id) const
-{
- auto s = certStatus_.find(cert_id);
- if (s == std::end(certStatus_)) {
- auto us = unknownCertStatus_.find(cert_id);
- if (us == std::end(unknownCertStatus_))
- return TrustStatus::UNTRUSTED;
- return us->second.trusted ? TrustStatus::TRUSTED : TrustStatus::UNTRUSTED;
- }
- return s->second.second.trusted ? TrustStatus::TRUSTED : TrustStatus::UNTRUSTED;
-}
-
std::vector<std::string>
TrustStore::getCertificatesByStatus(TrustStore::PermissionStatus status)
{
@@ -540,104 +496,27 @@ TrustStore::getCertificatesByStatus(TrustStore::PermissionStatus status)
}
bool
-TrustStore::isAllowed(const crypto::Certificate& crt)
-{
- // Match by certificate pinning (device)
- auto status = getCertificateStatus(crt.getId().toString());
- if (status == PermissionStatus::ALLOWED)
- return true;
- else if (status == PermissionStatus::BANNED)
- return false;
+TrustStore::isAllowed(const crypto::Certificate& crt, bool allowPublic)
+{
+ // Match by certificate pinning
+ bool allowed {allowPublic};
+ for (auto c = &crt; c; c = c->issuer.get()) {
+ auto status = getCertificateStatus(c->getId().toString());
+ if (status == PermissionStatus::ALLOWED)
+ allowed = true;
+ else if (status == PermissionStatus::BANNED)
+ return false;
+ }
// Match by certificate chain
updateKnownCerts();
- return matchTrustStore(getChain(crt), allowed_);
-}
-
-
-std::vector<gnutls_x509_crt_t>
-TrustStore::getTrustedCertificates() const
-{
- auto cas = CertificateStore::instance().getTrustedCertificates();
- for (const auto& i : certStatus_)
- if (i.second.second.trusted)
- cas.emplace_back(i.second.first->cert);
- return cas;
-}
-
-bool
-TrustStore::matchTrustStore(std::vector<gnutls_x509_crt_t>&& crts, gnutls_x509_trust_list_st* store)
-{
- unsigned result = 0;
-
-#if GNUTLS_VERSION_NUMBER > 0x030308
- auto ret = gnutls_x509_trust_list_verify_crt2(
- store,
- crts.data(), crts.size(),
- nullptr, 0,
- GNUTLS_PROFILE_TO_VFLAGS(GNUTLS_PROFILE_MEDIUM),
- &result, nullptr);
-#else
- auto ret = gnutls_x509_trust_list_verify_crt(
- store,
- crts.data(), crts.size(),
- 0,
- &result, nullptr);
-#endif
-
- if (ret < 0) {
- RING_ERR("Error verifying certificate: %s", gnutls_strerror(ret));
+ auto ret = allowed_.verify(crt);
+ if (not ret) {
+ RING_WARN("%s", ret.toString().c_str());
return false;
- } else if (result & GNUTLS_CERT_INVALID) {
- RING_WARN("Certificate check failed with code: %d", result);
- if (result & GNUTLS_CERT_SIGNATURE_FAILURE)
- RING_WARN("* The signature verification failed.");
- if (result & GNUTLS_CERT_REVOKED)
- RING_WARN("* Certificate is revoked");
- if (result & GNUTLS_CERT_SIGNER_NOT_FOUND)
- RING_WARN("* Certificate's issuer is not known");
- if (result & GNUTLS_CERT_SIGNER_NOT_CA)
- RING_WARN("* Certificate's issuer not a CA");
- if (result & GNUTLS_CERT_SIGNER_CONSTRAINTS_FAILURE)
- RING_WARN("* Certificate's signer constraints were violated");
- if (result & GNUTLS_CERT_INSECURE_ALGORITHM)
- RING_WARN("* Certificate was signed using an insecure algorithm");
- if (result & GNUTLS_CERT_NOT_ACTIVATED)
- RING_WARN("* Certificate is not yet activated");
- if (result & GNUTLS_CERT_EXPIRED)
- RING_WARN("* Certificate has expired");
- if (result & GNUTLS_CERT_UNEXPECTED_OWNER)
- RING_WARN("* The owner is not the expected one");
- if (result & GNUTLS_CERT_PURPOSE_MISMATCH)
- RING_WARN("* Certificate or an intermediate does not match the intended purpose");
- if (result & GNUTLS_CERT_MISMATCH)
- RING_WARN("* Certificate presented isn't the expected one");
}
- return !(result & GNUTLS_CERT_INVALID);
-}
-
-std::vector<gnutls_x509_crt_t>
-getChain(const crypto::Certificate& crt, bool copy)
-{
- std::vector<gnutls_x509_crt_t> crts;
- auto c = &crt;
- do {
- crts.emplace_back(copy ? c->getCopy() : c->cert);
- c = c->issuer.get();
- } while (c);
- return crts;
-}
-
-std::vector<gnutls_x509_crl_t>
-getRevocationList(const crypto::Certificate& crt)
-{
- std::vector<gnutls_x509_crl_t> crls_ret;
- const auto& crls = crt.getRevocationLists();
- crls_ret.reserve(crls.size());
- for (const auto& crl : crls)
- crls_ret.emplace_back(crl->getCopy());
- return crls_ret;
+ return allowed;
}
void
@@ -647,7 +526,7 @@ TrustStore::updateKnownCerts()
while (i != std::end(unknownCertStatus_)) {
if (auto crt = CertificateStore::instance().getCertificate(i->first)) {
certStatus_.emplace(i->first, std::make_pair(crt, i->second));
- setStoreCertStatus(*crt, i->second.allowed ? PermissionStatus::ALLOWED : PermissionStatus::UNDEFINED);
+ setStoreCertStatus(*crt, i->second.allowed);
i = unknownCertStatus_.erase(i);
} else
++i;
@@ -655,28 +534,20 @@ TrustStore::updateKnownCerts()
}
void
-TrustStore::setStoreCertStatus(const crypto::Certificate& crt, TrustStore::PermissionStatus status)
-{
- if (not crt.isCA())
- return;
-
- if (status == PermissionStatus::ALLOWED) {
- auto crt_copy = getChain(crt, true);
- gnutls_x509_trust_list_add_cas(allowed_, crt_copy.data(), crt_copy.size(), GNUTLS_TL_NO_DUPLICATES);
- auto crls = getRevocationList(crt);
- if (not crls.empty())
- if (gnutls_x509_trust_list_add_crls(
- allowed_,
- crls.data(), crls.size(),
- GNUTLS_TL_VERIFY_CRL | GNUTLS_TL_NO_DUPLICATES, 0) == 0)
- RING_WARN("No CRLs where added");
- }
+TrustStore::setStoreCertStatus(const crypto::Certificate& crt, bool status)
+{
+ if (status)
+ allowed_.add(crt);
else
- gnutls_x509_trust_list_remove_cas(allowed_, &crt.cert, 1);
+ allowed_.remove(crt);
+}
- RING_DBG("TrustStore: setting %s status to %s.",
- crt.getId().toString().c_str(),
- status == PermissionStatus::ALLOWED ? "ALLOWED" : "NOT ALLOWED");
+void
+TrustStore::rebuildTrust()
+{
+ allowed_ = {};
+ for (const auto& c : certStatus_)
+ setStoreCertStatus(*c.second.first, c.second.second.allowed);
}
}} // namespace ring::tls
diff --git a/src/security/certstore.h b/src/security/certstore.h
index 9e8183473a..b3aa775711 100644
--- a/src/security/certstore.h
+++ b/src/security/certstore.h
@@ -32,8 +32,6 @@
#include <future>
#include <mutex>
-struct gnutls_x509_trust_list_st;
-
namespace ring { namespace tls {
namespace crypto = dht::crypto;
@@ -108,18 +106,9 @@ private:
*/
class TrustStore {
public:
- TrustStore();
- virtual ~TrustStore();
- TrustStore(TrustStore&& o) :
- unknownCertStatus_(std::move(o.unknownCertStatus_)),
- certStatus_(std::move(o.certStatus_)),
- revokedList_(std::move(o.revokedList_)),
- allowed_(std::move(o.allowed_))
- {
- o.allowed_ = nullptr;
- }
-
- TrustStore& operator=(TrustStore&& o);
+ TrustStore() = default;
+ TrustStore(TrustStore&& o) = default;
+ TrustStore& operator=(TrustStore&& o) = default;
enum class PermissionStatus {
UNDEFINED = 0,
@@ -135,39 +124,43 @@ public:
bool setCertificateStatus(const std::string& cert_id, const PermissionStatus status);
bool setCertificateStatus(const std::shared_ptr<crypto::Certificate>& cert, PermissionStatus status, bool local = true);
- bool setCertificateStatus(const std::string& cert_id, const TrustStatus status);
- bool setCertificateStatus(const std::shared_ptr<crypto::Certificate>& cert, TrustStatus status, bool local = true);
-
PermissionStatus getCertificateStatus(const std::string& cert_id) const;
- TrustStatus getCertificateTrustStatus(const std::string& cert_id) const;
std::vector<std::string> getCertificatesByStatus(PermissionStatus status);
- bool isAllowed(const crypto::Certificate& crt);
-
- std::vector<gnutls_x509_crt_t> getTrustedCertificates() const;
+ /**
+ * Check that the certificate is allowed (valid and permited) for contact.
+ * Valid means the certificate chain matches with our CA list,
+ * has valid signatures, expiration dates etc.
+ * Permited means at least one of the certificate in the chain is
+ * ALLOWED (if allowPublic is false), and none is BANNED.
+ *
+ * @param crt the end certificate of the chain to check
+ * @param allowPublic if false, requires at least one ALLOWED certificate.
+ * (not required otherwise). In any case a BANNED
+ * certificate means permission refusal.
+ * @return true if the certificate is valid and permitted.
+ */
+ bool isAllowed(const crypto::Certificate& crt, bool allowPublic = false);
private:
NON_COPYABLE(TrustStore);
void updateKnownCerts();
- void setStoreCertStatus(const crypto::Certificate& crt, PermissionStatus status);
-
- static bool matchTrustStore(std::vector<gnutls_x509_crt_t>&& crts, gnutls_x509_trust_list_st* store);
+ bool setCertificateStatus(std::shared_ptr<crypto::Certificate> cert,
+ const std::string& cert_id,
+ const TrustStore::PermissionStatus status, bool local);
+ void setStoreCertStatus(const crypto::Certificate& crt, bool status);
+ void rebuildTrust();
struct Status {
- bool allowed : 1;
- bool trusted : 1;
+ bool allowed;
};
// unknown certificates with known status
std::map<std::string, Status> unknownCertStatus_;
std::map<std::string, std::pair<std::shared_ptr<crypto::Certificate>, Status>> certStatus_;
- std::vector<dht::crypto::RevocationList> revokedList_;
- gnutls_x509_trust_list_st* allowed_;
+ dht::crypto::TrustList allowed_;
};
-std::vector<gnutls_x509_crt_t> getChain(const crypto::Certificate& crt, bool copy = false);
-std::vector<gnutls_x509_crl_t> getRevocationList(const crypto::Certificate& crt);
-
}} // namespace ring::tls
diff --git a/src/security/tlsvalidator.cpp b/src/security/tlsvalidator.cpp
index 9af465942d..ccccb9a31b 100644
--- a/src/security/tlsvalidator.cpp
+++ b/src/security/tlsvalidator.cpp
@@ -447,9 +447,6 @@ unsigned int TlsValidator::compareToCa()
if (caChecked_)
return caValidationOutput_;
- // build the certificate chain
- auto crts = getChain(*x509crt_);
-
// build the CA trusted list
gnutls_x509_trust_list_t trust;
gnutls_x509_trust_list_init(&trust, 0);
@@ -466,6 +463,8 @@ unsigned int TlsValidator::compareToCa()
gnutls_x509_trust_list_add_trust_file(trust, caListPath_.c_str(), nullptr, GNUTLS_X509_FMT_PEM, 0, 0);
}
+ // build the certificate chain
+ auto crts = x509crt_->getChain();
err = gnutls_x509_trust_list_verify_crt2(
trust,
crts.data(), crts.size(),
--
GitLab