sockaddr: use more precise exception

securedht: add override

Makefile.am

@@ -16,7 +16,9 @@ if ENABLE_TESTS
 SUBDIRS += tests
 endif
 
+if HAVE_DOXYGEN
 SUBDIRS += doc
+endif
 
 ACLOCAL_AMFLAGS = -I m4
 

configure.ac

@@ -193,7 +193,9 @@ AM_COND_IF(ENABLE_PROXY_SERVER_IDENTITY,
     [CPPFLAGS+=" -DOPENDHT_PROXY_SERVER_IDENTITY=true"],
     [CPPFLAGS+=" -DOPENDHT_PROXY_SERVER_IDENTITY=false"])
 
-AC_CONFIG_FILES([doc/Doxyfile doc/Makefile])
+AM_COND_IF([HAVE_DOXYGEN], [
+  AC_CONFIG_FILES([doc/Doxyfile doc/Makefile])
+])
 
 dnl Configure setup.py if we build the python module
 AM_COND_IF([USE_CYTHON], [

include/opendht/crypto.h

@@ -173,8 +173,6 @@ private:
     PrivateKey(const PrivateKey&) = delete;
     PrivateKey& operator=(const PrivateKey&) = delete;
     Blob decryptBloc(const uint8_t* src, size_t src_size) const;
-
-    //friend dht::crypto::Identity dht::crypto::generateIdentity(const std::string&, dht::crypto::Identity, unsigned key_length);
 };
 
 

include/opendht/dht_proxy_client.h

@@ -375,7 +375,7 @@ private:
     void getPushRequest(Json::Value&) const;
 #endif // OPENDHT_PUSH_NOTIFICATIONS
 
-    bool isDestroying_ {false};
+    std::atomic_bool isDestroying_ {false};
 };
 
 }

include/opendht/securedht.h

@@ -71,7 +71,7 @@ public:
         return secureType(std::move(tmp_type));
     }
 
