Skip to content

Commit

Permalink
Client certificate subject validation: ignore non-required subject fi…
Browse files Browse the repository at this point in the history 
…elds when validating cert, allow different order, ignore empty requirements
  • Loading branch information
@radekg
radekg committed Sep 16, 2020
1 parent ae48d8e commit 8177347
Show file tree
Hide file tree
Showing 2 changed files with 204 additions and 17 deletions.
129 changes: 113 additions & 16 deletions proxy/tls.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"io/ioutil"
"net"
"sort"
"strings"
"time"

Expand Down Expand Up @@ -124,35 +125,131 @@ func newTLSListenerConfig(conf *config.Config) (*tls.Config, error) {

cfg.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
if conf.Proxy.TLS.ClientCert.ValidateSubject {
expected := fmt.Sprintf("s:/CN=%s/C=%v/S=%v/L=%v/O=%v/OU=%v",
conf.Proxy.TLS.ClientCert.Subject.CommonName,
conf.Proxy.TLS.ClientCert.Subject.Country,
conf.Proxy.TLS.ClientCert.Subject.Province,
conf.Proxy.TLS.ClientCert.Subject.Locality,
conf.Proxy.TLS.ClientCert.Subject.Organization,
conf.Proxy.TLS.ClientCert.Subject.OrganizationalUnit)

expectedFields := map[string]string{}
expectedParts := []string{"s:"}
values := []string{}

if conf.Proxy.TLS.ClientCert.Subject.CommonName != "" {
expectedFields["CN"] = conf.Proxy.TLS.ClientCert.Subject.CommonName
expectedParts = append(expectedParts, fmt.Sprintf("%s=%s", "CN", expectedFields["CN"]))
}
values = removeEmptyStrings(conf.Proxy.TLS.ClientCert.Subject.Country)
if len(values) > 0 {
sort.Strings(values)
expectedFields["C"] = fmt.Sprintf("%v", values)
expectedParts = append(expectedParts, fmt.Sprintf("%s=%s", "C", expectedFields["C"]))
}
values = removeEmptyStrings(conf.Proxy.TLS.ClientCert.Subject.Province)
if len(values) > 0 {
sort.Strings(values)
expectedFields["S"] = fmt.Sprintf("%v", values)
expectedParts = append(expectedParts, fmt.Sprintf("%s=%s", "S", expectedFields["S"]))
}
values = removeEmptyStrings(conf.Proxy.TLS.ClientCert.Subject.Locality)
if len(values) > 0 {
sort.Strings(values)
expectedFields["L"] = fmt.Sprintf("%v", values)
expectedParts = append(expectedParts, fmt.Sprintf("%s=%s", "L", expectedFields["L"]))
}
values = removeEmptyStrings(conf.Proxy.TLS.ClientCert.Subject.Organization)
if len(values) > 0 {
sort.Strings(values)
expectedFields["O"] = fmt.Sprintf("%v", values)
expectedParts = append(expectedParts, fmt.Sprintf("%s=%s", "O", expectedFields["O"]))
}
values = removeEmptyStrings(conf.Proxy.TLS.ClientCert.Subject.OrganizationalUnit)
if len(values) > 0 {
sort.Strings(values)
expectedFields["OU"] = fmt.Sprintf("%v", values)
expectedParts = append(expectedParts, fmt.Sprintf("%s=%s", "OU", expectedFields["OU"]))
}

if len(expectedFields) == 0 {
return nil // nothing to validate
}

for _, chain := range verifiedChains {
for _, cert := range chain {
current := fmt.Sprintf("s:/CN=%s/C=%v/S=%v/L=%v/O=%v/OU=%v",
cert.Subject.CommonName,
cert.Subject.Country,
cert.Subject.Province,
cert.Subject.Locality,
cert.Subject.Organization,
cert.Subject.OrganizationalUnit)
if current == expected {

certificateAcceptable := true

for k, v := range expectedFields {
if k == "CN" {
if v != cert.Subject.CommonName {
certificateAcceptable = false
break
}
}
if k == "C" {
currentValues := cert.Subject.Country
sort.Strings(currentValues)
if fmt.Sprintf("%v", currentValues) != v {
certificateAcceptable = false
break
}
}
if k == "S" {
currentValues := cert.Subject.Province
sort.Strings(currentValues)
if fmt.Sprintf("%v", currentValues) != v {
certificateAcceptable = false
break
}
}
if k == "L" {
currentValues := cert.Subject.Locality
sort.Strings(currentValues)
if fmt.Sprintf("%v", currentValues) != v {
certificateAcceptable = false
break
}
}
if k == "O" {
currentValues := cert.Subject.Organization
sort.Strings(currentValues)
if fmt.Sprintf("%v", currentValues) != v {
certificateAcceptable = false
break
}
}
if k == "OU" {
currentValues := cert.Subject.OrganizationalUnit
sort.Strings(currentValues)
if fmt.Sprintf("%v", currentValues) != v {
certificateAcceptable = false
break
}
}
}

if certificateAcceptable {
return nil
}

}
}
return fmt.Errorf("tls: no client certificate presented required subject '%s'", expected)

return fmt.Errorf("tls: no client certificate presented required subject '%s'", strings.Join(expectedParts, "/"))

}
return nil
}

return cfg, nil
}

