From 197aaa47f01249c6a7e16e91eb285930abce1630 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Adrien=20B=C3=A9raud?= <adrien.beraud@savoirfairelinux.com>
Date: Tue, 6 Dec 2016 16:59:54 -0500
Subject: [PATCH] crypto: prevent duplicate CRLs, reimport after signing

---
 include/opendht/crypto.h    | 129 +++++++++++++++++++-----------------
 include/opendht/securedht.h |   2 +-
 src/crypto.cpp              |  29 +++++++-
 3 files changed, 97 insertions(+), 63 deletions(-)

diff --git a/include/opendht/crypto.h b/include/opendht/crypto.h
index 6566e9e5..efee4c0c 100644
--- a/include/opendht/crypto.h
+++ b/include/opendht/crypto.h
@@ -161,6 +161,70 @@ private:
     //friend dht::crypto::Identity dht::crypto::generateIdentity(const std::string&, dht::crypto::Identity, unsigned key_length);
 };
 
+
+class OPENDHT_PUBLIC RevocationList
+{
+    using clock = std::chrono::system_clock;
+    using time_point = clock::time_point;
+    using duration = clock::duration;
+public:
+    RevocationList();
+    RevocationList(const Blob& b);
+    RevocationList(RevocationList&& o) : crl(o.crl) { o.crl = nullptr; }
+    ~RevocationList();
+
+    RevocationList& operator=(RevocationList&& o) { crl = o.crl; o.crl = nullptr; return *this; }
+
+    void pack(Blob& b) const;
+    void unpack(const uint8_t* dat, size_t dat_size);
+    Blob getPacked() const {
+        Blob b;
+        pack(b);
+        return b;
+    }
+
+    template <typename Packer>
+    void msgpack_pack(Packer& p) const
+    {
+        Blob b = getPacked();
+        p.pack_bin(b.size());
+        p.pack_bin_body((const char*)b.data(), b.size());
+    }
+
+    void msgpack_unpack(msgpack::object o);
+
+    void revoke(const Certificate& crt, time_point t = time_point::min());
+
+    bool isRevoked(const Certificate& crt) const;
+
+    /**
+     * Sign this revocation list using provided key and certificate.
+     * Validity_period sets the duration until next update (default to no next update).
+     */
+    void sign(const PrivateKey&, const Certificate&, duration validity_period = {});
+    void sign(const Identity& id) { sign(*id.first, *id.second); }
+
+    bool isSignedBy(const Certificate& issuer) const;
+
+    std::string toString() const;
+
+    /**
+     * Read the CRL number extension field.
+     */
+    Blob getNumber() const;
+
+    time_point getUpdateTime() const;
+    time_point getNextUpdateTime() const;
+
+    gnutls_x509_crl_t get() { return crl; }
+
+private:
+    gnutls_x509_crl_t crl {};
+    RevocationList(const RevocationList&) = delete;
+    RevocationList& operator=(const RevocationList&) = delete;
+};
+
+
 struct OPENDHT_PUBLIC Certificate {
     Certificate() {}
 
@@ -325,67 +389,14 @@ struct OPENDHT_PUBLIC Certificate {
 private:
     Certificate(const Certificate&) = delete;
     Certificate& operator=(const Certificate&) = delete;
-    std::set<std::shared_ptr<RevocationList>> revocation_lists;
-};
-
-
-class OPENDHT_PUBLIC RevocationList
-{
-    using clock = std::chrono::system_clock;
-    using time_point = clock::time_point;
-    using duration = clock::duration;
-public:
-    RevocationList();
-    RevocationList(const Blob& b);
-    RevocationList(RevocationList&& o) : crl(o.crl) { o.crl = nullptr; }
-    ~RevocationList();
-
-    RevocationList& operator=(RevocationList&& o) { crl = o.crl; o.crl = nullptr; return *this; }
 
-    void pack(Blob& b) const;
-    void unpack(const uint8_t* dat, size_t dat_size);
-    Blob getPacked() const {
-        Blob b;
-        pack(b);
-        return b;
-    }
-
-    template <typename Packer>
-    void msgpack_pack(Packer& p) const
-    {
-        Blob b = getPacked();
-        p.pack_bin(b.size());
-        p.pack_bin_body((const char*)b.data(), b.size());
-    }
-
-    void msgpack_unpack(msgpack::object o);
-
-    void revoke(const Certificate& crt, time_point t = time_point::min());
-
-    bool isRevoked(const Certificate& crt) const;
-
-    /**
-     * Sign this revocation list using provided key and certificate.
-     * Validity_period sets the duration until expiration (default to certificate expiration).
-     */
-    void sign(const PrivateKey&, const Certificate&, duration validity_period = {});
-    void sign(const Identity& id) { sign(*id.first, *id.second); }
-
-    bool isSignedBy(const Certificate& issuer) const;
-
-    std::string toString() const;
-
-    /**
-     * Read the CRL number extension field.
-     */
-    Blob getNumber() const;
-
-    gnutls_x509_crl_t get() { return crl; }
+    struct crlNumberCmp {
+        bool operator() (const std::shared_ptr<RevocationList>& lhs, const std::shared_ptr<RevocationList>& rhs) const {
+            return lhs->getNumber() < rhs->getNumber();
+        }
+    };
 
-private:
-    gnutls_x509_crl_t crl {};
-    RevocationList(const RevocationList&) = delete;
-    RevocationList& operator=(const RevocationList&) = delete;
+    std::set<std::shared_ptr<RevocationList>, crlNumberCmp> revocation_lists;
 };
 
 
diff --git a/include/opendht/securedht.h b/include/opendht/securedht.h
index a03b6f6f..63bfcb93 100644
--- a/include/opendht/securedht.h
+++ b/include/opendht/securedht.h
@@ -165,7 +165,7 @@ private:
 
 const ValueType CERTIFICATE_TYPE = {
     8, "Certificate", std::chrono::hours(24 * 7),
-    // A certificate can only be stored at it's public key ID.
+    // A certificate can only be stored at its public key ID.
     [](InfoHash id, std::shared_ptr<Value>& v, InfoHash, const sockaddr*, socklen_t) {
         try {
             crypto::Certificate crt(v->data);
diff --git a/src/crypto.cpp b/src/crypto.cpp
index 5a906fae..c320136c 100644
--- a/src/crypto.cpp
+++ b/src/crypto.cpp
@@ -962,6 +962,24 @@ RevocationList::revoke(const Certificate& crt, std::chrono::system_clock::time_p
         throw CryptoException(std::string("Can't revoke certificate: ") + gnutls_strerror(err));
 }
 
+RevocationList::time_point
+RevocationList::getNextUpdateTime() const
+{
+    auto t = gnutls_x509_crl_get_next_update(crl);
+    if (t == (time_t)-1)
+        return std::chrono::system_clock::time_point::min();
+    return std::chrono::system_clock::from_time_t(t);
+}
+
+RevocationList::time_point
+RevocationList::getUpdateTime() const
+{
+    auto t = gnutls_x509_crl_get_this_update(crl);
+    if (t == (time_t)-1)
+        return std::chrono::system_clock::time_point::min();
+    return std::chrono::system_clock::from_time_t(t);
+}
+
 enum class Endian : uint32_t
 {
     LITTLE = 0,
@@ -1003,14 +1021,19 @@ RevocationList::sign(const PrivateKey& key, const Certificate& ca, duration vali
     unsigned critical {0};
     gnutls_x509_crl_get_number(crl, &number, &number_sz, &critical);
     if (number == 0) {
-        number = (uint64_t)1 | ((uint64_t)1 << (7*8));
+        // initialize to a random number
         number_sz = sizeof(number);
-    }
-    number = endian(endian(number) + 1);
+        random_device rdev;
+        std::generate_n((uint8_t*)&number, sizeof(number), std::bind(rand_byte, std::ref(rdev)));
+    } else
+        number = endian(endian(number) + 1);
     if (auto err = gnutls_x509_crl_set_number(crl, &number, sizeof(number)))
         throw CryptoException(std::string("Can't set CRL update time: ") + gnutls_strerror(err));   
     if (auto err = gnutls_x509_crl_sign2(crl, ca.cert, key.x509_key, GNUTLS_DIG_SHA512, 0))
         throw CryptoException(std::string("Can't sign certificate revocation list: ") + gnutls_strerror(err));
+    // to be able to actually use the CRL we need to serialize/deserialize it
+    auto packed = getPacked();
+    unpack(packed.data(), packed.size());
 }
 
 bool
-- 
GitLab