Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[patch] Add insecure cipher suites log #24

Merged
merged 6 commits into from
Jul 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions handler/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package handler

import (
"context"
"crypto/tls"
"fmt"
"io"
"io/ioutil"
Expand Down Expand Up @@ -73,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
Expand All @@ -88,10 +90,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,
}
Expand Down
12 changes: 12 additions & 0 deletions handler/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ limitations under the License.
package handler

import (
"crypto/tls"
"net/http"
"strconv"
"strings"
Expand All @@ -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.
Expand Down Expand Up @@ -73,6 +76,15 @@ 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("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
}
}
}

req2 := cloneRequest(r) // per RoundTripper contract

req2.Header.Set("X-Athenz-Principal", p.Name())
Expand Down
71 changes: 63 additions & 8 deletions handler/transport_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package handler

import (
"crypto/tls"
"errors"
"io"
"net/http"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
28 changes: 24 additions & 4 deletions service/tls.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
41 changes: 41 additions & 0 deletions service/tls_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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",
},
Expand All @@ -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",
Expand Down