From 8e47bdedc6c3a6972e51144d06420819ad072837 Mon Sep 17 00:00:00 2001 From: stffabi Date: Wed, 26 Jun 2019 11:32:04 +0200 Subject: [PATCH] Clear TLS client headers if TLSMutualAuth is optional --- middlewares/tlsClientHeaders.go | 25 +- middlewares/tlsClientHeaders_test.go | 343 +++++++++++---------------- server/server_middlewares.go | 2 +- 3 files changed, 146 insertions(+), 224 deletions(-) diff --git a/middlewares/tlsClientHeaders.go b/middlewares/tlsClientHeaders.go index 0e5354b27..b2a2fd53d 100644 --- a/middlewares/tlsClientHeaders.go +++ b/middlewares/tlsClientHeaders.go @@ -44,7 +44,7 @@ type DistinguishedNameOptions struct { // TLSClientHeaders is a middleware that helps setup a few tls info features. type TLSClientHeaders struct { - Infos *TLSClientCertificateInfos // pass selected informations from the client certificate + Infos *TLSClientCertificateInfos // pass selected information from the client certificate PEM bool // pass the sanitized pem to the backend in a specific header } @@ -79,23 +79,14 @@ func newTLSClientInfos(infos *types.TLSClientCertificateInfos) *TLSClientCertifi } // NewTLSClientHeaders constructs a new TLSClientHeaders instance from supplied frontend header struct. -func NewTLSClientHeaders(frontend *types.Frontend) *TLSClientHeaders { - if frontend == nil { +func NewTLSClientHeaders(passTLSClientCert *types.TLSClientHeaders) *TLSClientHeaders { + if passTLSClientCert == nil { return nil } - var addPEM bool - var infos *TLSClientCertificateInfos - - if frontend.PassTLSClientCert != nil { - conf := frontend.PassTLSClientCert - addPEM = conf.PEM - infos = newTLSClientInfos(conf.Infos) - } - return &TLSClientHeaders{ - Infos: infos, - PEM: addPEM, + Infos: newTLSClientInfos(passTLSClientCert.Infos), + PEM: passTLSClientCert.PEM, } } @@ -221,7 +212,7 @@ func writePart(content *strings.Builder, entry string, prefix string) { } } -// getXForwardedTLSClientCertInfo Build a string with the wanted client certificates informations +// getXForwardedTLSClientCertInfo Build a string with the wanted client certificates information // like Subject="DC=%s,C=%s,ST=%s,L=%s,O=%s,CN=%s",NB=%d,NA=%d,SAN=%s; func (s *TLSClientHeaders) getXForwardedTLSClientCertInfo(certs []*x509.Certificate) string { var headerValues []string @@ -268,8 +259,9 @@ func (s *TLSClientHeaders) getXForwardedTLSClientCertInfo(certs []*x509.Certific return strings.Join(headerValues, ";") } -// ModifyRequestHeaders set the wanted headers with the certificates informations +// ModifyRequestHeaders set the wanted headers with the certificates information func (s *TLSClientHeaders) ModifyRequestHeaders(r *http.Request) { + r.Header.Del(xForwardedTLSClientCert) if s.PEM { if r.TLS != nil && len(r.TLS.PeerCertificates) > 0 { r.Header.Set(xForwardedTLSClientCert, getXForwardedTLSClientCert(r.TLS.PeerCertificates)) @@ -278,6 +270,7 @@ func (s *TLSClientHeaders) ModifyRequestHeaders(r *http.Request) { } } + r.Header.Del(xForwardedTLSClientCertInfos) if s.Infos != nil { if r.TLS != nil && len(r.TLS.PeerCertificates) > 0 { headerContent := s.getXForwardedTLSClientCertInfo(r.TLS.PeerCertificates) diff --git a/middlewares/tlsClientHeaders_test.go b/middlewares/tlsClientHeaders_test.go index 5d6ef3255..48124ddc4 100644 --- a/middlewares/tlsClientHeaders_test.go +++ b/middlewares/tlsClientHeaders_test.go @@ -240,26 +240,6 @@ mxcl71pV8i3NDU3kgVi2440JYpoMveTlXPCV2svHNCw0X238YHsSW4b93yGJO0gI ML9n/4zmm1PMhzZHcEA72ZAq0tKCxpz10djg5v2qL5V+Oaz8TtTOZbPsxpiKMQ== -----END CERTIFICATE----- ` - - minimalCert = `-----BEGIN CERTIFICATE----- -MIIDGTCCAgECCQCqLd75YLi2kDANBgkqhkiG9w0BAQsFADBYMQswCQYDVQQGEwJG -UjETMBEGA1UECAwKU29tZS1TdGF0ZTERMA8GA1UEBwwIVG91bG91c2UxITAfBgNV -BAoMGEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDAeFw0xODA3MTgwODI4MTZaFw0x -ODA4MTcwODI4MTZaMEUxCzAJBgNVBAYTAkZSMRMwEQYDVQQIDApTb21lLVN0YXRl -MSEwHwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwggEiMA0GCSqGSIb3 -DQEBAQUAA4IBDwAwggEKAoIBAQC/+frDMMTLQyXG34F68BPhQq0kzK4LIq9Y0/gl -FjySZNn1C0QDWA1ubVCAcA6yY204I9cxcQDPNrhC7JlS5QA8Y5rhIBrqQlzZizAi -Rj3NTrRjtGUtOScnHuJaWjLy03DWD+aMwb7q718xt5SEABmmUvLwQK+EjW2MeDwj -y8/UEIpvrRDmdhGaqv7IFpIDkcIF7FowJ/hwDvx3PMc+z/JWK0ovzpvgbx69AVbw -ZxCimeha65rOqVi+lEetD26le+WnOdYsdJ2IkmpPNTXGdfb15xuAc+gFXfMCh7Iw -3Ynl6dZtZM/Ok2kiA7/OsmVnRKkWrtBfGYkI9HcNGb3zrk6nAgMBAAEwDQYJKoZI -hvcNAQELBQADggEBAC/R+Yvhh1VUhcbK49olWsk/JKqfS3VIDQYZg1Eo+JCPbwgS -I1BSYVfMcGzuJTX6ua3m/AHzGF3Tap4GhF4tX12jeIx4R4utnjj7/YKkTvuEM2f4 -xT56YqI7zalGScIB0iMeyNz1QcimRl+M/49au8ow9hNX8C2tcA2cwd/9OIj/6T8q -SBRHc6ojvbqZSJCO0jziGDT1L3D+EDgTjED4nd77v/NRdP+egb0q3P0s4dnQ/5AV -aQlQADUn61j3ScbGJ4NSeZFFvsl38jeRi/MEzp0bGgNBcPj6JHi7qbbauZcZfQ05 -jECvgAY7Nfd9mZ1KtyNaW31is+kag7NsvjxU/kM= ------END CERTIFICATE-----` ) func getCleanCertContents(certContents []string) string { @@ -303,10 +283,6 @@ func buildTLSWith(certContents []string) *tls.ConnectionState { return &tls.ConnectionState{PeerCertificates: peerCertificates} } -var myPassTLSClientCustomHandler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Write([]byte("bar")) -}) - func getExpectedSanitized(s string) string { return url.QueryEscape(strings.Replace(s, "\n", "", -1)) } @@ -360,20 +336,13 @@ WqeUSNGYV//RunTeuRDAf5OxehERb1srzBXhRZ3cZdzXbgR/`), } -func TestTlsClientheadersWithPEM(t *testing.T) { +func TestTlsClientHeadersWithPEM(t *testing.T) { testCases := []struct { desc string certContents []string // set the request TLS attribute if defined tlsClientCertHeaders *types.TLSClientHeaders expectedHeader string }{ - { - desc: "No TLS, no option", - }, - { - desc: "TLS, no option", - certContents: []string{minimalCheeseCrt}, - }, { desc: "No TLS, with pem option true", tlsClientCertHeaders: &types.TLSClientHeaders{PEM: true}, @@ -399,20 +368,24 @@ func TestTlsClientheadersWithPEM(t *testing.T) { } for _, test := range testCases { - tlsClientHeaders := NewTLSClientHeaders(&types.Frontend{PassTLSClientCert: test.tlsClientCertHeaders}) - - res := httptest.NewRecorder() - req := testhelpers.MustNewRequest(http.MethodGet, "http://example.com/foo", nil) - - if test.certContents != nil && len(test.certContents) > 0 { - req.TLS = buildTLSWith(test.certContents) - } - - tlsClientHeaders.ServeHTTP(res, req, myPassTLSClientCustomHandler) - - test := test t.Run(test.desc, func(t *testing.T) { - t.Parallel() + + tlsClientHeaders := NewTLSClientHeaders(test.tlsClientCertHeaders) + + res := httptest.NewRecorder() + req := testhelpers.MustNewRequest(http.MethodGet, "http://example.com/foo", nil) + + if test.tlsClientCertHeaders != nil { + req.Header.Set(xForwardedTLSClientCert, "Unsanitized HEADER") + } + + if test.certContents != nil && len(test.certContents) > 0 { + req.TLS = buildTLSWith(test.certContents) + } + + tlsClientHeaders.ServeHTTP(res, req, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("bar")) + })) require.Equal(t, http.StatusOK, res.Code, "Http Status should be OK") require.Equal(t, "bar", res.Body.String(), "Should be the expected body") @@ -477,7 +450,7 @@ func TestGetSans(t *testing.T) { } -func TestTlsClientheadersWithCertInfos(t *testing.T) { +func TestTlsClientHeadersWithCertInfos(t *testing.T) { minimalCheeseCertAllInfos := `Subject="C=FR,ST=Some-State,O=Cheese",Issuer="DC=org,DC=cheese,C=FR,C=US,ST=Signing State,ST=Signing State 2,L=TOULOUSE,L=LYON,O=Cheese,O=Cheese 2,CN=Simple Signing CA 2",NB=1544094636,NA=1632568236,SAN=` completeCertAllInfos := `Subject="DC=org,DC=cheese,C=FR,C=US,ST=Cheese org state,ST=Cheese com state,L=TOULOUSE,L=LYON,O=Cheese,O=Cheese 2,CN=*.cheese.com",Issuer="DC=org,DC=cheese,C=FR,C=US,ST=Signing State,ST=Signing State 2,L=TOULOUSE,L=LYON,O=Cheese,O=Cheese 2,CN=Simple Signing CA 2",NB=1544094616,NA=1607166616,SAN=*.cheese.org,*.cheese.net,*.cheese.com,test@cheese.org,test@cheese.net,10.0.1.0,10.0.1.2` @@ -487,13 +460,6 @@ func TestTlsClientheadersWithCertInfos(t *testing.T) { tlsClientCertHeaders *types.TLSClientHeaders expectedHeader string }{ - { - desc: "No TLS, no option", - }, - { - desc: "TLS, no option", - certContents: []string{minimalCert}, - }, { desc: "No TLS, with pem option true", tlsClientCertHeaders: &types.TLSClientHeaders{ @@ -627,20 +593,24 @@ func TestTlsClientheadersWithCertInfos(t *testing.T) { }, } for _, test := range testCases { - tlsClientHeaders := NewTLSClientHeaders(&types.Frontend{PassTLSClientCert: test.tlsClientCertHeaders}) - - res := httptest.NewRecorder() - req := testhelpers.MustNewRequest(http.MethodGet, "http://example.com/foo", nil) - - if test.certContents != nil && len(test.certContents) > 0 { - req.TLS = buildTLSWith(test.certContents) - } - - tlsClientHeaders.ServeHTTP(res, req, myPassTLSClientCustomHandler) - - test := test t.Run(test.desc, func(t *testing.T) { - t.Parallel() + + tlsClientHeaders := NewTLSClientHeaders(test.tlsClientCertHeaders) + + res := httptest.NewRecorder() + req := testhelpers.MustNewRequest(http.MethodGet, "http://example.com/foo", nil) + + if test.tlsClientCertHeaders != nil { + req.Header.Set(xForwardedTLSClientCertInfos, "Unsanitized HEADER") + } + + if test.certContents != nil && len(test.certContents) > 0 { + req.TLS = buildTLSWith(test.certContents) + } + + tlsClientHeaders.ServeHTTP(res, req, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("bar")) + })) require.Equal(t, http.StatusOK, res.Code, "Http Status should be OK") require.Equal(t, "bar", res.Body.String(), "Should be the expected body") @@ -664,45 +634,31 @@ func TestTlsClientheadersWithCertInfos(t *testing.T) { func TestNewTLSClientHeadersFromStruct(t *testing.T) { testCases := []struct { - desc string - frontend *types.Frontend - expected *TLSClientHeaders + desc string + tlsClientHeaders *types.TLSClientHeaders + expected *TLSClientHeaders }{ { - desc: "Without frontend", - }, - { - desc: "frontend without the option", - frontend: &types.Frontend{}, - expected: &TLSClientHeaders{}, - }, - { - desc: "frontend with the pem set false", - frontend: &types.Frontend{ - PassTLSClientCert: &types.TLSClientHeaders{ - PEM: false, - }, + desc: "TLS client headers with the pem set false", + tlsClientHeaders: &types.TLSClientHeaders{ + PEM: false, }, expected: &TLSClientHeaders{PEM: false}, }, { - desc: "frontend with the pem set true", - frontend: &types.Frontend{ - PassTLSClientCert: &types.TLSClientHeaders{ - PEM: true, - }, + desc: "TLS client headers with the pem set true", + tlsClientHeaders: &types.TLSClientHeaders{ + PEM: true, }, expected: &TLSClientHeaders{PEM: true}, }, { - desc: "frontend with the Infos with no flag", - frontend: &types.Frontend{ - PassTLSClientCert: &types.TLSClientHeaders{ - Infos: &types.TLSClientCertificateInfos{ - NotAfter: false, - NotBefore: false, - Sans: false, - }, + desc: "TLS client headers with the Infos with no flag", + tlsClientHeaders: &types.TLSClientHeaders{ + Infos: &types.TLSClientCertificateInfos{ + NotAfter: false, + NotBefore: false, + Sans: false, }, }, expected: &TLSClientHeaders{ @@ -711,14 +667,12 @@ func TestNewTLSClientHeadersFromStruct(t *testing.T) { }, }, { - desc: "frontend with the Infos basic", - frontend: &types.Frontend{ - PassTLSClientCert: &types.TLSClientHeaders{ - Infos: &types.TLSClientCertificateInfos{ - NotAfter: true, - NotBefore: true, - Sans: true, - }, + desc: "TLS client headers with the Infos basic", + tlsClientHeaders: &types.TLSClientHeaders{ + Infos: &types.TLSClientCertificateInfos{ + NotAfter: true, + NotBefore: true, + Sans: true, }, }, expected: &TLSClientHeaders{ @@ -731,12 +685,10 @@ func TestNewTLSClientHeadersFromStruct(t *testing.T) { }, }, { - desc: "frontend with the Infos NotAfter", - frontend: &types.Frontend{ - PassTLSClientCert: &types.TLSClientHeaders{ - Infos: &types.TLSClientCertificateInfos{ - NotAfter: true, - }, + desc: "TLS client headers with the Infos NotAfter", + tlsClientHeaders: &types.TLSClientHeaders{ + Infos: &types.TLSClientCertificateInfos{ + NotAfter: true, }, }, expected: &TLSClientHeaders{ @@ -747,12 +699,10 @@ func TestNewTLSClientHeadersFromStruct(t *testing.T) { }, }, { - desc: "frontend with the Infos NotBefore", - frontend: &types.Frontend{ - PassTLSClientCert: &types.TLSClientHeaders{ - Infos: &types.TLSClientCertificateInfos{ - NotBefore: true, - }, + desc: "TLS client headers with the Infos NotBefore", + tlsClientHeaders: &types.TLSClientHeaders{ + Infos: &types.TLSClientCertificateInfos{ + NotBefore: true, }, }, expected: &TLSClientHeaders{ @@ -763,12 +713,10 @@ func TestNewTLSClientHeadersFromStruct(t *testing.T) { }, }, { - desc: "frontend with the Infos Sans", - frontend: &types.Frontend{ - PassTLSClientCert: &types.TLSClientHeaders{ - Infos: &types.TLSClientCertificateInfos{ - Sans: true, - }, + desc: "TLS client headers with the Infos Sans", + tlsClientHeaders: &types.TLSClientHeaders{ + Infos: &types.TLSClientCertificateInfos{ + Sans: true, }, }, expected: &TLSClientHeaders{ @@ -779,13 +727,11 @@ func TestNewTLSClientHeadersFromStruct(t *testing.T) { }, }, { - desc: "frontend with the Infos Subject Organization", - frontend: &types.Frontend{ - PassTLSClientCert: &types.TLSClientHeaders{ - Infos: &types.TLSClientCertificateInfos{ - Subject: &types.TLSCLientCertificateDNInfos{ - Organization: true, - }, + desc: "TLS client headers with the Infos Subject Organization", + tlsClientHeaders: &types.TLSClientHeaders{ + Infos: &types.TLSClientCertificateInfos{ + Subject: &types.TLSCLientCertificateDNInfos{ + Organization: true, }, }, }, @@ -799,13 +745,11 @@ func TestNewTLSClientHeadersFromStruct(t *testing.T) { }, }, { - desc: "frontend with the Infos Subject Country", - frontend: &types.Frontend{ - PassTLSClientCert: &types.TLSClientHeaders{ - Infos: &types.TLSClientCertificateInfos{ - Subject: &types.TLSCLientCertificateDNInfos{ - Country: true, - }, + desc: "TLS client headers with the Infos Subject Country", + tlsClientHeaders: &types.TLSClientHeaders{ + Infos: &types.TLSClientCertificateInfos{ + Subject: &types.TLSCLientCertificateDNInfos{ + Country: true, }, }, }, @@ -819,13 +763,11 @@ func TestNewTLSClientHeadersFromStruct(t *testing.T) { }, }, { - desc: "frontend with the Infos Subject SerialNumber", - frontend: &types.Frontend{ - PassTLSClientCert: &types.TLSClientHeaders{ - Infos: &types.TLSClientCertificateInfos{ - Subject: &types.TLSCLientCertificateDNInfos{ - SerialNumber: true, - }, + desc: "TLS client headers with the Infos Subject SerialNumber", + tlsClientHeaders: &types.TLSClientHeaders{ + Infos: &types.TLSClientCertificateInfos{ + Subject: &types.TLSCLientCertificateDNInfos{ + SerialNumber: true, }, }, }, @@ -839,13 +781,11 @@ func TestNewTLSClientHeadersFromStruct(t *testing.T) { }, }, { - desc: "frontend with the Infos Subject Province", - frontend: &types.Frontend{ - PassTLSClientCert: &types.TLSClientHeaders{ - Infos: &types.TLSClientCertificateInfos{ - Subject: &types.TLSCLientCertificateDNInfos{ - Province: true, - }, + desc: "TLS client headers with the Infos Subject Province", + tlsClientHeaders: &types.TLSClientHeaders{ + Infos: &types.TLSClientCertificateInfos{ + Subject: &types.TLSCLientCertificateDNInfos{ + Province: true, }, }, }, @@ -859,13 +799,11 @@ func TestNewTLSClientHeadersFromStruct(t *testing.T) { }, }, { - desc: "frontend with the Infos Subject Locality", - frontend: &types.Frontend{ - PassTLSClientCert: &types.TLSClientHeaders{ - Infos: &types.TLSClientCertificateInfos{ - Subject: &types.TLSCLientCertificateDNInfos{ - Locality: true, - }, + desc: "TLS client headers with the Infos Subject Locality", + tlsClientHeaders: &types.TLSClientHeaders{ + Infos: &types.TLSClientCertificateInfos{ + Subject: &types.TLSCLientCertificateDNInfos{ + Locality: true, }, }, }, @@ -879,13 +817,11 @@ func TestNewTLSClientHeadersFromStruct(t *testing.T) { }, }, { - desc: "frontend with the Infos Subject CommonName", - frontend: &types.Frontend{ - PassTLSClientCert: &types.TLSClientHeaders{ - Infos: &types.TLSClientCertificateInfos{ - Subject: &types.TLSCLientCertificateDNInfos{ - CommonName: true, - }, + desc: "TLS client headers with the Infos Subject CommonName", + tlsClientHeaders: &types.TLSClientHeaders{ + Infos: &types.TLSClientCertificateInfos{ + Subject: &types.TLSCLientCertificateDNInfos{ + CommonName: true, }, }, }, @@ -899,19 +835,17 @@ func TestNewTLSClientHeadersFromStruct(t *testing.T) { }, }, { - desc: "frontend with the Infos Issuer", - frontend: &types.Frontend{ - PassTLSClientCert: &types.TLSClientHeaders{ - Infos: &types.TLSClientCertificateInfos{ - Issuer: &types.TLSCLientCertificateDNInfos{ - CommonName: true, - Country: true, - DomainComponent: true, - Locality: true, - Organization: true, - SerialNumber: true, - Province: true, - }, + desc: "TLS client headers with the Infos Issuer", + tlsClientHeaders: &types.TLSClientHeaders{ + Infos: &types.TLSClientCertificateInfos{ + Issuer: &types.TLSCLientCertificateDNInfos{ + CommonName: true, + Country: true, + DomainComponent: true, + Locality: true, + Organization: true, + SerialNumber: true, + Province: true, }, }, }, @@ -931,12 +865,10 @@ func TestNewTLSClientHeadersFromStruct(t *testing.T) { }, }, { - desc: "frontend with the Sans Infos", - frontend: &types.Frontend{ - PassTLSClientCert: &types.TLSClientHeaders{ - Infos: &types.TLSClientCertificateInfos{ - Sans: true, - }, + desc: "TLS client headers with the Sans Infos", + tlsClientHeaders: &types.TLSClientHeaders{ + Infos: &types.TLSClientCertificateInfos{ + Sans: true, }, }, expected: &TLSClientHeaders{ @@ -947,30 +879,28 @@ func TestNewTLSClientHeadersFromStruct(t *testing.T) { }, }, { - desc: "frontend with the Infos all", - frontend: &types.Frontend{ - PassTLSClientCert: &types.TLSClientHeaders{ - Infos: &types.TLSClientCertificateInfos{ - NotAfter: true, - NotBefore: true, - Subject: &types.TLSCLientCertificateDNInfos{ - CommonName: true, - Country: true, - Locality: true, - Organization: true, - Province: true, - SerialNumber: true, - }, - Issuer: &types.TLSCLientCertificateDNInfos{ - Country: true, - DomainComponent: true, - Locality: true, - Organization: true, - SerialNumber: true, - Province: true, - }, - Sans: true, + desc: "TLS client headers with the Infos all", + tlsClientHeaders: &types.TLSClientHeaders{ + Infos: &types.TLSClientCertificateInfos{ + NotAfter: true, + NotBefore: true, + Subject: &types.TLSCLientCertificateDNInfos{ + CommonName: true, + Country: true, + Locality: true, + Organization: true, + Province: true, + SerialNumber: true, }, + Issuer: &types.TLSCLientCertificateDNInfos{ + Country: true, + DomainComponent: true, + Locality: true, + Organization: true, + SerialNumber: true, + Province: true, + }, + Sans: true, }, }, expected: &TLSClientHeaders{ @@ -1004,8 +934,7 @@ func TestNewTLSClientHeadersFromStruct(t *testing.T) { t.Run(test.desc, func(t *testing.T) { t.Parallel() - require.Equal(t, test.expected, NewTLSClientHeaders(test.frontend)) + require.Equal(t, test.expected, NewTLSClientHeaders(test.tlsClientHeaders)) }) } - } diff --git a/server/server_middlewares.go b/server/server_middlewares.go index 5c4672cae..931fcd77a 100644 --- a/server/server_middlewares.go +++ b/server/server_middlewares.go @@ -113,7 +113,7 @@ func (s *Server) buildMiddlewares(frontendName string, frontend *types.Frontend, } // TLSClientHeaders - tlsClientHeadersMiddleware := middlewares.NewTLSClientHeaders(frontend) + tlsClientHeadersMiddleware := middlewares.NewTLSClientHeaders(frontend.PassTLSClientCert) if tlsClientHeadersMiddleware != nil { log.Debugf("Adding TLSClientHeaders middleware for frontend %s", frontendName)