Skip to content

Commit e050f63

Browse files
UgnineSirdisUgnineSirdis
authored
Support filter on DNS names in client cert authentication (ydb-platform#7797)
1 parent 0ce24a5 commit e050f63

File tree

10 files changed

+445
-63
lines changed

10 files changed

+445
-63
lines changed

‎ydb/core/protos/config.proto

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1795,8 +1795,15 @@ message TClientCertificateAuthorization {
17951795
repeated string Suffixes = 3;
17961796
}
17971797

1798+
// Matches subject alternative names (DNS) and Common Name (CN) in certificate
1799+
message TSubjectDns {
1800+
repeated string Values = 1;
1801+
repeated string Suffixes = 2;
1802+
}
1803+
17981804
message TClientCertificateDefinition {
17991805
repeated TSubjectTerm SubjectTerms = 1;
1806+
optional TSubjectDns SubjectDns = 5;
18001807
optional bool CanCheckNodeHostByCN = 2 [default = false];
18011808
repeated string MemberGroups = 3;
18021809
optional bool RequireSameIssuer = 4 [default = true];

‎ydb/core/security/certificate_check/cert_auth_processor.cpp

Lines changed: 100 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "cert_auth_processor.h"
22

33
#include <openssl/x509.h>
4+
#include <openssl/x509v3.h>
45
#include <openssl/pem.h>
56
#include <openssl/bio.h>
67
#include <openssl/objects.h>
@@ -100,6 +101,56 @@ TVector<std::pair<TString, TString>> X509CertificateReader::ReadIssuerTerms(cons
100101
return ReadTerms(name);
101102
}
102103

104+
static void FreeList(GENERAL_NAMES* list) {
105+
sk_GENERAL_NAME_pop_free(list, GENERAL_NAME_free);
106+
}
107+
108+
TVector<TString> X509CertificateReader::ReadSubjectDns(const X509Ptr& x509, const std::vector<std::pair<TString, TString>>& subjectTerms) {
109+
TVector<TString> result;
110+
// 1. Subject's common name (CN) must be a subject DNS name, so add it to DNS names of subject first
111+
for (const auto& [k, v] : subjectTerms) {
112+
if (k == "CN") {
113+
result.emplace_back(v);
114+
}
115+
}
116+
117+
using TGeneralNamesPtr = std::unique_ptr<GENERAL_NAMES, deleter_from_fn<&FreeList>>;
118+
TGeneralNamesPtr subjectAltNames((GENERAL_NAMES*)X509_get_ext_d2i(x509.get(), NID_subject_alt_name, NULL, NULL));
119+
if (!subjectAltNames) {
120+
return result;
121+
}
122+
const int subjectAltNamesCount = sk_GENERAL_NAME_num(subjectAltNames.get());
123+
if (subjectAltNamesCount <= 0) {
124+
return result;
125+
}
126+
127+
result.reserve(static_cast<size_t>(subjectAltNamesCount) + result.size());
128+
// 2. Additionally find subject alternative names with type=DNS
129+
for (int i = 0; i < subjectAltNamesCount; ++i) {
130+
const GENERAL_NAME* name = sk_GENERAL_NAME_value(subjectAltNames.get(), i);
131+
if (!name) {
132+
continue;
133+
}
134+
if (name->type == GEN_DNS) {
135+
const ASN1_STRING* value = name->d.dNSName;
136+
if (!value) {
137+
continue;
138+
}
139+
140+
const char* data = reinterpret_cast<const char*>(ASN1_STRING_get0_data(value));
141+
if (!data) {
142+
continue;
143+
}
144+
int size = ASN1_STRING_length(value);
145+
if (size <= 0) {
146+
continue;
147+
}
148+
result.emplace_back(data, static_cast<size_t>(size));
149+
}
150+
}
151+
return result;
152+
}
153+
103154
TString X509CertificateReader::GetFingerprint(const X509Ptr& x509) {
104155
static constexpr size_t FINGERPRINT_LENGTH = SHA_DIGEST_LENGTH;
105156
unsigned char fingerprint[FINGERPRINT_LENGTH];
@@ -109,14 +160,16 @@ TString X509CertificateReader::GetFingerprint(const X509Ptr& x509) {
109160
return HexEncode(fingerprint, FINGERPRINT_LENGTH);
110161
}
111162

112-
TCertificateAuthorizationParams::TCertificateAuthorizationParams(const TDN& dn, bool requireSameIssuer, const std::vector<TString>& groups)
163+
TCertificateAuthorizationParams::TCertificateAuthorizationParams(const TDN& dn, const std::optional<TRDN>& subjectDns, bool requireSameIssuer, const std::vector<TString>& groups)
113164
: SubjectDn(dn)
165+
, SubjectDns(subjectDns)
114166
, RequireSameIssuer(requireSameIssuer)
115167
, Groups(groups)
116168
{}
117169

118-
TCertificateAuthorizationParams::TCertificateAuthorizationParams(TDN&& dn, bool requireSameIssuer, std::vector<TString>&& groups)
170+
TCertificateAuthorizationParams::TCertificateAuthorizationParams(TDN&& dn, std::optional<TRDN>&& subjectDns, bool requireSameIssuer, std::vector<TString>&& groups)
119171
: SubjectDn(std::move(dn))
172+
, SubjectDns(std::move(subjectDns))
120173
, RequireSameIssuer(requireSameIssuer)
121174
, Groups(std::move(groups))
122175
{}
@@ -127,59 +180,44 @@ TCertificateAuthorizationParams::TDN& TCertificateAuthorizationParams::TDN::AddR
127180
}
128181

129182
TCertificateAuthorizationParams::operator bool() const {
130-
return SubjectDn;
183+
return SubjectDn || SubjectDns;
131184
}
132185

133-
bool TCertificateAuthorizationParams::CheckSubject(const std::unordered_map<TString, std::vector<TString>>& subjectDescription) const {
134-
bool isDescriptionMatched = false;
135-
for (const auto& rdn: SubjectDn.RDNs) {
136-
isDescriptionMatched = false;
186+
bool TCertificateAuthorizationParams::CheckSubject(const std::unordered_map<TString, std::vector<TString>>& subjectDescription, const std::vector<TString>& subjectDns) const {
187+
for (const TRDN& rdn: SubjectDn.RDNs) {
137188
auto fieldIt = subjectDescription.find(rdn.Attribute);
138189
if (fieldIt == subjectDescription.cend()) {
139-
break;
190+
return false;
140191
}
141192

142193
const auto& attributeValues = fieldIt->second;
143-
bool attributeMatched = false;
144-
for (const auto& attributeValue : attributeValues) {
145-
attributeMatched = false;
146-
for (const auto& value: rdn.Values) {
147-
if (value == attributeValue) {
148-
attributeMatched = true;
149-
break;
150-
}
151-
}
152-
if (!attributeMatched) {
153-
for (const auto& suffix: rdn.Suffixes) {
154-
if (attributeValue.EndsWith(suffix)) {
155-
attributeMatched = true;
156-
break;
157-
}
158-
}
159-
}
160-
if (!attributeMatched) {
194+
if (!rdn.Match(attributeValues)) {
195+
return false;
196+
}
197+
}
198+
199+
if (SubjectDns) {
200+
bool dnsMatched = false;
201+
for (const TString& dns : subjectDns) {
202+
if (SubjectDns->Match(dns)) {
203+
dnsMatched = true;
161204
break;
162205
}
163206
}
164-
if (!attributeMatched) {
165-
isDescriptionMatched = false;
166-
break;
207+
if (!dnsMatched) {
208+
return false;
167209
}
168-
isDescriptionMatched = true;
169210
}
170211

171-
if (isDescriptionMatched) {
172-
return true;
173-
}
174-
return false;
212+
return true;
175213
}
176214

177215
TCertificateAuthorizationParams::TDN::operator bool() const {
178216
return !RDNs.empty();
179217
}
180218

181-
TCertificateAuthorizationParams::TRDN::TRDN(const TString& Attribute)
182-
:Attribute(Attribute)
219+
TCertificateAuthorizationParams::TRDN::TRDN(const TString& attribute)
220+
: Attribute(attribute)
183221
{}
184222

185223
TCertificateAuthorizationParams::TRDN& TCertificateAuthorizationParams::TRDN::AddValue(const TString& val)
@@ -194,4 +232,30 @@ TCertificateAuthorizationParams::TRDN& TCertificateAuthorizationParams::TRDN::Ad
194232
return *this;
195233
}
196234

235+
bool TCertificateAuthorizationParams::TRDN::Match(const TString& value) const
236+
{
237+
for (const auto& v : Values) {
238+
if (value == v) {
239+
return true;
240+
}
241+
}
242+
for (const auto& s : Suffixes) {
243+
if (value.EndsWith(s)) {
244+
return true;
245+
}
246+
}
247+
248+
return false;
249+
}
250+
251+
bool TCertificateAuthorizationParams::TRDN::Match(const std::vector<TString>& values) const
252+
{
253+
for (const auto& value : values) {
254+
if (!Match(value)) {
255+
return false;
256+
}
257+
}
258+
return true;
259+
}
260+
197261
} //namespace NKikimr {

‎ydb/core/security/certificate_check/cert_auth_processor.h

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,11 @@ struct TCertificateAuthorizationParams {
1515
TVector<TString> Values;
1616
TVector<TString> Suffixes;
1717

18-
TRDN(const TString& Attribute);
18+
TRDN(const TString& attribute);
1919
TRDN& AddValue(const TString& val);
2020
TRDN& AddSuffix(const TString& suffix);
21+
bool Match(const std::vector<TString>& values) const;
22+
bool Match(const TString& value) const;
2123
};
2224

2325
struct TDN {
@@ -27,11 +29,11 @@ struct TCertificateAuthorizationParams {
2729
operator bool () const;
2830
};
2931

30-
TCertificateAuthorizationParams(const TDN& dn = TDN(), bool requireSameIssuer = true, const std::vector<TString>& groups = {});
31-
TCertificateAuthorizationParams(TDN&& dn, bool requireSameIssuer = true, std::vector<TString>&& groups = {});
32+
TCertificateAuthorizationParams(const TDN& dn = TDN(), const std::optional<TRDN>& subjectDns = std::nullopt, bool requireSameIssuer = true, const std::vector<TString>& groups = {});
33+
TCertificateAuthorizationParams(TDN&& dn, std::optional<TRDN>&& subjectDns, bool requireSameIssuer = true, std::vector<TString>&& groups = {});
3234

3335
operator bool () const;
34-
bool CheckSubject(const std::unordered_map<TString, std::vector<TString>>& subjectDescription) const;
36+
bool CheckSubject(const std::unordered_map<TString, std::vector<TString>>& subjectDescription, const std::vector<TString>& subjectDns) const;
3537
void SetSubjectDn(const TDN& subjectDn) {
3638
SubjectDn = subjectDn;
3739
}
@@ -42,6 +44,7 @@ struct TCertificateAuthorizationParams {
4244

4345
bool CanCheckNodeByAttributeCN = false;
4446
TDN SubjectDn;
47+
std::optional<TRDN> SubjectDns;
4548
bool RequireSameIssuer = true;
4649
std::vector<TString> Groups;
4750
};
@@ -61,6 +64,7 @@ struct X509CertificateReader {
6164

6265
static X509Ptr ReadCertAsPEM(const TStringBuf& cert);
6366
static TVector<std::pair<TString, TString>> ReadSubjectTerms(const X509Ptr& x509);
67+
static TVector<TString> ReadSubjectDns(const X509Ptr& x509, const std::vector<std::pair<TString, TString>>& subjectTerms);
6468
static TVector<std::pair<TString, TString>> ReadAllSubjectTerms(const X509Ptr& x509);
6569
static TVector<std::pair<TString, TString>> ReadIssuerTerms(const X509Ptr& x509);
6670
static TString GetFingerprint(const X509Ptr& x509);

‎ydb/core/security/certificate_check/cert_auth_utils.cpp

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
namespace NKikimr {
1919

20-
std::vector<TCertificateAuthorizationParams> GetCertificateAuthorizationParams(const NKikimrConfig::TClientCertificateAuthorization &clientCertificateAuth) {
20+
std::vector<TCertificateAuthorizationParams> GetCertificateAuthorizationParams(const NKikimrConfig::TClientCertificateAuthorization& clientCertificateAuth) {
2121
std::vector<TCertificateAuthorizationParams> certAuthParams;
2222
certAuthParams.reserve(clientCertificateAuth.ClientCertificateDefinitionsSize());
2323

@@ -33,9 +33,19 @@ std::vector<TCertificateAuthorizationParams> GetCertificateAuthorizationParams(c
3333
}
3434
dn.AddRDN(std::move(rdn));
3535
}
36-
if (dn) {
36+
std::optional<TCertificateAuthorizationParams::TRDN> subjectDns;
37+
if (const auto& subjectDnsCfg = clientCertificateDefinition.GetSubjectDns(); subjectDnsCfg.ValuesSize() || subjectDnsCfg.SuffixesSize()) {
38+
TCertificateAuthorizationParams::TRDN& dns = subjectDns.emplace(TString());
39+
for (const auto& value: subjectDnsCfg.GetValues()) {
40+
dns.AddValue(value);
41+
}
42+
for (const auto& suffix: subjectDnsCfg.GetSuffixes()) {
43+
dns.AddSuffix(suffix);
44+
}
45+
}
46+
if (dn || subjectDns) {
3747
std::vector<TString> groups(clientCertificateDefinition.GetMemberGroups().cbegin(), clientCertificateDefinition.GetMemberGroups().cend());
38-
certAuthParams.emplace_back(std::move(dn), clientCertificateDefinition.GetRequireSameIssuer(), std::move(groups));
48+
certAuthParams.emplace_back(std::move(dn), std::move(subjectDns), clientCertificateDefinition.GetRequireSameIssuer(), std::move(groups));
3949
}
4050
}
4151

@@ -130,8 +140,8 @@ int FillNameFromProps(X509_NAME* name, const TProps& props) {
130140
return 1;
131141
}
132142

133-
if (!props.Coutry.empty()) {
134-
X509_NAME_add_entry_by_txt(name, SN_countryName, MBSTRING_ASC, (const unsigned char*)props.Coutry.c_str(), -1, -1, 0);
143+
if (!props.Country.empty()) {
144+
X509_NAME_add_entry_by_txt(name, SN_countryName, MBSTRING_ASC, (const unsigned char*)props.Country.c_str(), -1, -1, 0);
135145
}
136146

137147
if (!props.State.empty()) {
@@ -377,7 +387,7 @@ X509REQPtr GenerateRequest(PKeyPtr& pkey, const TProps& props) {
377387
return std::move(request);
378388
}
379389

380-
X509Ptr SingRequest(X509REQPtr& request, X509Ptr& rootCert, PKeyPtr& rootKey, const TProps& props) {
390+
X509Ptr SignRequest(X509REQPtr& request, X509Ptr& rootCert, PKeyPtr& rootKey, const TProps& props) {
381391
auto* pktmp = X509_REQ_get0_pubkey(request.get()); // X509_REQ_get0_pubkey returns the key, that shouldn't freed
382392
CHECK(pktmp, "Error unpacking public key from request.");
383393

@@ -455,7 +465,7 @@ TCertAndKey GenerateSignedCert(const TCertAndKey& rootCA, const TProps& props) {
455465

456466
auto rootCert = ReadCertAsPEM(rootCA.Certificate);
457467
auto rootKey = ReadPrivateKeyAsPEM(rootCA.PrivateKey);
458-
auto cert = SingRequest(request, rootCert, rootKey, props); // NID_authority_key_identifier must see ca
468+
auto cert = SignRequest(request, rootCert, rootKey, props); // NID_authority_key_identifier must see ca
459469

460470
TCertAndKey result;
461471
result.Certificate = WriteAsPEM(cert);
@@ -475,7 +485,7 @@ TProps TProps::AsCA() {
475485
TProps props;
476486

477487
props.SecondsValid = 3*365 * 24 * 60 *60; // 3 years
478-
props.Coutry = "RU";
488+
props.Country = "RU";
479489
props.State = "MSK";
480490
props.Location = "MSK";
481491
props.Organization = "YA";

‎ydb/core/security/certificate_check/cert_auth_utils.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ struct TCertAndKey {
2222

2323
struct TProps {
2424
long SecondsValid = 0;
25-
std::string Coutry; // C
25+
std::string Country; // C
2626
std::string State; // ST
2727
std::string Location; // L
2828
std::string Organization; // O

‎ydb/core/security/certificate_check/cert_check.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ TCertificateChecker::TReadClientSubjectResult TCertificateChecker::ReadSubjectFr
7171
result.Error = { .Message = "Cannot extract subject from client certificate", .Retryable = false };
7272
return result;
7373
}
74+
result.SubjectDns = X509CertificateReader::ReadSubjectDns(pemCertificates.ClientCertX509, result.SubjectDn);
7475
return result;
7576
}
7677

@@ -84,14 +85,14 @@ TString TCertificateChecker::CreateUserSidFromSubjectDn(const std::vector<std::p
8485
return userSid;
8586
}
8687

87-
TEvTicketParser::TError TCertificateChecker::CheckClientSubject(const std::vector<std::pair<TString, TString>>& subjectDn, const TCertificateAuthorizationParams& authParams) const {
88+
TEvTicketParser::TError TCertificateChecker::CheckClientSubject(const TReadClientSubjectResult& subjectInfo, const TCertificateAuthorizationParams& authParams) const {
8889
std::unordered_map<TString, std::vector<TString>> subjectDescription;
89-
for (const auto& [attribute, value] : subjectDn) {
90+
for (const auto& [attribute, value] : subjectInfo.SubjectDn) {
9091
auto& attributeValues = subjectDescription[attribute];
9192
attributeValues.push_back(value);
9293
}
9394

94-
if (!authParams.CheckSubject(subjectDescription)) {
95+
if (!authParams.CheckSubject(subjectDescription, subjectInfo.SubjectDns)) {
9596
return { .Message = "Client certificate failed verification", .Retryable = false };
9697
}
9798
return {};
@@ -128,7 +129,7 @@ TCertificateChecker::TCertificateCheckResult TCertificateChecker::CheckClientCer
128129
continue;
129130
}
130131

131-
auto checkClientSubjectError = CheckClientSubject(readClientSubjectResult.SubjectDn, authParams);
132+
auto checkClientSubjectError = CheckClientSubject(readClientSubjectResult, authParams);
132133
if (!checkClientSubjectError.empty()) {
133134
continue;
134135
}

‎ydb/core/security/certificate_check/cert_check.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ class TCertificateChecker {
2727

2828
struct TReadClientSubjectResult {
2929
std::vector<std::pair<TString, TString>> SubjectDn;
30+
std::vector<TString> SubjectDns; // Subject alternative names, DNS
3031
TEvTicketParser::TError Error;
3132
};
3233

@@ -47,7 +48,7 @@ class TCertificateChecker {
4748
TEvTicketParser::TError CheckIssuers(const TPemCertificates& pemCertificates) const;
4849
TReadClientSubjectResult ReadSubjectFromClientCertificate(const TPemCertificates& pemCertificates) const;
4950
TString CreateUserSidFromSubjectDn(const std::vector<std::pair<TString, TString>>& subjectDn) const;
50-
TEvTicketParser::TError CheckClientSubject(const std::vector<std::pair<TString, TString>>& subjectDn, const TCertificateAuthorizationParams& authParams) const;
51+
TEvTicketParser::TError CheckClientSubject(const TReadClientSubjectResult& subjectInfo, const TCertificateAuthorizationParams& authParams) const;
5152
TCertificateCheckResult DefaultCheckClientCertificate(const TPemCertificates& pemCertificates) const;
5253
TCertificateCheckResult CheckClientCertificate(const TPemCertificates& pemCertificates) const;
5354
TString GetDefaultGroup() const;

0 commit comments

Comments
 (0)