func removeEmptyStrings(input []string) []string {
output := []string{}
for _, value := range input {
if value == "" {
continue
}
output = append(output, value)
}
return output
}

func getCipherSuites(enabledCipherSuites []string) ([]uint16, error) {
suites := make([]uint16, 0)
for _, suite := range enabledCipherSuites {
Expand Down
92 changes: 91 additions & 1 deletion proxy/tls_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ func TestValidEnabledClientCertSubjectValidate(t *testing.T) {
testSubject := pkix.Name{
CommonName: "integration-test",
Country: []string{"DE"},
Province: []string{"NRW"},
Locality: []string{"test-file"},
Organization: []string{"integration-test"},
OrganizationalUnit: []string{"invalid-OrganizationalUnit"},
Expand All @@ -68,6 +69,7 @@ func TestValidEnabledClientCertSubjectValidate(t *testing.T) {
c.Proxy.TLS.ClientCert.ValidateSubject = true
c.Proxy.TLS.ClientCert.Subject.CommonName = testSubject.CommonName
c.Proxy.TLS.ClientCert.Subject.Country = testSubject.Country
c.Proxy.TLS.ClientCert.Subject.Province = testSubject.Province
c.Proxy.TLS.ClientCert.Subject.Locality = testSubject.Locality
c.Proxy.TLS.ClientCert.Subject.Organization = testSubject.Organization
c.Proxy.TLS.ClientCert.Subject.OrganizationalUnit = testSubject.OrganizationalUnit
Expand All @@ -89,6 +91,7 @@ func TestInvalidEnabledClientCertSubjectValidate(t *testing.T) {
testSubject := pkix.Name{
CommonName: "integration-test",
Country: []string{"DE"},
Province: []string{"NRW"},
Locality: []string{"test-file"},
Organization: []string{"integration-test"},
OrganizationalUnit: []string{"invalid-OrganizationalUnit"},
Expand All @@ -99,6 +102,7 @@ func TestInvalidEnabledClientCertSubjectValidate(t *testing.T) {
c.Proxy.TLS.ClientCert.ValidateSubject = true
c.Proxy.TLS.ClientCert.Subject.CommonName = testSubject.CommonName
c.Proxy.TLS.ClientCert.Subject.Country = testSubject.Country
c.Proxy.TLS.ClientCert.Subject.Province = testSubject.Province
c.Proxy.TLS.ClientCert.Subject.Locality = testSubject.Locality
c.Proxy.TLS.ClientCert.Subject.Organization = testSubject.Organization
c.Proxy.TLS.ClientCert.Subject.OrganizationalUnit = []string{"expected-OrganizationalUnit"}
Expand All @@ -113,7 +117,93 @@ func TestInvalidEnabledClientCertSubjectValidate(t *testing.T) {
_, _, _, err := makeTLSPipe(c, nil)

a.NotNil(err)
a.Contains(err.Error(), "tls: no client certificate presented required subject 's:/CN=integration-test/C=[DE]/S=[]/L=[test-file]/O=[integration-test]/OU=[expected-OrganizationalUnit]'")
a.Contains(err.Error(), "tls: no client certificate presented required subject 's:/CN=integration-test/C=[DE]/S=[NRW]/L=[test-file]/O=[integration-test]/OU=[expected-OrganizationalUnit]'")
}

func TestValidEnabledClientCertSubjectMayContainNotRequiredValues(t *testing.T) {
a := assert.New(t)
testSubject := pkix.Name{
CommonName: "integration-test",
Country: []string{"DE"},
Province: []string{"NRW"},
Locality: []string{"locality-not-validated"},
Organization: []string{"integration-test"},
OrganizationalUnit: []string{"invalid-OrganizationalUnit"},
}
bundle := NewCertsBundleWithSubject(testSubject)
defer bundle.Close()
c := new(config.Config)
c.Proxy.TLS.ClientCert.ValidateSubject = true
c.Proxy.TLS.ClientCert.Subject.CommonName = testSubject.CommonName
c.Proxy.TLS.ClientCert.Subject.Country = testSubject.Country
c.Proxy.TLS.ClientCert.Subject.Province = testSubject.Province
c.Proxy.TLS.ClientCert.Subject.Organization = testSubject.Organization
c.Proxy.TLS.ClientCert.Subject.OrganizationalUnit = testSubject.OrganizationalUnit
c.Proxy.TLS.ListenerCertFile = bundle.ServerCert.Name()
c.Proxy.TLS.ListenerKeyFile = bundle.ServerKey.Name()
c.Proxy.TLS.CAChainCertFile = bundle.CACert.Name()

c.Kafka.TLS.CAChainCertFile = bundle.CACert.Name()
c.Kafka.TLS.ClientCertFile = bundle.ClientCert.Name()
c.Kafka.TLS.ClientKeyFile = bundle.ClientKey.Name()

_, _, _, err := makeTLSPipe(c, nil)

a.Nil(err)
}

func TestValidEnabledClientCertSubjectMayContainValuesInDifferentOrder(t *testing.T) {
a := assert.New(t)
testSubject := pkix.Name{
CommonName: "integration-test",
Country: []string{"DE", "PL"},
Province: []string{"NRW"},
Organization: []string{"integration-test"},
OrganizationalUnit: []string{"invalid-OrganizationalUnit"},
}
bundle := NewCertsBundleWithSubject(testSubject)
defer bundle.Close()
c := new(config.Config)
c.Proxy.TLS.ClientCert.ValidateSubject = true
c.Proxy.TLS.ClientCert.Subject.Country = []string{"PL", "DE"}
c.Proxy.TLS.ListenerCertFile = bundle.ServerCert.Name()
c.Proxy.TLS.ListenerKeyFile = bundle.ServerKey.Name()
c.Proxy.TLS.CAChainCertFile = bundle.CACert.Name()

c.Kafka.TLS.CAChainCertFile = bundle.CACert.Name()
c.Kafka.TLS.ClientCertFile = bundle.ClientCert.Name()
c.Kafka.TLS.ClientKeyFile = bundle.ClientKey.Name()

_, _, _, err := makeTLSPipe(c, nil)

a.Nil(err)
}

func TestValidEnabledClientCertSubjectEemptyValuesAreIgnored(t *testing.T) {
a := assert.New(t)
testSubject := pkix.Name{
CommonName: "integration-test",
Country: []string{"DE", "PL"},
Province: []string{"NRW"},
Organization: []string{"integration-test"},
OrganizationalUnit: []string{"invalid-OrganizationalUnit"},
}
bundle := NewCertsBundleWithSubject(testSubject)
defer bundle.Close()
c := new(config.Config)
c.Proxy.TLS.ClientCert.ValidateSubject = true
c.Proxy.TLS.ClientCert.Subject.Country = []string{"PL", "", "DE"}
c.Proxy.TLS.ListenerCertFile = bundle.ServerCert.Name()
c.Proxy.TLS.ListenerKeyFile = bundle.ServerKey.Name()
c.Proxy.TLS.CAChainCertFile = bundle.CACert.Name()

c.Kafka.TLS.CAChainCertFile = bundle.CACert.Name()
c.Kafka.TLS.ClientCertFile = bundle.ClientCert.Name()
c.Kafka.TLS.ClientKeyFile = bundle.ClientKey.Name()

_, _, _, err := makeTLSPipe(c, nil)

a.Nil(err)
}

func TestTLSUnknownAuthorityNoCAChainCert(t *testing.T) {
Expand Down

0 comments on commit 8177347

Please sign in to comment.