Merge 70005b6 into c70d3eb

ydb/core/protos/config.proto

@@ -1795,8 +1795,15 @@ message TClientCertificateAuthorization {
         repeated string Suffixes = 3;
     }
 
+    // Matches subject alternative names (DNS) and Common Name (CN) in certificate
+    message TSubjectDns {
+        repeated string Values = 1;
+        repeated string Suffixes = 2;
+    }
+
     message TClientCertificateDefinition {
         repeated TSubjectTerm SubjectTerms = 1;
+        optional TSubjectDns SubjectDns = 5;
         optional bool CanCheckNodeHostByCN = 2 [default = false];
         repeated string MemberGroups = 3;
         optional bool RequireSameIssuer = 4 [default = true];

ydb/core/security/certificate_check/cert_auth_processor.cpp

@@ -1,6 +1,7 @@
 #include "cert_auth_processor.h"
 
 #include <openssl/x509.h>
+#include <openssl/x509v3.h>
 #include <openssl/pem.h>
 #include <openssl/bio.h>
 #include <openssl/objects.h>
@@ -100,6 +101,56 @@ TVector<std::pair<TString, TString>> X509CertificateReader::ReadIssuerTerms(cons
     return ReadTerms(name);
 }
 
+static void FreeList(GENERAL_NAMES* list) {
+    sk_GENERAL_NAME_pop_free(list, GENERAL_NAME_free);
+}
+
+TVector<TString> X509CertificateReader::ReadSubjectDns(const X509Ptr& x509, const std::vector<std::pair<TString, TString>>& subjectTerms) {
+    TVector<TString> result;
+    // 1. Subject's common name (CN) must be a subject DNS name, so add it to DNS names of subject first
+    for (const auto& [k, v] : subjectTerms) {
+        if (k == "CN") {
+            result.emplace_back(v);
+        }
+    }
+
+    using TGeneralNamesPtr = std::unique_ptr<GENERAL_NAMES, deleter_from_fn<&FreeList>>;
+    TGeneralNamesPtr subjectAltNames((GENERAL_NAMES*)X509_get_ext_d2i(x509.get(), NID_subject_alt_name, NULL, NULL));
+    if (!subjectAltNames) {
+        return result;
+    }
+    const int subjectAltNamesCount = sk_GENERAL_NAME_num(subjectAltNames.get());
+    if (subjectAltNamesCount <= 0) {
+        return result;
+    }
+
+    result.reserve(static_cast<size_t>(subjectAltNamesCount) + result.size());
+    // 2. Additionally find subject alternative names with type=DNS
+    for (int i = 0; i < subjectAltNamesCount; ++i) {
+        const GENERAL_NAME* name = sk_GENERAL_NAME_value(subjectAltNames.get(), i);
+        if (!name) {
+            continue;
+        }
+        if (name->type == GEN_DNS) {
+            const ASN1_STRING* value = name->d.dNSName;
+            if (!value) {
+                continue;
+            }
+
+            const char* data = reinterpret_cast<const char*>(ASN1_STRING_get0_data(value));
+            if (!data) {
+                continue;
+            }
+            int size = ASN1_STRING_length(value);
+            if (size <= 0) {
+                continue;
+            }
+            result.emplace_back(data, static_cast<size_t>(size));
+        }
+    }
+    return result;
+}
+
 TString X509CertificateReader::GetFingerprint(const X509Ptr& x509) {
     static constexpr size_t FINGERPRINT_LENGTH = SHA_DIGEST_LENGTH;
     unsigned char fingerprint[FINGERPRINT_LENGTH];
@@ -109,14 +160,16 @@ TString X509CertificateReader::GetFingerprint(const X509Ptr& x509) {
     return HexEncode(fingerprint, FINGERPRINT_LENGTH);
 }
 
-TCertificateAuthorizationParams::TCertificateAuthorizationParams(const TDN& dn, bool requireSameIssuer, const std::vector<TString>& groups)
+TCertificateAuthorizationParams::TCertificateAuthorizationParams(const TDN& dn, const std::optional<TRDN>& subjectDns, bool requireSameIssuer, const std::vector<TString>& groups)
     : SubjectDn(dn)
+    , SubjectDns(subjectDns)
     , RequireSameIssuer(requireSameIssuer)
     , Groups(groups)
 {}
 
-TCertificateAuthorizationParams::TCertificateAuthorizationParams(TDN&& dn, bool requireSameIssuer, std::vector<TString>&& groups)
+TCertificateAuthorizationParams::TCertificateAuthorizationParams(TDN&& dn, std::optional<TRDN>&& subjectDns, bool requireSameIssuer, std::vector<TString>&& groups)
     : SubjectDn(std::move(dn))
+    , SubjectDns(std::move(subjectDns))
     , RequireSameIssuer(requireSameIssuer)
     , Groups(std::move(groups))
 {}
@@ -127,59 +180,44 @@ TCertificateAuthorizationParams::TDN& TCertificateAuthorizationParams::TDN::AddR
 }
 
 TCertificateAuthorizationParams::operator bool() const {
-    return SubjectDn;
+    return SubjectDn || SubjectDns;
 }
 
-bool TCertificateAuthorizationParams::CheckSubject(const std::unordered_map<TString, std::vector<TString>>& subjectDescription) const {
-    bool isDescriptionMatched = false;
-    for (const auto& rdn: SubjectDn.RDNs) {
-        isDescriptionMatched = false;
+bool TCertificateAuthorizationParams::CheckSubject(const std::unordered_map<TString, std::vector<TString>>& subjectDescription, const std::vector<TString>& subjectDns) const {
+    for (const TRDN& rdn: SubjectDn.RDNs) {
         auto fieldIt = subjectDescription.find(rdn.Attribute);
         if (fieldIt == subjectDescription.cend()) {
-            break;
+            return false;
         }
 
         const auto& attributeValues = fieldIt->second;
-        bool attributeMatched = false;
-        for (const auto& attributeValue : attributeValues) {
-            attributeMatched = false;
-            for (const auto& value: rdn.Values) {
-                if (value == attributeValue) {
-                    attributeMatched = true;
-                    break;
-                }
-            }
-            if (!attributeMatched) {
-                for (const auto& suffix: rdn.Suffixes) {
-                    if (attributeValue.EndsWith(suffix)) {
-                        attributeMatched = true;
-                        break;
-                    }
-                }
-            }
-            if (!attributeMatched) {
+        if (!rdn.Match(attributeValues)) {
+            return false;
+        }
+    }
+
+    if (SubjectDns) {
+        bool dnsMatched = false;
+        for (const TString& dns : subjectDns) {
+            if (SubjectDns->Match(dns)) {
+                dnsMatched = true;
                 break;
             }
         }
-        if (!attributeMatched) {
-            isDescriptionMatched = false;
-            break;
+        if (!dnsMatched) {
+            return false;
         }
-        isDescriptionMatched = true;
     }
 
-    if (isDescriptionMatched) {
-        return true;
-    }
-    return false;
+    return true;
 }
 
 TCertificateAuthorizationParams::TDN::operator bool() const {
     return !RDNs.empty();
 }
 
-TCertificateAuthorizationParams::TRDN::TRDN(const TString& Attribute)
-    :Attribute(Attribute)
+TCertificateAuthorizationParams::TRDN::TRDN(const TString& attribute)
+    : Attribute(attribute)
 {}
 
 TCertificateAuthorizationParams::TRDN& TCertificateAuthorizationParams::TRDN::AddValue(const TString& val)
@@ -194,4 +232,30 @@ TCertificateAuthorizationParams::TRDN& TCertificateAuthorizationParams::TRDN::Ad
     return *this;
 }
 
+bool TCertificateAuthorizationParams::TRDN::Match(const TString& value) const
+{
+    for (const auto& v : Values) {
+        if (value == v) {
+            return true;
+        }
+    }
+    for (const auto& s : Suffixes) {
+        if (value.EndsWith(s)) {
+            return true;
+        }
+    }
+
+    return false;
+}
+
+bool TCertificateAuthorizationParams::TRDN::Match(const std::vector<TString>& values) const
+{
+    for (const auto& value : values) {
+        if (!Match(value)) {
+            return false;
+        }
+    }
+    return true;
+}
+
 }  //namespace NKikimr {

ydb/core/security/certificate_check/cert_auth_processor.h

@@ -15,9 +15,11 @@ struct TCertificateAuthorizationParams {
         TVector<TString> Values;
         TVector<TString> Suffixes;
 
-        TRDN(const TString& Attribute);
+        TRDN(const TString& attribute);
         TRDN& AddValue(const TString& val);
         TRDN& AddSuffix(const TString& suffix);
+        bool Match(const std::vector<TString>& values) const;
+        bool Match(const TString& value) const;
     };
 
     struct TDN {
@@ -27,11 +29,11 @@ struct TCertificateAuthorizationParams {
         operator bool () const;
     };
 
-    TCertificateAuthorizationParams(const TDN& dn = TDN(), bool requireSameIssuer = true, const std::vector<TString>& groups = {});
-    TCertificateAuthorizationParams(TDN&& dn, bool requireSameIssuer = true, std::vector<TString>&& groups = {});
+    TCertificateAuthorizationParams(const TDN& dn = TDN(), const std::optional<TRDN>& subjectDns = std::nullopt, bool requireSameIssuer = true, const std::vector<TString>& groups = {});
+    TCertificateAuthorizationParams(TDN&& dn, std::optional<TRDN>&& subjectDns, bool requireSameIssuer = true, std::vector<TString>&& groups = {});
 
     operator bool () const;
-    bool CheckSubject(const std::unordered_map<TString, std::vector<TString>>& subjectDescription) const;
+    bool CheckSubject(const std::unordered_map<TString, std::vector<TString>>& subjectDescription, const std::vector<TString>& subjectDns) const;
     void SetSubjectDn(const TDN& subjectDn) {
         SubjectDn = subjectDn;
     }
@@ -42,6 +44,7 @@ struct TCertificateAuthorizationParams {
 
     bool CanCheckNodeByAttributeCN = false;
     TDN SubjectDn;
+    std::optional<TRDN> SubjectDns;
     bool RequireSameIssuer = true;
     std::vector<TString> Groups;
 };
@@ -61,6 +64,7 @@ struct X509CertificateReader {
 
     static X509Ptr ReadCertAsPEM(const TStringBuf& cert);
     static TVector<std::pair<TString, TString>> ReadSubjectTerms(const X509Ptr& x509);
+    static TVector<TString> ReadSubjectDns(const X509Ptr& x509, const std::vector<std::pair<TString, TString>>& subjectTerms);
     static TVector<std::pair<TString, TString>> ReadAllSubjectTerms(const X509Ptr& x509);
     static TVector<std::pair<TString, TString>> ReadIssuerTerms(const X509Ptr& x509);
     static TString GetFingerprint(const X509Ptr& x509);

ydb/core/security/certificate_check/cert_auth_utils.cpp

@@ -17,7 +17,7 @@
 
 namespace NKikimr {
 
-std::vector<TCertificateAuthorizationParams> GetCertificateAuthorizationParams(const NKikimrConfig::TClientCertificateAuthorization &clientCertificateAuth) {
+std::vector<TCertificateAuthorizationParams> GetCertificateAuthorizationParams(const NKikimrConfig::TClientCertificateAuthorization& clientCertificateAuth) {
     std::vector<TCertificateAuthorizationParams> certAuthParams;
     certAuthParams.reserve(clientCertificateAuth.ClientCertificateDefinitionsSize());
 
@@ -33,9 +33,19 @@ std::vector<TCertificateAuthorizationParams> GetCertificateAuthorizationParams(c
             }
             dn.AddRDN(std::move(rdn));
         }
-        if (dn) {
+        std::optional<TCertificateAuthorizationParams::TRDN> subjectDns;
+        if (const auto& subjectDnsCfg = clientCertificateDefinition.GetSubjectDns(); subjectDnsCfg.ValuesSize() || subjectDnsCfg.SuffixesSize()) {
+            TCertificateAuthorizationParams::TRDN& dns = subjectDns.emplace(TString());
+            for (const auto& value: subjectDnsCfg.GetValues()) {
+                dns.AddValue(value);
+            }
+            for (const auto& suffix: subjectDnsCfg.GetSuffixes()) {
+                dns.AddSuffix(suffix);
+            }
+        }
+        if (dn || subjectDns) {
             std::vector<TString> groups(clientCertificateDefinition.GetMemberGroups().cbegin(), clientCertificateDefinition.GetMemberGroups().cend());
-            certAuthParams.emplace_back(std::move(dn), clientCertificateDefinition.GetRequireSameIssuer(), std::move(groups));
+            certAuthParams.emplace_back(std::move(dn), std::move(subjectDns), clientCertificateDefinition.GetRequireSameIssuer(), std::move(groups));
         }
     }
 
@@ -130,8 +140,8 @@ int FillNameFromProps(X509_NAME* name, const TProps& props) {
         return 1;
     }
 
-    if (!props.Coutry.empty()) {
-        X509_NAME_add_entry_by_txt(name, SN_countryName, MBSTRING_ASC, (const unsigned char*)props.Coutry.c_str(), -1, -1, 0);
+    if (!props.Country.empty()) {
+        X509_NAME_add_entry_by_txt(name, SN_countryName, MBSTRING_ASC, (const unsigned char*)props.Country.c_str(), -1, -1, 0);
     }
 
     if (!props.State.empty()) {
@@ -377,7 +387,7 @@ X509REQPtr GenerateRequest(PKeyPtr& pkey, const TProps& props) {
     return std::move(request);
 }
 
-X509Ptr SingRequest(X509REQPtr& request, X509Ptr& rootCert, PKeyPtr& rootKey, const TProps& props) {
+X509Ptr SignRequest(X509REQPtr& request, X509Ptr& rootCert, PKeyPtr& rootKey, const TProps& props) {
     auto* pktmp = X509_REQ_get0_pubkey(request.get()); // X509_REQ_get0_pubkey returns the key, that shouldn't freed
     CHECK(pktmp, "Error unpacking public key from request.");
 
@@ -455,7 +465,7 @@ TCertAndKey GenerateSignedCert(const TCertAndKey& rootCA, const TProps& props) {
 
     auto rootCert = ReadCertAsPEM(rootCA.Certificate);
     auto rootKey = ReadPrivateKeyAsPEM(rootCA.PrivateKey);
-    auto cert = SingRequest(request, rootCert, rootKey, props); // NID_authority_key_identifier must see ca
+    auto cert = SignRequest(request, rootCert, rootKey, props); // NID_authority_key_identifier must see ca
 
     TCertAndKey result;
     result.Certificate = WriteAsPEM(cert);
@@ -475,7 +485,7 @@ TProps TProps::AsCA() {
     TProps props;
 
     props.SecondsValid = 3*365 * 24 * 60 *60; // 3 years
-    props.Coutry = "RU";
+    props.Country = "RU";
     props.State = "MSK";
     props.Location = "MSK";
     props.Organization = "YA";

ydb/core/security/certificate_check/cert_auth_utils.h

@@ -22,7 +22,7 @@ struct TCertAndKey {
 
 struct TProps {
     long SecondsValid = 0;
-    std::string Coutry; // C
+    std::string Country; // C
     std::string State; // ST
     std::string Location; // L
     std::string Organization; // O

ydb/core/security/certificate_check/cert_check.cpp

@@ -71,6 +71,7 @@ TCertificateChecker::TReadClientSubjectResult TCertificateChecker::ReadSubjectFr
         result.Error = { .Message = "Cannot extract subject from client certificate", .Retryable = false };
         return result;
     }
+    result.SubjectDns = X509CertificateReader::ReadSubjectDns(pemCertificates.ClientCertX509, result.SubjectDn);
     return result;
 }
 
@@ -84,14 +85,14 @@ TString TCertificateChecker::CreateUserSidFromSubjectDn(const std::vector<std::p
     return userSid;
 }
 
-TEvTicketParser::TError TCertificateChecker::CheckClientSubject(const std::vector<std::pair<TString, TString>>& subjectDn, const TCertificateAuthorizationParams& authParams) const {
+TEvTicketParser::TError TCertificateChecker::CheckClientSubject(const TReadClientSubjectResult& subjectInfo, const TCertificateAuthorizationParams& authParams) const {
     std::unordered_map<TString, std::vector<TString>> subjectDescription;
-    for (const auto& [attribute, value] : subjectDn) {
+    for (const auto& [attribute, value] : subjectInfo.SubjectDn) {
         auto& attributeValues = subjectDescription[attribute];
         attributeValues.push_back(value);
     }
 
-    if (!authParams.CheckSubject(subjectDescription)) {
+    if (!authParams.CheckSubject(subjectDescription, subjectInfo.SubjectDns)) {
         return { .Message = "Client certificate failed verification", .Retryable = false };
     }
     return {};
@@ -128,7 +129,7 @@ TCertificateChecker::TCertificateCheckResult TCertificateChecker::CheckClientCer
             continue;
         }
 
-        auto checkClientSubjectError = CheckClientSubject(readClientSubjectResult.SubjectDn, authParams);
+        auto checkClientSubjectError = CheckClientSubject(readClientSubjectResult, authParams);
         if (!checkClientSubjectError.empty()) {
             continue;
         }

ydb/core/security/certificate_check/cert_check.h

@@ -27,6 +27,7 @@ class TCertificateChecker {
 
     struct TReadClientSubjectResult {
         std::vector<std::pair<TString, TString>> SubjectDn;
+        std::vector<TString> SubjectDns; // Subject alternative names, DNS
         TEvTicketParser::TError Error;
     };
 
@@ -47,7 +48,7 @@ class TCertificateChecker {
     TEvTicketParser::TError CheckIssuers(const TPemCertificates& pemCertificates) const;
     TReadClientSubjectResult ReadSubjectFromClientCertificate(const TPemCertificates& pemCertificates) const;
     TString CreateUserSidFromSubjectDn(const std::vector<std::pair<TString, TString>>& subjectDn) const;
-    TEvTicketParser::TError CheckClientSubject(const std::vector<std::pair<TString, TString>>& subjectDn, const TCertificateAuthorizationParams& authParams) const;
+    TEvTicketParser::TError CheckClientSubject(const TReadClientSubjectResult& subjectInfo, const TCertificateAuthorizationParams& authParams) const;
     TCertificateCheckResult DefaultCheckClientCertificate(const TPemCertificates& pemCertificates) const;
     TCertificateCheckResult CheckClientCertificate(const TPemCertificates& pemCertificates) const;
     TString GetDefaultGroup() const;