Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support filter on DNS names in client cert authentication #7797

Merged
merged 1 commit into from
Aug 15, 2024
Merged

Support filter on DNS names in client cert authentication #7797

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions ydb/core/protos/config.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1777,8 +1777,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];
Expand Down
136 changes: 100 additions & 36 deletions ydb/core/security/certificate_check/cert_auth_processor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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>
Expand Down Expand Up @@ -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];
Expand All @@ -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))
{}
Expand All @@ -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)
Expand All @@ -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 {
12 changes: 8 additions & 4 deletions ydb/core/security/certificate_check/cert_auth_processor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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;
}
Expand All @@ -42,6 +44,7 @@ struct TCertificateAuthorizationParams {

bool CanCheckNodeByAttributeCN = false;
TDN SubjectDn;
std::optional<TRDN> SubjectDns;
bool RequireSameIssuer = true;
std::vector<TString> Groups;
};
Expand All @@ -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);
Expand Down
26 changes: 18 additions & 8 deletions ydb/core/security/certificate_check/cert_auth_utils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand All @@ -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));
}
}

Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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.");

Expand Down Expand Up @@ -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);
Expand All @@ -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";
Expand Down
2 changes: 1 addition & 1 deletion ydb/core/security/certificate_check/cert_auth_utils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions ydb/core/security/certificate_check/cert_check.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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 {};
Expand Down Expand Up @@ -128,7 +129,7 @@ TCertificateChecker::TCertificateCheckResult TCertificateChecker::CheckClientCer
continue;
}

auto checkClientSubjectError = CheckClientSubject(readClientSubjectResult.SubjectDn, authParams);
auto checkClientSubjectError = CheckClientSubject(readClientSubjectResult, authParams);
if (!checkClientSubjectError.empty()) {
continue;
}
Expand Down
3 changes: 2 additions & 1 deletion ydb/core/security/certificate_check/cert_check.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand All @@ -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;
Expand Down
Loading
Loading