Commit 26ad3277 authored by Adrien Béraud's avatar Adrien Béraud Committed by Guillaume Roguez

device revocation: refuse loading revoked device

The missing piece for working device revocation.
* cert store: always use certificate chain match to check
  parent certificate.
  We used to allow a certificate if the parent certificate
  was explicitly allowed. This doesn't check for revoked
  device certificate. Instead, always check certificates
  using the standard certificate chain method except if
  the certificate itself is whitelisted.
* ringaccount: check the account own device certificate chain
  This cause a revoked account to fail loading and propose
  the user to generate a new device from the archive
  through the standard account migration procedure.
* ringaccount: check device loaded from disc using
  the common routine to discard devices revoked since the last
  save.

Tuleap: #1457
Change-Id: I03f015e78d1d14f5f2e9f99a6d3dd47a6f2c5bfe
parent f9e1dd5d
......@@ -821,7 +821,15 @@ RingAccount::useIdentity(const dht::crypto::Identity& identity)
auto accountCertificate = identity.second->issuer;
if (not accountCertificate) {
RING_ERR("Device certificate must be issued by the account certificate");
RING_ERR("[Account %s] device certificate must be issued by the account certificate", getAccountID().c_str());
return false;
}
// match certificate chain
tls::TrustStore account_trust;
account_trust.setCertificateStatus(accountCertificate, tls::TrustStore::PermissionStatus::ALLOWED, false);
if (not account_trust.isAllowed(*identity.second)) {
RING_ERR("[Account %s] can't use identity: device certificate chain can't be verified", getAccountID().c_str());
return false;
}
......@@ -871,13 +879,14 @@ RingAccount::useIdentity(const dht::crypto::Identity& identity)
// success, make use of this identity (certificate chain and private key)
identity_ = identity;
accountTrust_ = std::move(account_trust);
ringAccountId_ = id;
ringDeviceId_ = identity.first->getPublicKey().getId().toString();
username_ = RING_URI_PREFIX + id;
announce_ = std::make_shared<dht::Value>(std::move(announce_val));
ethAccount_ = root["eth"].asString();
RING_DBG("[Account %s] device receipt checked successfully", getAccountID().c_str());
RING_DBG("[Account %s] ring:%s device %s receipt checked successfully", getAccountID().c_str(), id.c_str(), ringDeviceId_.c_str());
return true;
}
......@@ -2331,12 +2340,8 @@ RingAccount::foundAccountDevice(const std::shared_ptr<dht::crypto::Certificate>&
if (not crt)
return false;
// Trust store with Ring account main certificate as the only CA
tls::TrustStore account_trust;
account_trust.setCertificateStatus(identity_.second->issuer, tls::TrustStore::PermissionStatus::ALLOWED, false);
// match certificate chain
if (not account_trust.isAllowed(*crt)) {
if (not accountTrust_.isAllowed(*crt)) {
RING_WARN("[Account %s] Found invalid account device: %s", getAccountID().c_str(), crt->getId().toString().c_str());
return false;
}
......@@ -2606,28 +2611,9 @@ RingAccount::saveTreatedMessages() const
saveIdList(cachePath_+DIR_SEPARATOR_STR "treatedMessages", treatedMessages_);
}
void
RingAccount::loadKnownDevicesOld()
{
auto knownDevices = loadIdList<dht::InfoHash>(idPath_+DIR_SEPARATOR_STR "knownDevices");
for (const auto& d : knownDevices) {
RING_DBG("[Account %s]: loaded known account device %s", getAccountID().c_str(), d.toString().c_str());
if (auto crt = tls::CertificateStore::instance().getCertificate(d.toString()))
if (crt->issuer and crt->issuer->getId() == identity_.second->issuer->getId())
knownDevices_.emplace(d, KnownDevice{crt});
else
RING_ERR("Known device certificate not matching identity.");
else
RING_WARN("Can't find known device certificate.");
}
fileutils::remove(idPath_+DIR_SEPARATOR_STR "knownDevices");
}
void
RingAccount::loadKnownDevices()
{
loadKnownDevicesOld();
std::map<dht::InfoHash, std::pair<std::string, uint64_t>> knownDevices;
try {
// read file
......@@ -2644,18 +2630,13 @@ RingAccount::loadKnownDevices()
RING_DBG("[Account %s] loading known account device %s %s", getAccountID().c_str(),
d.second.first.c_str(),
d.first.toString().c_str());
if (auto crt = tls::CertificateStore::instance().getCertificate(d.first.toString()))
if (crt->issuer and crt->issuer->getId() == identity_.second->issuer->getId())
knownDevices_.emplace(d.first,
KnownDevice {
crt,
d.second.first,
clock::from_time_t(d.second.second)
});
else
RING_WARN("[Account %s] known device certificate not matching identity", getAccountID().c_str());
else
RING_WARN("[Account %s] can't find known device certificate", getAccountID().c_str());
if (auto crt = tls::CertificateStore::instance().getCertificate(d.first.toString())) {
if (not foundAccountDevice(crt, d.second.first, clock::from_time_t(d.second.second)))
RING_WARN("[Account %s] can't add device %s", getAccountID().c_str(), d.first.toString().c_str());
}
else {
RING_WARN("[Account %s] can't find certificate for device %s", getAccountID().c_str(), d.first.toString().c_str());
}
}
}
......
......@@ -470,6 +470,9 @@ class RingAccount : public SIPAccountBase {
void saveContacts() const;
void updateContact(const dht::InfoHash&, const Contact&);
// Trust store with Ring account main certificate as the only CA
tls::TrustStore accountTrust_;
// Trust store for to match peer certificates
tls::TrustStore trust_;
std::shared_ptr<dht::Value> announce_;
......@@ -512,7 +515,6 @@ class RingAccount : public SIPAccountBase {
void loadTreatedMessages();
void saveTreatedMessages() const;
void loadKnownDevicesOld();
void loadKnownDevices();
void saveKnownDevices() const;
......
......@@ -538,15 +538,6 @@ TrustStore::isAllowed(const crypto::Certificate& crt)
else if (status == PermissionStatus::BANNED)
return false;
// Match by certificate pinning (Ring account)
if (crt.issuer) {
status = getCertificateStatus(crt.issuer->getId().toString());
if (status == PermissionStatus::ALLOWED)
return true;
else if (status == PermissionStatus::BANNED)
return false;
}
// Match by certificate chain
updateKnownCerts();
return matchTrustStore(getChain(crt), allowed_);
......@@ -566,7 +557,6 @@ TrustStore::getTrustedCertificates() const
bool
TrustStore::matchTrustStore(std::vector<gnutls_x509_crt_t>&& crts, gnutls_x509_trust_list_st* store)
{
RING_ERR("TrustStore::matchTrustStore: %lu", crts.size());
unsigned result = 0;
#if GNUTLS_VERSION_NUMBER > 0x030308
......
......@@ -110,6 +110,23 @@ 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) {
unknownCertStatus_ = std::move(o.unknownCertStatus_);
certStatus_ = std::move(o.certStatus_);
revokedList_ = std::move(o.revokedList_),
allowed_ = std::move(o.allowed_);
o.allowed_ = nullptr;
return *this;
}
enum class PermissionStatus {
UNDEFINED = 0,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment