From 65e35da8c2aa124176fc83cb89eeead0100fac18 Mon Sep 17 00:00:00 2001
From: Guillaume Roguez <guillaume.roguez@savoirfairelinux.com>
Date: Tue, 22 Mar 2016 18:12:52 -0400
Subject: [PATCH] security: use anon+certificate authentification

The TLS handshaking using certificate authentification leaks
them: they are exchanged before cryptographic parameters,
so the are in plaintext. This is an issue in TLS protocol itself.

So this patch implements a new method to make a crypted channel
first, then uses it to exchange certificates and permit trusted
authentification based on certificates.

This implementation is backware compatible.
This is implies that old daemon can continue to work with patched one,
but in such case certificates are leaked!

Change-Id: Id5906df37b29bb938abdcdf25b875052527437e8
Tuleap: #494
---
 src/security/tls_session.cpp | 173 ++++++++++++++++++++++++++++-------
 src/security/tls_session.h   |   6 +-
 2 files changed, 144 insertions(+), 35 deletions(-)

diff --git a/src/security/tls_session.cpp b/src/security/tls_session.cpp
index c5818096fc..ce91c6ae08 100644
--- a/src/security/tls_session.cpp
+++ b/src/security/tls_session.cpp
@@ -38,7 +38,8 @@
 
 namespace ring { namespace tls {
 
-static constexpr const char* TLS_PRIORITY_STRING {"SECURE192:-RSA:-VERS-TLS-ALL:+VERS-DTLS-ALL:%SERVER_PRECEDENCE"};
+static constexpr const char* TLS_CERT_PRIORITY_STRING {"SECURE192:-VERS-TLS-ALL:+VERS-DTLS-ALL:-RSA:%SERVER_PRECEDENCE:%SAFE_RENEGOTIATION"};
+static constexpr const char* TLS_FULL_PRIORITY_STRING {"SECURE192:-KX-ALL:+ANON-ECDH:+ANON-DH:+SECURE192:-VERS-TLS-ALL:+VERS-DTLS-ALL:-RSA:%SERVER_PRECEDENCE:%SAFE_RENEGOTIATION"};
 static constexpr int DTLS_MTU {1400}; // limit for networks like ADSL
 static constexpr std::size_t INPUT_MAX_SIZE {1000}; // Maximum packet to store before dropping (pkt size = DTLS_MTU)
 static constexpr ssize_t FLOOD_THRESHOLD {4*1024};
@@ -101,24 +102,65 @@ private:
     T creds_;
 };
 
+class TlsSession::TlsAnonymousClientCredendials
+{
+    using T = gnutls_anon_client_credentials_t;
+public:
+    TlsAnonymousClientCredendials() {
+        int ret = gnutls_anon_allocate_client_credentials(&creds_);
+        if (ret < 0) {
+            RING_ERR("gnutls_anon_allocate_client_credentials() failed with ret=%d", ret);
+            throw std::bad_alloc();
+        }
+    }
+
+    ~TlsAnonymousClientCredendials() {
+        gnutls_anon_free_client_credentials(creds_);
+    }
+
+    operator T() { return creds_; }
+
+private:
+    NON_COPYABLE(TlsAnonymousClientCredendials);
+    T creds_;
+};
+
+class TlsSession::TlsAnonymousServerCredendials
+{
+    using T = gnutls_anon_server_credentials_t;
+public:
+    TlsAnonymousServerCredendials() {
+        int ret = gnutls_anon_allocate_server_credentials(&creds_);
+        if (ret < 0) {
+            RING_ERR("gnutls_anon_allocate_server_credentials() failed with ret=%d", ret);
+            throw std::bad_alloc();
+        }
+    }
+
+    ~TlsAnonymousServerCredendials() {
+        gnutls_anon_free_server_credentials(creds_);
+    }
+
+    operator T() { return creds_; }
+
+private:
+    NON_COPYABLE(TlsAnonymousServerCredendials);
+    T creds_;
+};
+
 TlsSession::TlsSession(std::shared_ptr<IceTransport> ice, int ice_comp_id,
                        const TlsParams& params, const TlsSessionCallbacks& cbs)
     : socket_(new IceSocket(ice, ice_comp_id))
     , isServer_(not ice->isInitiator())
     , params_(params)
     , callbacks_(cbs)
+    , cacred_(nullptr)
+    , sacred_(nullptr)
     , xcred_(nullptr)
     , thread_([this] { return setup(); },
               [this] { process(); },
               [this] { cleanup(); })
 {
-    // Setup TLS algorithms priority list
-    auto ret = gnutls_priority_init(&priority_cache_, TLS_PRIORITY_STRING, nullptr);
-    if (ret != GNUTLS_E_SUCCESS) {
-        RING_ERR("[TLS] priority setup failed: %s", gnutls_strerror(ret));
-        throw std::runtime_error("TlsSession");
-    }
-
     socket_->setOnRecv([this](uint8_t* buf, size_t len) {
             std::lock_guard<std::mutex> lk {ioMutex_};
             if (rxQueue_.size() == INPUT_MAX_SIZE) {
@@ -142,9 +184,6 @@ TlsSession::~TlsSession()
     thread_.join();
 
     socket_->setOnRecv(nullptr);
-
-    if (priority_cache_)
-        gnutls_priority_deinit(priority_cache_);
 }
 
 const char*
@@ -182,6 +221,24 @@ TlsSession::setupServer()
     return TlsSessionState::COOKIE;
 }
 
+void
+TlsSession::initAnonymous()
+{
+    // credentials for handshaking and transmission
+    if (isServer_)
+        sacred_.reset(new TlsAnonymousServerCredendials());
+    else
+        cacred_.reset(new TlsAnonymousClientCredendials());
+
+    // Setup DH-params for anonymous authentification
+    if (isServer_) {
+        if (const auto& dh_params = params_.dh_params.get().get())
+            gnutls_anon_set_server_dh_params(*sacred_, dh_params);
+        else
+            RING_WARN("[TLS] DH params unavailable"); // YOMGUI: need to stop?
+    }
+}
+
 void
 TlsSession::initCredentials()
 {
@@ -238,15 +295,28 @@ TlsSession::commonSessionInit()
 {
     int ret;
 
-    ret = gnutls_priority_set(session_, priority_cache_);
+    // Force anonymous connection, see handleStateHandshake how we handle failures
+    ret = gnutls_priority_set_direct(session_, TLS_FULL_PRIORITY_STRING, nullptr);
+    if (ret != GNUTLS_E_SUCCESS) {
+        RING_ERR("[TLS] TLS priority set failed: %s", gnutls_strerror(ret));
+        return false;
+    }
+
+    // Add anonymous credentials
+    if (isServer_)
+        ret = gnutls_credentials_set(session_, GNUTLS_CRD_ANON, *sacred_);
+    else
+        ret = gnutls_credentials_set(session_, GNUTLS_CRD_ANON, *cacred_);
+
     if (ret != GNUTLS_E_SUCCESS) {
-        RING_ERR("[TLS] session TLS priority set failed: %s", gnutls_strerror(ret));
+        RING_ERR("[TLS] anonymous credential set failed: %s", gnutls_strerror(ret));
         return false;
     }
 
+    // Add certificate credentials
     ret = gnutls_credentials_set(session_, GNUTLS_CRD_CERTIFICATE, *xcred_);
     if (ret != GNUTLS_E_SUCCESS) {
-        RING_ERR("[TLS] session credential set failed: %s", gnutls_strerror(ret));
+        RING_ERR("[TLS] certificate credential set failed: %s", gnutls_strerror(ret));
         return false;
     }
 
@@ -462,9 +532,10 @@ TlsSession::handleStateSetup(UNUSED TlsSessionState state)
     RING_DBG("[TLS] Start %s DTLS session", typeName());
 
     try {
+        initAnonymous();
         initCredentials();
     } catch (const std::exception& e) {
-        RING_ERR("[TLS] credential init failed: %s", e.what());
+        RING_ERR("[TLS] authentifications init failed: %s", e.what());
         return TlsSessionState::SHUTDOWN;
     }
 
@@ -560,30 +631,65 @@ TlsSession::handleStateHandshake(TlsSessionState state)
 
     auto ret = gnutls_handshake(session_);
 
-    if (ret == GNUTLS_E_SUCCESS) {
-        auto desc = gnutls_session_get_desc(session_);
-        RING_WARN("[TLS] Session established: %s", desc);
-        gnutls_free(desc);
-
-        // Aware about certificates updates
-        if (callbacks_.onCertificatesUpdate) {
-            unsigned int remote_count;
-            auto local = gnutls_certificate_get_ours(session_);
-            auto remote = gnutls_certificate_get_peers(session_, &remote_count);
-            callbacks_.onCertificatesUpdate(local, remote, remote_count);
-        }
+    // Stop on fatal error
+    if (gnutls_error_is_fatal(ret)) {
+        RING_ERR("[TLS] handshake failed: %s", gnutls_strerror(ret));
+        return TlsSessionState::SHUTDOWN;
+    }
 
-        return TlsSessionState::ESTABLISHED;
+    // Continue handshaking on non-fatal error
+    if (ret != GNUTLS_E_SUCCESS) {
+        // TODO: handle GNUTLS_E_LARGE_PACKET (MTU must be lowered)
+        RING_DBG("[TLS] non-fatal handshake error: %s", gnutls_strerror(ret));
+        return state;
     }
 
-    if (gnutls_error_is_fatal(ret)) {
-        RING_ERR("[TLS] handshake failed: %s", gnutls_strerror(ret));
-        dump_io_stats();
+    // Safe-Renegotiation status shall always be true to prevent MiM attack
+    if (!gnutls_safe_renegotiation_status(session_)) {
+        RING_ERR("[TLS] server identity changed! MiM attack?");
         return TlsSessionState::SHUTDOWN;
     }
 
-    // TODO: handle GNUTLS_E_LARGE_PACKET (MTU must be lowered)
-    return state;
+    auto desc = gnutls_session_get_desc(session_);
+    RING_WARN("[TLS] session established: %s", desc);
+    gnutls_free(desc);
+
+    // Anonymous connection? rehandshake immediatly with certificate authentification forced
+    auto cred = gnutls_auth_get_type(session_);
+    if (cred == GNUTLS_CRD_ANON) {
+        RING_DBG("[TLS] renogotiate with certificate authentification");
+
+        // Re-setup TLS algorithms priority list with only certificate based cipher suites
+        ret = gnutls_priority_set_direct(session_, TLS_CERT_PRIORITY_STRING, nullptr);
+        if (ret != GNUTLS_E_SUCCESS) {
+            RING_ERR("[TLS] session TLS cert-only priority set failed: %s", gnutls_strerror(ret));
+            return TlsSessionState::SHUTDOWN;
+        }
+
+        // remove anon credentials and re-enable certificate ones
+        gnutls_credentials_clear(session_);
+        ret = gnutls_credentials_set(session_, GNUTLS_CRD_CERTIFICATE, *xcred_);
+        if (ret != GNUTLS_E_SUCCESS) {
+            RING_ERR("[TLS] session credential set failed: %s", gnutls_strerror(ret));
+            return TlsSessionState::SHUTDOWN;
+        }
+
+        return state; // handshake
+
+    } else if (cred != GNUTLS_CRD_CERTIFICATE) {
+        RING_ERR("[TLS] spurious session credential (%u)", cred);
+        return TlsSessionState::SHUTDOWN;
+    }
+
+    // Aware about certificates updates
+    if (callbacks_.onCertificatesUpdate) {
+        unsigned int remote_count;
+        auto local = gnutls_certificate_get_ours(session_);
+        auto remote = gnutls_certificate_get_peers(session_, &remote_count);
+        callbacks_.onCertificatesUpdate(local, remote, remote_count);
+    }
+
+    return TlsSessionState::ESTABLISHED;
 }
 
 TlsSessionState
@@ -678,7 +784,6 @@ TlsSession::process()
         callbacks_.onStateChange(new_state);
 }
 
-
 DhParams
 DhParams::generate()
 {
diff --git a/src/security/tls_session.h b/src/security/tls_session.h
index c51e697225..0a09ba2eb8 100644
--- a/src/security/tls_session.h
+++ b/src/security/tls_session.h
@@ -197,15 +197,19 @@ private:
 
     // GnuTLS backend and connection state
     class TlsCertificateCredendials;
+    class TlsAnonymousClientCredendials;
+    class TlsAnonymousServerCredendials;
+    std::unique_ptr<TlsAnonymousClientCredendials> cacred_; // ctor init.
+    std::unique_ptr<TlsAnonymousServerCredendials> sacred_; // ctor init.
     std::unique_ptr<TlsCertificateCredendials> xcred_; // ctor init.
     gnutls_session_t session_ {nullptr};
     gnutls_datum_t cookie_key_ {nullptr, 0};
-    gnutls_priority_t priority_cache_ {nullptr};
     gnutls_dtls_prestate_st prestate_ {};
     ssize_t cookie_count_ {0};
 
     TlsSessionState setupClient();
     TlsSessionState setupServer();
+    void initAnonymous();
     void initCredentials();
     bool commonSessionInit();
 
-- 
GitLab