Skip to content

Commit

Permalink
Merge pull request #65 from Klarrio/fix-certificate-validation
Browse files Browse the repository at this point in the history 
Fix certificate validation
  • Loading branch information
@everesio
everesio committed Sep 19, 2020
2 parents 0db5165 + 3c385ba commit b8d1fee
Show file tree
Hide file tree
Showing 4 changed files with 262 additions and 20 deletions.
31 changes: 31 additions & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,14 @@ See:
--tls-insecure-skip-verify It controls whether a client verifies the server's certificate chain and host name
--tls-same-client-cert-enable Use only when mutual TLS is enabled on proxy and broker. It controls whether a proxy validates if proxy client certificate exactly matches brokers client cert (tls-client-cert-file)

--proxy-listener-tls-client-cert-validate-subject bool Whether to validate client certificate subject (default false)
--proxy-listener-tls-required-client-subject-common-name string Required client certificate subject common name
--proxy-listener-tls-required-client-subject-country stringArray Required client certificate subject country
--proxy-listener-tls-required-client-subject-province stringArray Required client certificate subject province
--proxy-listener-tls-required-client-subject-locality stringArray Required client certificate subject locality
--proxy-listener-tls-required-client-subject-organization stringArray Required client certificate subject organization
--proxy-listener-tls-required-client-subject-organizational-unit stringArray Required client certificate subject organizational unit

### Usage example

kafka-proxy server --bootstrap-server-mapping "192.168.99.100:32400,0.0.0.0:32399"
Expand Down Expand Up @@ -312,6 +320,29 @@ Connect through test HTTP Proxy server using CONNECT method
--forward-proxy http://my-proxy-user:my-proxy-password@localhost:3128
```

### Validating client certificate DN

Sometimes it might be necessary to not only validate that the client certificate is valid but also that the client certificate DN is issued for a concrete use case. This can be achieved using the following set of arguments:

```
--proxy-listener-tls-client-cert-validate-subject bool Whether to validate client certificate subject (default false)
--proxy-listener-tls-required-client-subject-common-name string Required client certificate subject common name
--proxy-listener-tls-required-client-subject-country stringArray Required client certificate subject country
--proxy-listener-tls-required-client-subject-province stringArray Required client certificate subject province
--proxy-listener-tls-required-client-subject-locality stringArray Required client certificate subject locality
--proxy-listener-tls-required-client-subject-organization stringArray Required client certificate subject organization
--proxy-listener-tls-required-client-subject-organizational-unit stringArray Required client certificate subject organizational unit
```

By setting `--proxy-listener-tls-client-cert-validate-subject true`, Kafka Proxy will inspect client certificate DN fields for the expected values set with the `--proxy-listener-tls-required-client-*` arguments. The matches are always exact and used together, fo all non empty values. For example, to allow a valid certificate for `country=DE` and `organization=grepplabs`, configure Kafka Proxy in the following way:

```
kafka-proxy server \
--proxy-listener-tls-client-cert-validate-subject true \
--proxy-listener-tls-required-client-subject-country DE \
--proxy-listener-tls-required-client-subject-organization grepplabs
```

### Kubernetes sidecar container example

```yaml
Expand Down
2 changes: 1 addition & 1 deletion cmd/kafka-proxy/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func initFlags() {
Server.Flags().StringSliceVar(&c.Proxy.TLS.ListenerCurvePreferences, "proxy-listener-curve-preferences", []string{}, "List of curve preferences")

Server.Flags().BoolVar(&c.Proxy.TLS.ClientCert.ValidateSubject, "proxy-listener-tls-client-cert-validate-subject", false, "Whether to validate client certificate subject")
Server.Flags().StringVar(&c.Proxy.TLS.ClientCert.Subject.CommonName, "proxy-listener-tls-client-cert-subject-common-name", "", "Required client certificate subject common name")
Server.Flags().StringVar(&c.Proxy.TLS.ClientCert.Subject.CommonName, "proxy-listener-tls-required-client-subject-common-name", "", "Required client certificate subject common name")
Server.Flags().StringSliceVar(&c.Proxy.TLS.ClientCert.Subject.Country, "proxy-listener-tls-required-client-subject-country", []string{}, "Required client certificate subject country")
Server.Flags().StringSliceVar(&c.Proxy.TLS.ClientCert.Subject.Province, "proxy-listener-tls-required-client-subject-province", []string{}, "Required client certificate subject province")
Server.Flags().StringSliceVar(&c.Proxy.TLS.ClientCert.Subject.Locality, "proxy-listener-tls-required-client-subject-locality", []string{}, "Required client certificate subject locality")
Expand Down
157 changes: 139 additions & 18 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 All @@ -15,6 +16,22 @@ import (
"github.com/pkg/errors"
)

type clientCertSubjectField string

const (
clientCertSubjectCommonName = "CN"
clientCertSubjectCountry = "C"
clientCertSubjectProvince = "S"
clientCertSubjectLocality = "L"
clientCertSubjectOrganization = "O"
clientCertSubjectOrganizationalUnit = "OU"
)

type clientCertExpectedData struct {
fields map[clientCertSubjectField]string
parts []string
}

var (
defaultCurvePreferences = []tls.CurveID{
tls.CurveP256,
Expand Down Expand Up @@ -122,35 +139,139 @@ func newTLSListenerConfig(conf *config.Config) (*tls.Config, error) {
cfg.ClientAuth = tls.RequireAndVerifyClientCert
}

cfg.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
cfg.VerifyPeerCertificate = tlsClientCertVerificationFunc(conf)

return cfg, nil
}

func tlsClientCertVerificationFunc(conf *config.Config) func([][]byte, [][]*x509.Certificate) error {
expectedData := getClientCertExpectedData(conf)
return 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)

if len(expectedData.fields) == 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 expectedData.fields {
switch k {
case clientCertSubjectCommonName:
if v != cert.Subject.CommonName {
certificateAcceptable = false
break
}
case clientCertSubjectCountry:
currentValues := cert.Subject.Country
sort.Strings(currentValues)
if fmt.Sprintf("%v", currentValues) != v {
certificateAcceptable = false
break
}
case clientCertSubjectProvince:
currentValues := cert.Subject.Province
sort.Strings(currentValues)
if fmt.Sprintf("%v", currentValues) != v {
certificateAcceptable = false
break
}
case clientCertSubjectLocality:
currentValues := cert.Subject.Locality
sort.Strings(currentValues)
if fmt.Sprintf("%v", currentValues) != v {
certificateAcceptable = false
break
}
case clientCertSubjectOrganization:
currentValues := cert.Subject.Organization
sort.Strings(currentValues)
if fmt.Sprintf("%v", currentValues) != v {
certificateAcceptable = false
break
}
case clientCertSubjectOrganizationalUnit:
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(expectedData.parts, "/"))

}
return nil
}
}

