Support forbidden certificate algorithms list in IC (#7974)

alexvru · web-flow · commit fc74b88f22b0 · 2024-08-19T15:03:04.000+03:00
diff --git a/ydb/core/driver_lib/run/kikimr_services_initializers.cpp b/ydb/core/driver_lib/run/kikimr_services_initializers.cpp
@@ -397,6 +397,9 @@ static TInterconnectSettings GetInterconnectSettings(const NKikimrConfig::TInter
             break;
     }
     result.TlsAuthOnly = config.GetTlsAuthOnly();
+    if (const auto& forbidden = config.GetForbiddenSignatureAlgorithms(); !forbidden.empty()) {
+        result.ForbiddenSignatureAlgorithms = {forbidden.begin(), forbidden.end()};
+    }
 
     if (config.HasTCPSocketBufferSize())
         result.TCPSocketBufferSize = config.GetTCPSocketBufferSize();
diff --git a/ydb/core/protos/config.proto b/ydb/core/protos/config.proto
@@ -397,6 +397,7 @@ message TInterconnectConfig {
     optional bool BindOnAllAddresses = 16 [default = true];
     optional EEncryptionMode EncryptionMode = 17 [default = DISABLED];
     optional bool TlsAuthOnly = 38; // do not encrypt traffic
+    repeated string ForbiddenSignatureAlgorithms = 50;
     optional bool EnforceScopeValidation = 18;
     optional bytes Certificate = 30; // in PEM format
     optional bytes PrivateKey = 31; // in PEM format
diff --git a/ydb/library/actors/interconnect/interconnect_common.h b/ydb/library/actors/interconnect/interconnect_common.h
@@ -43,6 +43,7 @@ namespace NActors {
         TString PrivateKey; // private key for the certificate in PEM format
         TString CaFilePath; // path to certificate authority file
         TString CipherList; // encryption algorithms
+        THashSet<TString> ForbiddenSignatureAlgorithms;
         TDuration MessagePendingTimeout = TDuration::Seconds(1); // timeout for which messages are queued while in PendingConnection state
         ui64 MessagePendingSize = Max<ui64>(); // size of the queue
         ui32 MaxSerializedEventSize = NActors::EventMaxByteSize;
diff --git a/ydb/library/actors/interconnect/interconnect_stream.cpp b/ydb/library/actors/interconnect/interconnect_stream.cpp
@@ -1,4 +1,5 @@
 #include "interconnect_stream.h"
+#include "interconnect_common.h"
 #include "logging.h"
 #include "poller_actor.h"
 #include <library/cpp/openssl/init/init.h>
@@ -287,10 +288,12 @@ namespace NInterconnect {
 
     class TSecureSocketContext::TImpl {
         std::unique_ptr<SSL_CTX, TDeleter> Ctx;
+        TIntrusivePtr<NActors::TInterconnectProxyCommon> Common;
 
     public:
-        TImpl(const TString& certificate, const TString& privateKey, const TString& caFilePath,
-                const TString& ciphers) {
+        TImpl(TIntrusivePtr<NActors::TInterconnectProxyCommon> common)
+            : Common(std::move(common))
+        {
             int ret;
             InitOpenSSL();
 #if OPENSSL_VERSION_NUMBER < 0x10100000L
@@ -308,7 +311,7 @@ namespace NInterconnect {
             SSL_CTX_set_mode(*this, SSL_MODE_ENABLE_PARTIAL_WRITE | SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);
 
             // apply certificates in SSL context
-            if (certificate) {
+            if (const TString& certificate = Common->Settings.Certificate) {
                 std::unique_ptr<BIO, TDeleter> bio(BIO_new_mem_buf(certificate.data(), certificate.size()));
                 Y_ABORT_UNLESS(bio);
 
@@ -329,19 +332,19 @@ namespace NInterconnect {
                     // we must not free memory if certificate was added successfully by SSL_CTX_add0_chain_cert
                 }
             }
-            if (privateKey) {
+            if (const TString& privateKey = Common->Settings.PrivateKey) {
                 std::unique_ptr<BIO, TDeleter> bio(BIO_new_mem_buf(privateKey.data(), privateKey.size()));
                 Y_ABORT_UNLESS(bio);
                 std::unique_ptr<EVP_PKEY, TDeleter> pkey(PEM_read_bio_PrivateKey(bio.get(), nullptr, nullptr, nullptr));
                 Y_ABORT_UNLESS(pkey);
                 ret = SSL_CTX_use_PrivateKey(Ctx.get(), pkey.get());
                 Y_ABORT_UNLESS(ret == 1);
             }
-            if (caFilePath) {
+            if (const TString& caFilePath = Common->Settings.CaFilePath) {
                 ret = SSL_CTX_load_verify_locations(Ctx.get(), caFilePath.data(), nullptr);
                 Y_ABORT_UNLESS(ret == 1);
             }
-
+            const TString& ciphers = Common->Settings.CipherList;
             int success = SSL_CTX_set_cipher_list(Ctx.get(), ciphers ? ciphers.data() : "AES128-GCM-SHA256");
             Y_ABORT_UNLESS(success, "failed to set cipher list");
         }
@@ -355,34 +358,55 @@ namespace NInterconnect {
             return index;
         }
 
+        static int GetContextIndex() {
+            static int index = SSL_get_ex_new_index(0, nullptr, nullptr, nullptr, nullptr);
+            return index;
+        }
+
     private:
         static int Verify(int preverify, X509_STORE_CTX *ctx) {
+            X509* const current = X509_STORE_CTX_get_current_cert(ctx);
+            auto* const ssl = static_cast<SSL*>(X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()));
+            auto* const errp = static_cast<TString*>(SSL_get_ex_data(ssl, GetExIndex()));
+            auto* const secureSocketContext = static_cast<TSecureSocketContext*>(SSL_get_ex_data(ssl, GetContextIndex()));
+
             if (!preverify) {
-                X509 *badCert = X509_STORE_CTX_get_current_cert(ctx);
                 int err = X509_STORE_CTX_get_error(ctx);
                 int depth = X509_STORE_CTX_get_error_depth(ctx);
-                SSL *ssl = static_cast<SSL*>(X509_STORE_CTX_get_ex_data(ctx, SSL_get_ex_data_X509_STORE_CTX_idx()));
-                TString *errp = static_cast<TString*>(SSL_get_ex_data(ssl, GetExIndex()));
                 char buffer[1024];
-                X509_NAME_oneline(X509_get_subject_name(badCert), buffer, sizeof(buffer));
+                X509_NAME_oneline(X509_get_subject_name(current), buffer, sizeof(buffer));
                 TStringBuilder s;
                 s << "Error during certificate validation"
                     << " error# " << X509_verify_cert_error_string(err)
                     << " depth# " << depth
                     << " cert# " << buffer;
                 if (err == X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT) {
-                    X509_NAME_oneline(X509_get_issuer_name(badCert), buffer, sizeof(buffer));
+                    X509_NAME_oneline(X509_get_issuer_name(current), buffer, sizeof(buffer));
                     s << " issuer# " << buffer;
                 }
                 *errp = s;
+            } else if (auto& forbidden = secureSocketContext->Impl->Common->Settings.ForbiddenSignatureAlgorithms) {
+                do {
+                    int pknid;
+                    if (X509_get_signature_info(current, nullptr, &pknid, nullptr, nullptr) != 1) {
+                        *errp = TStringBuilder() << "failed to acquire signature info";
+                    } else if (const char *ln = OBJ_nid2ln(pknid); ln && forbidden.contains(ln)) {
+                        *errp = TStringBuilder() << "forbidden signature algorithm: " << ln;
+                    } else if (const char *sn = OBJ_nid2ln(pknid); sn && forbidden.contains(sn)) {
+                        *errp = TStringBuilder() << "forbidden signature algorithm: " << sn;
+                    } else {
+                        break;
+                    }
+                    X509_STORE_CTX_set_error(ctx, X509_V_ERR_UNSUPPORTED_SIGNATURE_ALGORITHM);
+                    return 0;
+                } while (false);
             }
             return preverify;
         }
     };
 
-    TSecureSocketContext::TSecureSocketContext(const TString& certificate, const TString& privateKey,
-            const TString& caFilePath, const TString& ciphers)
-        : Impl(new TImpl(certificate, privateKey, caFilePath, ciphers))
+    TSecureSocketContext::TSecureSocketContext(TIntrusivePtr<NActors::TInterconnectProxyCommon> common)
+        : Impl(new TImpl(std::move(common)))
     {}
 
     TSecureSocketContext::~TSecureSocketContext()
@@ -395,12 +419,13 @@ namespace NInterconnect {
         bool WantWrite_ = false;
 
     public:
-        TImpl(SSL_CTX *ctx, int fd)
+        TImpl(SSL_CTX *ctx, int fd, TSecureSocketContext *secureSocketContext)
             : Ssl(SSL_new(ctx))
         {
             Y_ABORT_UNLESS(Ssl, "SSL_new() failed");
             SSL_set_fd(Ssl, fd);
             SSL_set_ex_data(Ssl, TSecureSocketContext::TImpl::GetExIndex(), &ErrorDescription);
+            SSL_set_ex_data(Ssl, TSecureSocketContext::TImpl::GetContextIndex(), secureSocketContext);
         }
 
         ~TImpl() {
@@ -546,18 +571,37 @@ namespace NInterconnect {
 
         TString GetPeerCommonName() const {
             TString res;
-            if (X509 *cert = SSL_get_peer_certificate(Ssl)) {
+            if (std::unique_ptr<X509, void(*)(X509*)> cert{SSL_get_peer_certificate(Ssl), &X509_free}) {
                 char buffer[256];
                 memset(buffer, 0, sizeof(buffer));
-                if (X509_NAME *name = X509_get_subject_name(cert)) {
+                if (X509_NAME *name = X509_get_subject_name(cert.get())) {
                     X509_NAME_get_text_by_NID(name, NID_commonName, buffer, sizeof(buffer));
                 }
-                X509_free(cert);
                 res = TString(buffer, strnlen(buffer, sizeof(buffer)));
             }
             return res;
         }
 
+        TString GetSignatureAlgorithm() const {
+            TString res;
+            if (std::unique_ptr<X509, void(*)(X509*)> cert{SSL_get_peer_certificate(Ssl), &X509_free}) {
+                int mdnid;
+                int pknid;
+                int secbits;
+                if (X509_get_signature_info(cert.get(), &mdnid, &pknid, &secbits, nullptr) == 1) {
+                    res = TStringBuilder()
+                        << "md: " << OBJ_nid2ln(mdnid)
+                        << " pk: " << OBJ_nid2ln(pknid)
+                        << " bits: " << secbits;
+                } else {
+                    res = "<failed to get signature info>";
+                }
+            } else {
+                res = "<failed to get peer certificate>";
+            }
+            return res;
+        }
+
         bool WantRead() const {
             return WantRead_;
         }
@@ -605,7 +649,7 @@ namespace NInterconnect {
     TSecureSocket::TSecureSocket(TStreamSocket& socket, TSecureSocketContext::TPtr context)
         : TStreamSocket(socket.ReleaseDescriptor())
         , Context(std::move(context))
-        , Impl(new TImpl(*Context->Impl, Descriptor))
+        , Impl(new TImpl(*Context->Impl, Descriptor, Context.get()))
     {}
 
     TSecureSocket::~TSecureSocket()
@@ -651,6 +695,10 @@ namespace NInterconnect {
         return Impl->GetPeerCommonName();
     }
 
+    TString TSecureSocket::GetSignatureAlgorithm() const {
+        return Impl->GetSignatureAlgorithm();
+    }
+
     bool TSecureSocket::WantRead() const {
         return Impl->WantRead();
     }
diff --git a/ydb/library/actors/interconnect/interconnect_stream.h b/ydb/library/actors/interconnect/interconnect_stream.h
@@ -16,6 +16,7 @@
 
 namespace NActors {
     class TPollerToken;
+    struct TInterconnectProxyCommon;
 }
 
 namespace NInterconnect {
@@ -82,8 +83,7 @@ namespace NInterconnect {
         friend class TSecureSocket;
 
     public:
-        TSecureSocketContext(const TString& certificate, const TString& privateKey, const TString& caFilePath,
-            const TString& ciphers);
+        TSecureSocketContext(TIntrusivePtr<NActors::TInterconnectProxyCommon> common);
         ~TSecureSocketContext();
 
     public:
@@ -121,6 +121,7 @@ namespace NInterconnect {
         int GetCipherBits() const;
         TString GetProtocolName() const;
         TString GetPeerCommonName() const;
+        TString GetSignatureAlgorithm() const;
 
         bool WantRead() const;
         bool WantWrite() const;
diff --git a/ydb/library/actors/interconnect/interconnect_tcp_proxy.cpp b/ydb/library/actors/interconnect/interconnect_tcp_proxy.cpp
@@ -21,8 +21,7 @@ namespace NActors {
         , PeerNodeId(node)
         , DynamicPtr(dynamicPtr)
         , Common(std::move(common))
-        , SecureContext(new NInterconnect::TSecureSocketContext(Common->Settings.Certificate, Common->Settings.PrivateKey,
-                Common->Settings.CaFilePath, Common->Settings.CipherList))
+        , SecureContext(new NInterconnect::TSecureSocketContext(Common))
     {
         Y_ABORT_UNLESS(Common);
         Y_ABORT_UNLESS(Common->NameserviceId);
diff --git a/ydb/library/actors/interconnect/interconnect_tcp_session.cpp b/ydb/library/actors/interconnect/interconnect_tcp_session.cpp
@@ -1183,6 +1183,14 @@ namespace NActors {
                                         str << x->GetPeerCommonName();
                                     }
                                 }
+                                TABLER() {
+                                    TABLED() {
+                                        str << "Signature algorithm";
+                                    }
+                                    TABLED() {
+                                        str << x->GetSignatureAlgorithm();
+                                    }
+                                }
                             }
                             TABLER() {
                                 TABLED() { str << "AuthOnly CN"; }

Original file line number	Diff line number	Diff line change
`@@ -397,6 +397,9 @@ static TInterconnectSettings GetInterconnectSettings(const NKikimrConfig::TInter`
`397`	`397`	`break;`
`398`	`398`	`}`
`399`	`399`	`result.TlsAuthOnly = config.GetTlsAuthOnly();`
	`400`	`+ if (const auto& forbidden = config.GetForbiddenSignatureAlgorithms(); !forbidden.empty()) {`
	`401`	`+ result.ForbiddenSignatureAlgorithms = {forbidden.begin(), forbidden.end()};`
	`402`	`+ }`
`400`	`403`
`401`	`404`	`if (config.HasTCPSocketBufferSize())`
`402`	`405`	`result.TCPSocketBufferSize = config.GetTCPSocketBufferSize();`
Original file line number	Diff line number	Diff line change
`@@ -1183,6 +1183,14 @@ namespace NActors {`
`1183`	`1183`	`str << x->GetPeerCommonName();`
`1184`	`1184`	`}`
`1185`	`1185`	`}`
	`1186`	`+ TABLER() {`
	`1187`	`+ TABLED() {`
	`1188`	`+ str << "Signature algorithm";`
	`1189`	`+ }`
	`1190`	`+ TABLED() {`
	`1191`	`+ str << x->GetSignatureAlgorithm();`
	`1192`	`+ }`
	`1193`	`+ }`
`1186`	`1194`	`}`
`1187`	`1195`	`TABLER() {`
`1188`	`1196`	`TABLED() { str << "AuthOnly CN"; }`