-    void registerType(const ValueType& type) {
+    void registerType(const ValueType& type) override {
         if (dht_)
             dht_->registerType(secureType(type));
     }
@@ -89,14 +89,14 @@ public:
      * If the signature can't be checked, or if the data can't be decrypted, it is not returned.
      * Public, non-signed & non-encrypted data is retransmitted as-is.
      */
-    void get(const InfoHash& id, GetCallback cb, DoneCallback donecb={}, Value::Filter&& = {}, Where&& w = {});
-    void get(const InfoHash& id, GetCallback cb, DoneCallbackSimple donecb={}, Value::Filter&& f = {}, Where&& w = {}) {
+    void get(const InfoHash& id, GetCallback cb, DoneCallback donecb={}, Value::Filter&& = {}, Where&& w = {}) override;
+    void get(const InfoHash& id, GetCallback cb, DoneCallbackSimple donecb={}, Value::Filter&& f = {}, Where&& w = {}) override {
         get(id, cb, bindDoneCb(donecb), std::forward<Value::Filter>(f), std::forward<Where>(w));
     }
-    void get(const InfoHash& key, GetCallbackSimple cb, DoneCallback donecb={}, Value::Filter&& f={}, Where&& w = {}) {
+    void get(const InfoHash& key, GetCallbackSimple cb, DoneCallback donecb={}, Value::Filter&& f={}, Where&& w = {}) override {
         get(key, bindGetCb(cb), donecb, std::forward<Value::Filter>(f), std::forward<Where>(w));
     }
-    void get(const InfoHash& key, GetCallbackSimple cb, DoneCallbackSimple donecb, Value::Filter&& f={}, Where&& w = {}) {
+    void get(const InfoHash& key, GetCallbackSimple cb, DoneCallbackSimple donecb, Value::Filter&& f={}, Where&& w = {}) override {
         get(key, bindGetCb(cb), bindDoneCb(donecb), std::forward<Value::Filter>(f), std::forward<Where>(w));
     }
 
@@ -148,99 +148,100 @@ public:
     /**
      * SecureDht to Dht proxy
      */
-    void shutdown(ShutdownCallback cb) {
+    void shutdown(ShutdownCallback cb) override {
         dht_->shutdown(cb);
     }
-    void dumpTables() const {
+    void dumpTables() const override {
         dht_->dumpTables();
     }
-    inline const InfoHash& getNodeId() const { return dht_->getNodeId(); }
-    std::pair<size_t, size_t> getStoreSize() const {
+    inline const InfoHash& getNodeId() const override { return dht_->getNodeId(); }
+
+    std::pair<size_t, size_t> getStoreSize() const override {
         return dht_->getStoreSize();
     }
-    std::string getStorageLog() const {
+    std::string getStorageLog() const override {
         return dht_->getStorageLog();
     }
-    std::string getStorageLog(const InfoHash& h) const {
+    std::string getStorageLog(const InfoHash& h) const override {
         return dht_->getStorageLog(h);
     }
-    void setStorageLimit(size_t limit = DEFAULT_STORAGE_LIMIT) {
+    void setStorageLimit(size_t limit = DEFAULT_STORAGE_LIMIT) override {
         dht_->setStorageLimit(limit);
     }
-    std::vector<NodeExport> exportNodes() {
+    std::vector<NodeExport> exportNodes() override {
         return dht_->exportNodes();
     }
-    std::vector<ValuesExport> exportValues() const {
+    std::vector<ValuesExport> exportValues() const override {
         return dht_->exportValues();
     }
-    void importValues(const std::vector<ValuesExport>& v) {
+    void importValues(const std::vector<ValuesExport>& v) override {
         dht_->importValues(v);
     }
-    NodeStats getNodesStats(sa_family_t af) const {
+    NodeStats getNodesStats(sa_family_t af) const override {
         return dht_->getNodesStats(af);
     }
-    std::vector<unsigned> getNodeMessageStats(bool in = false) {
+    std::vector<unsigned> getNodeMessageStats(bool in = false) override {
         return dht_->getNodeMessageStats(in);
     }
-    std::string getRoutingTablesLog(sa_family_t af) const {
+    std::string getRoutingTablesLog(sa_family_t af) const override {
         return dht_->getRoutingTablesLog(af);
     }
-    std::string getSearchesLog(sa_family_t af) const {
+    std::string getSearchesLog(sa_family_t af) const override {
         return dht_->getSearchesLog(af);
     }
-    std::string getSearchLog(const InfoHash& h, sa_family_t af = AF_UNSPEC) const {
+    std::string getSearchLog(const InfoHash& h, sa_family_t af = AF_UNSPEC) const override {
         return dht_->getSearchLog(h, af);
     }
-    std::vector<SockAddr> getPublicAddress(sa_family_t family = 0) {
+    std::vector<SockAddr> getPublicAddress(sa_family_t family = 0) override {
         return dht_->getPublicAddress(family);
     }
-    time_point periodic(const uint8_t *buf, size_t buflen, const SockAddr& sa) {
+    time_point periodic(const uint8_t *buf, size_t buflen, const SockAddr& sa) override {
         return dht_->periodic(buf, buflen, sa);
     }
-    time_point periodic(const uint8_t *buf, size_t buflen, const sockaddr* from, socklen_t fromlen) {
+    time_point periodic(const uint8_t *buf, size_t buflen, const sockaddr* from, socklen_t fromlen) override {
         return dht_->periodic(buf, buflen, from, fromlen);
     }
-    NodeStatus getStatus(sa_family_t af) const {
+    NodeStatus getStatus(sa_family_t af) const override {
         return dht_->getStatus(af);
     }
-    NodeStatus getStatus() const {
+    NodeStatus getStatus() const override {
         return dht_->getStatus();
     }
-    bool isRunning(sa_family_t af = 0) const {
+    bool isRunning(sa_family_t af = 0) const override {
         return dht_->isRunning(af);
     }
-    const ValueType& getType(ValueType::Id type_id) const {
+    const ValueType& getType(ValueType::Id type_id) const override {
         return dht_->getType(type_id);
     }
-    void insertNode(const InfoHash& id, const SockAddr& sa) {
+    void insertNode(const InfoHash& id, const SockAddr& sa) override {
         dht_->insertNode(id, sa);
     }
-    void insertNode(const InfoHash& id, const sockaddr* sa, socklen_t salen) {
+    void insertNode(const InfoHash& id, const sockaddr* sa, socklen_t salen) override {
         dht_->insertNode(id, sa, salen);
     }
-    void insertNode(const NodeExport& n) {
+    void insertNode(const NodeExport& n) override {
         dht_->insertNode(n);
     }
-    void pingNode(const sockaddr* sa, socklen_t salen, DoneCallbackSimple&& cb={}) {
+    void pingNode(const sockaddr* sa, socklen_t salen, DoneCallbackSimple&& cb={}) override {
         dht_->pingNode(sa, salen, std::move(cb));
     }
-    void query(const InfoHash& key, QueryCallback cb, DoneCallback done_cb = {}, Query&& q = {}) {
+    void query(const InfoHash& key, QueryCallback cb, DoneCallback done_cb = {}, Query&& q = {}) override {
         dht_->query(key, cb, done_cb, std::move(q));
     }
-    void query(const InfoHash& key, QueryCallback cb, DoneCallbackSimple done_cb = {}, Query&& q = {}) {
+    void query(const InfoHash& key, QueryCallback cb, DoneCallbackSimple done_cb = {}, Query&& q = {}) override {
         dht_->query(key, cb, done_cb, std::move(q));
     }
-    std::vector<Sp<Value>> getLocal(const InfoHash& key, Value::Filter f = Value::AllFilter()) const {
+    std::vector<Sp<Value>> getLocal(const InfoHash& key, Value::Filter f = Value::AllFilter()) const override {
         return dht_->getLocal(key, f);
     }
-    Sp<Value> getLocalById(const InfoHash& key, Value::Id vid) const {
+    Sp<Value> getLocalById(const InfoHash& key, Value::Id vid) const override {
         return dht_->getLocalById(key, vid);
     }
     void put(const InfoHash& key,
             Sp<Value> v,
             DoneCallback cb=nullptr,
             time_point created=time_point::max(),
-            bool permanent = false)
+            bool permanent = false) override
     {
         dht_->put(key, v, cb, created, permanent);
     }
@@ -248,7 +249,7 @@ public:
             const Sp<Value>& v,
             DoneCallbackSimple cb,
             time_point created=time_point::max(),
-            bool permanent = false)
+            bool permanent = false) override
     {
         dht_->put(key, v, cb, created, permanent);
     }
@@ -257,7 +258,7 @@ public:
             Value&& v,
             DoneCallback cb=nullptr,
             time_point created=time_point::max(),
-            bool permanent = false)
+            bool permanent = false) override
     {
         dht_->put(key, std::move(v), cb, created, permanent);
     }
@@ -265,32 +266,32 @@ public:
             Value&& v,
             DoneCallbackSimple cb,
             time_point created=time_point::max(),
-            bool permanent = false)
+            bool permanent = false) override
     {
         dht_->put(key, std::move(v), cb, created, permanent);
     }
-    std::vector<Sp<Value>> getPut(const InfoHash& h) {
+    std::vector<Sp<Value>> getPut(const InfoHash& h) override {
         return dht_->getPut(h);
     }
-    Sp<Value> getPut(const InfoHash& h, const Value::Id& vid) {
+    Sp<Value> getPut(const InfoHash& h, const Value::Id& vid) override {
         return dht_->getPut(h, vid);
     }
-    bool cancelPut(const InfoHash& h, const Value::Id& vid) {
+    bool cancelPut(const InfoHash& h, const Value::Id& vid) override {
         return dht_->cancelPut(h, vid);
     }
 
-    size_t listen(const InfoHash& key, ValueCallback, Value::Filter={}, Where={});
-    size_t listen(const InfoHash& key, GetCallback cb, Value::Filter = {}, Where w = {});
-    size_t listen(const InfoHash& key, GetCallbackSimple cb, Value::Filter f={}, Where w = {}) {
+    size_t listen(const InfoHash& key, ValueCallback, Value::Filter={}, Where={}) override;
+    size_t listen(const InfoHash& key, GetCallback cb, Value::Filter = {}, Where w = {}) override;
+    size_t listen(const InfoHash& key, GetCallbackSimple cb, Value::Filter f={}, Where w = {}) override {
         return listen(key, bindGetCb(cb), f, w);
     }
-    bool cancelListen(const InfoHash& h, size_t token) {
+    bool cancelListen(const InfoHash& h, size_t token) override {
         return dht_->cancelListen(h, token);
     }
-    void connectivityChanged(sa_family_t af) {
+    void connectivityChanged(sa_family_t af) override {
         dht_->connectivityChanged(af);
     }
-    void connectivityChanged() {
+    void connectivityChanged() override {
         dht_->connectivityChanged();
     }
 
@@ -298,7 +299,7 @@ public:
         forward_all_ = forward;
     }
 
-    void setPushNotificationToken(const std::string& token = "") {
+    void setPushNotificationToken(const std::string& token = "") override {
         dht_->setPushNotificationToken(token);
     }
 
@@ -306,7 +307,7 @@ public:
      * Call linked callback with push_notification
      * @param notification to process
      */
-    void pushNotificationReceived(const std::map<std::string, std::string>& notification) {
+    void pushNotificationReceived(const std::map<std::string, std::string>& notification) override {
         dht_->pushNotificationReceived(notification);
     }
 
@@ -319,7 +320,7 @@ public:
     /**
      * Only print logs related to the given InfoHash (if given), or disable filter (if zeroes).
      */
-    void setLogFilter(const InfoHash& f) {
+    void setLogFilter(const InfoHash& f) override {
         DHT_LOG.setFilter(f);
         dht_->setLogFilter(f);
     }

include/opendht/sockaddr.h

@@ -67,7 +67,7 @@ public:
      */
     SockAddr(const sockaddr* sa, socklen_t length) {
         if (length > sizeof(sockaddr_storage))
-            throw std::runtime_error("Socket address length is too large");
+            throw std::invalid_argument("Socket address length is too large");
         set(sa, length);
     }
     SockAddr(const sockaddr* sa) {
@@ -78,7 +78,7 @@ public:
             else if(sa->sa_family == AF_INET6)
                 len = sizeof(sockaddr_in6);
             else
-                throw std::runtime_error("Unknown address family");
+                throw std::invalid_argument("Unknown address family");
         }
         set(sa, len);
     }

include/opendht/value.h

@@ -577,7 +577,7 @@ struct OPENDHT_PUBLIC Value
     std::string user_type {};
 
     /**
-     * Sequence number to avoid replay attacks
+     * Sequence number
      */
     uint16_t seq {0};
 
@@ -591,8 +591,10 @@ struct OPENDHT_PUBLIC Value
      */
     Blob cypher {};
 
+    bool checkSignature();
+    Sp<Value> decrypt(const crypto::PrivateKey& key);
+
 private:
-    friend class SecureDht;
     /* Cache for crypto ops */
     bool signatureChecked {false};
     bool signatureValid {false};

src/dht_proxy_client.cpp

@@ -578,6 +578,8 @@ DhtProxyClient::getProxyInfos()
 void
 DhtProxyClient::onProxyInfos(const Json::Value& proxyInfos, sa_family_t family)
 {
+    if (isDestroying_)
+        return;
     std::lock_guard<std::mutex> l(lockCurrentProxyInfos_);
     auto oldStatus = std::max(statusIpv4_, statusIpv6_);
     auto& status = family == AF_INET ? statusIpv4_ : statusIpv6_;
@@ -951,6 +953,8 @@ DhtProxyClient::getConnectivityStatus()
 void
 DhtProxyClient::restartListeners()
 {
+    if (isDestroying_) return;
+    std::lock_guard<std::mutex> lock(searchLock_);
     DHT_LOG.d("Refresh permanent puts");
     for (auto& search : searches_) {
         for (auto& put : search.second.puts) {
@@ -974,7 +978,6 @@ DhtProxyClient::restartListeners()
         return;
     }
     DHT_LOG.d("Restarting listeners");
-    std::lock_guard<std::mutex> lock(searchLock_);
     for (auto& search: searches_) {
         for (auto& l: search.second.listeners) {
             auto& listener = l.second;

src/dhtrunner.cpp

@@ -37,7 +37,7 @@
 #include <winsock2.h>
 #include <ws2tcpip.h>
 #define close(x) closesocket(x)
-#define write(...) _write(__VA_ARGS__)
+#define write(s, b, f) send(s, b, (int)strlen(b), 0)
 #endif
 
 namespace dht {

src/securedht.cpp

@@ -68,33 +68,19 @@ ValueType
 SecureDht::secureType(ValueType&& type)
 {
     type.storePolicy = [this,type](InfoHash id, Sp<Value>& v, const InfoHash& nid, const SockAddr& a) {
-        if (v->isSigned()) {
-            if (!v->signatureChecked) {
-                v->signatureChecked = true;
-                v->signatureValid = v->owner and v->owner->checkSignature(v->getToSign(), v->signature);
-            }
-            if (!v->signatureValid) {
-                DHT_LOG.WARN("Signature verification failed");
-                return false;
-            }
-        }
+        if (v->isSigned())
+            return v->checkSignature();
         return type.storePolicy(id, v, nid, a);
     };
     type.editPolicy = [this,type](InfoHash id, const Sp<Value>& o, Sp<Value>& n, const InfoHash& nid, const SockAddr& a) {
-        if (!o->isSigned())
+        if (not o->isSigned())
             return type.editPolicy(id, o, n, nid, a);
-        if (o->owner != n->owner) {
+        if (o->owner != n->owner or not n->isSigned()) {
             DHT_LOG.WARN("Edition forbidden: owner changed.");
             return false;
         }
-        if (!n->signatureChecked) {
-            n->signatureChecked = true;
-            n->signatureValid = o->owner and o->owner->checkSignature(n->getToSign(), n->signature);
-        }
-        if (!n->signatureValid) {
-            DHT_LOG.WARN("Edition forbidden: signature verification failed.");
+        if (not n->checkSignature())
             return false;
-        }
         if (o->seq == n->seq) {
             // If the data is exactly the same,
             // it can be reannounced, possibly by someone else.
@@ -241,35 +227,23 @@ SecureDht::checkValue(const Sp<Value>& v)
 #endif
             return {};
         }
-        if (v->decrypted) {
-            return v->decryptedValue;
-        }
-        v->decrypted = true;
         try {
-            Value decrypted_val (decrypt(*v));
-            if (decrypted_val.recipient == getId()) {
-                if (decrypted_val.owner)
-                    nodesPubKeys_[decrypted_val.owner->getId()] = decrypted_val.owner;
-                v->decryptedValue = std::make_shared<Value>(std::move(decrypted_val));
-                return v->decryptedValue;
+            if (auto decrypted_val = v->decrypt(*key_)) {
+                if (decrypted_val->owner)
+                    nodesPubKeys_[decrypted_val->owner->getId()] = decrypted_val->owner;
+                return decrypted_val;
             }
-            // Ignore values belonging to other people
         } catch (const std::exception& e) {
             DHT_LOG.WARN("Could not decrypt value %s : %s", v->toString().c_str(), e.what());
         }
     }
     // Check signed values
     else if (v->isSigned()) {
-        if (v->signatureChecked) {
-            return v->signatureValid ? v : Sp<Value>{};
-        }
-        v->signatureChecked = true;
-        if (v->owner and v->owner->checkSignature(v->getToSign(), v->signature)) {
-            v->signatureValid = true;
-            nodesPubKeys_[v->owner->getId()] = v->owner;
+        if (v->checkSignature()) {
+            if (v->owner)
+                nodesPubKeys_[v->owner->getId()] = v->owner;
             return v;
-        }
-        else
+        } else
             DHT_LOG.WARN("Signature verification failed for %s", v->toString().c_str());
     }
     // Forward normal values

src/value.cpp

@@ -233,6 +233,39 @@ Value::toJson() const
     return val;
 }
 
+bool
+Value::checkSignature()
+{
+    if (!signatureChecked) {
+        signatureChecked = true;
+        if (isSigned()) {
+            signatureValid = owner and owner->checkSignature(getToSign(), signature);
+        }
+    }
+    return signatureValid;
+}
+
+Sp<Value>
+Value::decrypt(const crypto::PrivateKey& key)
+{
+    if (not decrypted) {
+        decrypted = true;
+        if (isEncrypted()) {
+            auto decryptedBlob = key.decrypt(cypher);
+            std::unique_ptr<Value> v {new Value(id)};
+            auto msg = msgpack::unpack((const char*)decryptedBlob.data(), decryptedBlob.size());
+            v->msgpack_unpack_body(msg.get());
+            if (v->recipient != key.getPublicKey().getId())
+                throw crypto::DecryptError("Recipient mismatch");
+            // Ignore values belonging to other people
+            if (not v->owner or not v->owner->checkSignature(v->getToSign(), v->signature))
+                throw crypto::DecryptError("Signature mismatch");
+            decryptedValue = std::move(v);
+        }
+    }
+    return decryptedValue;
+}
+
 uint64_t
 unpackId(const Json::Value& json, const std::string& key) {
     uint64_t ret = 0;