func getClientCertExpectedData(conf *config.Config) *clientCertExpectedData {

expectedFields := map[clientCertSubjectField]string{}
expectedParts := []string{"s:"} // these are calculated here because the order is relevant to us
values := []string{}

if conf.Proxy.TLS.ClientCert.Subject.CommonName != "" {
expectedFields[clientCertSubjectCommonName] = conf.Proxy.TLS.ClientCert.Subject.CommonName
expectedParts = append(expectedParts, fmt.Sprintf("%s=%s", clientCertSubjectCommonName, expectedFields[clientCertSubjectCommonName]))
}
values = removeEmptyStrings(conf.Proxy.TLS.ClientCert.Subject.Country)
if len(values) > 0 {
sort.Strings(values)
expectedFields[clientCertSubjectCountry] = fmt.Sprintf("%v", values)
expectedParts = append(expectedParts, fmt.Sprintf("%s=%s", clientCertSubjectCountry, expectedFields[clientCertSubjectCountry]))
}
values = removeEmptyStrings(conf.Proxy.TLS.ClientCert.Subject.Province)
if len(values) > 0 {
sort.Strings(values)
expectedFields[clientCertSubjectProvince] = fmt.Sprintf("%v", values)
expectedParts = append(expectedParts, fmt.Sprintf("%s=%s", clientCertSubjectProvince, expectedFields[clientCertSubjectProvince]))
}
values = removeEmptyStrings(conf.Proxy.TLS.ClientCert.Subject.Locality)
if len(values) > 0 {
sort.Strings(values)
expectedFields[clientCertSubjectLocality] = fmt.Sprintf("%v", values)
expectedParts = append(expectedParts, fmt.Sprintf("%s=%s", clientCertSubjectLocality, expectedFields[clientCertSubjectLocality]))
}
values = removeEmptyStrings(conf.Proxy.TLS.ClientCert.Subject.Organization)
if len(values) > 0 {
sort.Strings(values)
expectedFields[clientCertSubjectOrganization] = fmt.Sprintf("%v", values)
expectedParts = append(expectedParts, fmt.Sprintf("%s=%s", clientCertSubjectOrganization, expectedFields[clientCertSubjectOrganization]))
}
values = removeEmptyStrings(conf.Proxy.TLS.ClientCert.Subject.OrganizationalUnit)
if len(values) > 0 {
sort.Strings(values)
expectedFields[clientCertSubjectOrganizationalUnit] = fmt.Sprintf("%v", values)
expectedParts = append(expectedParts, fmt.Sprintf("%s=%s", clientCertSubjectOrganizationalUnit, expectedFields[clientCertSubjectOrganizationalUnit]))
}
return &clientCertExpectedData{
parts: expectedParts,
fields: expectedFields,
}
}

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) {
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 b8d1fee

Please sign in to comment.