From afa9a34a35cf54cf3af7f556a732af6800ba64f8 Mon Sep 17 00:00:00 2001 From: Kyo Fujisaki Date: Mon, 26 Jun 2023 19:53:47 +0900 Subject: [PATCH 1/6] Add warn log Signed-off-by: Kyo Fujisaki --- handler/handler.go | 10 ++++++---- handler/transport.go | 11 +++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/handler/handler.go b/handler/handler.go index e3a58e4..819ee11 100644 --- a/handler/handler.go +++ b/handler/handler.go @@ -18,6 +18,7 @@ package handler import ( "context" + "crypto/tls" "fmt" "io" "io/ioutil" @@ -88,10 +89,11 @@ func New(cfg config.Proxy, bp httputil.BufferPool, prov service.Authorizationd) }, ModifyResponse: modifyResponse, Transport: &transport{ - prov: prov, - RoundTripper: updateDialContext(transportFromCfg(cfg.Transport), cfg.Transport.DialContext.Timeout), - cfg: cfg, - noAuthPaths: mapPathToAssertion(cfg.NoAuthPaths), + prov: prov, + RoundTripper: updateDialContext(transportFromCfg(cfg.Transport), cfg.Transport.DialContext.Timeout), + cfg: cfg, + noAuthPaths: mapPathToAssertion(cfg.NoAuthPaths), + insecureCipherSuites: tls.InsecureCipherSuites(), }, ErrorHandler: handleError, } diff --git a/handler/transport.go b/handler/transport.go index 62f3a50..70436a4 100644 --- a/handler/transport.go +++ b/handler/transport.go @@ -17,6 +17,7 @@ limitations under the License. package handler import ( + "crypto/tls" "net/http" "strconv" "strings" @@ -36,6 +37,8 @@ type transport struct { prov service.Authorizationd cfg config.Proxy noAuthPaths []*policy.Assertion + // List to check for deprecated cipher suites + insecureCipherSuites []*tls.CipherSuite } // Based on the following. @@ -73,6 +76,14 @@ func (t *transport) RoundTrip(r *http.Request) (*http.Response, error) { return nil, errors.Wrap(err, ErrMsgUnverified) } + if r.TLS != nil { + for _, cipherSuite := range t.insecureCipherSuites { + if cipherSuite.ID == r.TLS.CipherSuite { + glg.Warnf("Deprecated cipher suite is being used. Domain: %s, Role: [%s], Principal: %s, Cipher Suite: %s", p.Domain(), strings.Join(p.Roles(), ","), p.Name(), cipherSuite.Name) + } + } + } + req2 := cloneRequest(r) // per RoundTripper contract req2.Header.Set("X-Athenz-Principal", p.Name()) From 57c291359f0d8da0cb3fd2030e018a3ade185778 Mon Sep 17 00:00:00 2001 From: Kyo Fujisaki Date: Tue, 27 Jun 2023 10:05:14 +0900 Subject: [PATCH 2/6] Add break Signed-off-by: Kyo Fujisaki --- handler/transport.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/handler/transport.go b/handler/transport.go index 70436a4..3540e8e 100644 --- a/handler/transport.go +++ b/handler/transport.go @@ -79,7 +79,8 @@ func (t *transport) RoundTrip(r *http.Request) (*http.Response, error) { if r.TLS != nil { for _, cipherSuite := range t.insecureCipherSuites { if cipherSuite.ID == r.TLS.CipherSuite { - glg.Warnf("Deprecated cipher suite is being used. Domain: %s, Role: [%s], Principal: %s, Cipher Suite: %s", p.Domain(), strings.Join(p.Roles(), ","), p.Name(), cipherSuite.Name) + glg.Warnf("A connection was made with a deprecated cipher suite. Domain: %s, Role: [%s], Principal: %s, Cipher Suite: %s", p.Domain(), strings.Join(p.Roles(), ","), p.Name(), cipherSuite.Name) + break } } } From a4e730359793e5ab3c2bb624572bca558f5376a8 Mon Sep 17 00:00:00 2001 From: Kyo Fujisaki Date: Tue, 27 Jun 2023 14:23:45 +0900 Subject: [PATCH 3/6] Add default cipher suites Signed-off-by: Kyo Fujisaki --- service/tls.go | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/service/tls.go b/service/tls.go index eda095c..3bfdf2b 100644 --- a/service/tls.go +++ b/service/tls.go @@ -251,10 +251,7 @@ func isValidDuration(durationString string) (bool, error) { // cipherSuites returns list of available cipher suites func cipherSuites(dcs []string, eics []string) ([]uint16, error) { - ciphers := make(map[string]uint16) - for _, c := range tls.CipherSuites() { - ciphers[c.Name] = c.ID - } + ciphers := defaultCipherSuitesMap() if len(dcs) != 0 { for _, cipher := range dcs { if _, ok := ciphers[cipher]; !ok { @@ -289,3 +286,26 @@ func cipherSuites(dcs []string, eics []string) ([]uint16, error) { return availableCipherSuites, nil } + +// defaultCipherSuitesMap returns a map of name and id in default cipher suites +func defaultCipherSuitesMap() map[string]uint16 { + var ( + // allowInsecureCipherSuites is a list of cipher suites supported in tls.InsecureCipherSuites() + // Default cipher suites is a list of tls.CipherSuites() and allowInsecureCipherSuites + allowInsecureCipherSuites = map[string]uint16{ + "TLS_RSA_WITH_3DES_EDE_CBC_SHA": tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, + "TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA": tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, + } + ) + + ciphers := make(map[string]uint16) + for _, c := range tls.CipherSuites() { + ciphers[c.Name] = c.ID + } + for _, c := range tls.InsecureCipherSuites() { + if _, ok := allowInsecureCipherSuites[c.Name]; ok { + ciphers[c.Name] = c.ID + } + } + return ciphers +} From 4043b02f79725292c2c084a36c4ed46ee2837d12 Mon Sep 17 00:00:00 2001 From: Kyo Fujisaki Date: Tue, 27 Jun 2023 14:52:27 +0900 Subject: [PATCH 4/6] Fix test for default cipher suites Signed-off-by: Kyo Fujisaki --- service/tls_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/service/tls_test.go b/service/tls_test.go index 750ec49..9dbee60 100644 --- a/service/tls_test.go +++ b/service/tls_test.go @@ -1390,6 +1390,8 @@ func Test_cipherSuites(t *testing.T) { for _, id := range ciphers { cipherSuites = append(cipherSuites, id) } + cipherSuites = append(cipherSuites, tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA) + cipherSuites = append(cipherSuites, tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA) return }(), wantErr: nil, @@ -1409,10 +1411,12 @@ func Test_cipherSuites(t *testing.T) { name: "Check disable cipher suites containing SHA-1", args: args{ dcs: []string{ + "TLS_RSA_WITH_3DES_EDE_CBC_SHA", "TLS_RSA_WITH_AES_128_CBC_SHA", "TLS_RSA_WITH_AES_256_CBC_SHA", "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", + "TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA", "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", }, @@ -1435,6 +1439,43 @@ func Test_cipherSuites(t *testing.T) { }, { name: "Check enable insecure cipher suites", + args: args{ + dcs: []string{ + "TLS_RSA_WITH_AES_128_CBC_SHA", + "TLS_RSA_WITH_AES_256_CBC_SHA", + "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", + "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", + "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", + "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", + }, + eics: []string{ + "TLS_RSA_WITH_RC4_128_SHA", + "TLS_ECDHE_ECDSA_WITH_RC4_128_SHA", + "TLS_ECDHE_RSA_WITH_RC4_128_SHA", + }, + }, + want: []uint16{ + tls.TLS_RSA_WITH_AES_128_GCM_SHA256, + tls.TLS_RSA_WITH_AES_256_GCM_SHA384, + tls.TLS_AES_128_GCM_SHA256, + tls.TLS_AES_256_GCM_SHA384, + tls.TLS_CHACHA20_POLY1305_SHA256, + tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, + tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305, + tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305, + tls.TLS_RSA_WITH_RC4_128_SHA, + tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA, + tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA, + tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, + tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, + }, + wantErr: nil, + }, + { + name: "Check enable allowInsecureCipherSuites", args: args{ dcs: []string{ "TLS_RSA_WITH_AES_128_CBC_SHA", From 2af3f7b45278c301ab665a429244aa5c804ca4fd Mon Sep 17 00:00:00 2001 From: Kyo Fujisaki Date: Tue, 27 Jun 2023 15:48:26 +0900 Subject: [PATCH 5/6] Add test for insecure cipher suite log Signed-off-by: Kyo Fujisaki --- handler/transport_test.go | 71 ++++++++++++++++++++++++++++++++++----- 1 file changed, 63 insertions(+), 8 deletions(-) diff --git a/handler/transport_test.go b/handler/transport_test.go index afcfc53..7fcc362 100644 --- a/handler/transport_test.go +++ b/handler/transport_test.go @@ -1,6 +1,7 @@ package handler import ( + "crypto/tls" "errors" "io" "net/http" @@ -36,10 +37,11 @@ func Test_transport_RoundTrip(t *testing.T) { return a } type fields struct { - RoundTripper http.RoundTripper - prov service.Authorizationd - cfg config.Proxy - noAuthPaths []*policy.Assertion + RoundTripper http.RoundTripper + prov service.Authorizationd + cfg config.Proxy + noAuthPaths []*policy.Assertion + insecureCipherSuites []*tls.CipherSuite } type args struct { r *http.Request @@ -381,14 +383,67 @@ func Test_transport_RoundTrip(t *testing.T) { wantErr: true, wantCloseCount: 1, }, + { + name: "allowed insecure cipher suites success", + fields: fields{ + RoundTripper: &RoundTripperMock{ + RoundTripFunc: func(req *http.Request) (*http.Response, error) { + return &http.Response{ + StatusCode: 200, + }, nil + }, + }, + prov: &service.AuthorizerdMock{ + VerifyFunc: func(r *http.Request, act, res string) (authorizerd.Principal, error) { + return &PrincipalMock{ + NameFunc: func() string { + return "testPrincipal" + }, + RolesFunc: func() []string { + return []string{"testRole1", "testRole2"} + }, + DomainFunc: func() string { + return "testDomain" + }, + IssueTimeFunc: func() int64 { + return 0 + }, + ExpiryTimeFunc: func() int64 { + return 0 + }, + }, nil + }, + }, + cfg: config.Proxy{}, + insecureCipherSuites: tls.InsecureCipherSuites(), + }, + args: args{ + r: func() *http.Request { + r, _ := http.NewRequest("GET", "http://athenz.io", nil) + r.TLS = &tls.ConnectionState{ + CipherSuite: tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, + } + return r + }(), + body: &readCloseCounter{ + ReadErr: errors.New("readCloseCounter.Read not implemented"), + }, + }, + want: &http.Response{ + StatusCode: 200, + }, + wantErr: false, + wantCloseCount: 0, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { tr := &transport{ - RoundTripper: tt.fields.RoundTripper, - prov: tt.fields.prov, - cfg: tt.fields.cfg, - noAuthPaths: tt.fields.noAuthPaths, + RoundTripper: tt.fields.RoundTripper, + prov: tt.fields.prov, + cfg: tt.fields.cfg, + noAuthPaths: tt.fields.noAuthPaths, + insecureCipherSuites: tt.fields.insecureCipherSuites, } if tt.args.body != nil { tt.args.r.Body = tt.args.body From f27ab07f824741ad9f6787faeb6337f7d7ed6aa4 Mon Sep 17 00:00:00 2001 From: Kyo Fujisaki Date: Mon, 3 Jul 2023 17:22:02 +0900 Subject: [PATCH 6/6] Add ip address logging Signed-off-by: Kyo Fujisaki --- handler/handler.go | 1 + handler/transport.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/handler/handler.go b/handler/handler.go index 819ee11..171c6e4 100644 --- a/handler/handler.go +++ b/handler/handler.go @@ -74,6 +74,7 @@ func New(cfg config.Proxy, bp httputil.BufferPool, prov service.Authorizationd) return } req.Header = r.Header + req.RemoteAddr = r.RemoteAddr req.TLS = r.TLS if cfg.PreserveHost { req.Host = r.Host diff --git a/handler/transport.go b/handler/transport.go index 3540e8e..1efe2c1 100644 --- a/handler/transport.go +++ b/handler/transport.go @@ -79,7 +79,7 @@ func (t *transport) RoundTrip(r *http.Request) (*http.Response, error) { if r.TLS != nil { for _, cipherSuite := range t.insecureCipherSuites { if cipherSuite.ID == r.TLS.CipherSuite { - glg.Warnf("A connection was made with a deprecated cipher suite. Domain: %s, Role: [%s], Principal: %s, Cipher Suite: %s", p.Domain(), strings.Join(p.Roles(), ","), p.Name(), cipherSuite.Name) + glg.Warnf("A connection was made with a deprecated cipher suite. Client IP adress: %s, Domain: %s, Role: [%s], Principal: %s, Cipher Suite: %s", r.RemoteAddr, p.Domain(), strings.Join(p.Roles(), ","), p.Name(), cipherSuite.Name) break } }