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

Allow TLS cipher suites to be set for the OPA server #6537

Merged
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
53 changes: 53 additions & 0 deletions cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ type runCmdParams struct {
skipBundleVerify bool
skipKnownSchemaCheck bool
excludeVerifyFiles []string
cipherSuites []string
}

func newRunParams() runCmdParams {
Expand Down Expand Up @@ -181,6 +182,17 @@ be expanded in the future. To disable this, use the --skip-known-schema-check fl
The --v1-compatible flag can be used to opt-in to OPA features and behaviors that will be enabled by default in a future OPA v1.0 release.
Current behaviors enabled by this flag include:
- setting OPA's listening address to "localhost:8181" by default.

The --tls-cipher-suites flag can be used to specify the list of enabled TLS 1.0–1.2 cipher suites. Note that TLS 1.3
cipher suites are not configurable. Following are the supported TLS 1.0 - 1.2 cipher suites (IANA):
TLS_RSA_WITH_RC4_128_SHA, TLS_RSA_WITH_3DES_EDE_CBC_SHA, TLS_RSA_WITH_AES_128_CBC_SHA, TLS_RSA_WITH_AES_256_CBC_SHA,
TLS_RSA_WITH_AES_128_CBC_SHA256, TLS_RSA_WITH_AES_128_GCM_SHA256, TLS_RSA_WITH_AES_256_GCM_SHA384, TLS_ECDHE_ECDSA_WITH_RC4_128_SHA,
TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, TLS_ECDHE_RSA_WITH_RC4_128_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, TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256,
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256

See https://godoc.org/crypto/tls#pkg-constants for more information.
`,

Run: func(cmd *cobra.Command, args []string) {
Expand Down Expand Up @@ -221,6 +233,7 @@ Current behaviors enabled by this flag include:
runCommand.Flags().IntVar(&cmdParams.rt.GracefulShutdownPeriod, "shutdown-grace-period", 10, "set the time (in seconds) that the server will wait to gracefully shut down")
runCommand.Flags().IntVar(&cmdParams.rt.ShutdownWaitPeriod, "shutdown-wait-period", 0, "set the time (in seconds) that the server will wait before initiating shutdown")
runCommand.Flags().BoolVar(&cmdParams.skipKnownSchemaCheck, "skip-known-schema-check", false, "disables type checking on known input schemas")
runCommand.Flags().StringSliceVar(&cmdParams.cipherSuites, "tls-cipher-suites", []string{}, "set list of enabled TLS 1.0–1.2 cipher suites (IANA)")
addConfigOverrides(runCommand.Flags(), &cmdParams.rt.ConfigOverrides)
addConfigOverrideFiles(runCommand.Flags(), &cmdParams.rt.ConfigOverrideFiles)
addBundleModeFlag(runCommand.Flags(), &cmdParams.rt.BundleMode, false)
Expand Down Expand Up @@ -332,6 +345,15 @@ func initRuntime(ctx context.Context, params runCmdParams, args []string, addrSe

params.rt.SkipKnownSchemaCheck = params.skipKnownSchemaCheck

if len(params.cipherSuites) > 0 {
cipherSuites, err := verifyCipherSuites(params.cipherSuites)
if err != nil {
return nil, err
}

params.rt.CipherSuites = cipherSuites
}

rt, err := runtime.NewRuntime(ctx, params.rt)
if err != nil {
return nil, err
Expand All @@ -355,6 +377,37 @@ func startRuntime(ctx context.Context, rt *runtime.Runtime, serverMode bool) {
}
}

func verifyCipherSuites(cipherSuites []string) (*[]uint16, error) {
cipherSuitesMap := map[string]*tls.CipherSuite{}

for _, c := range tls.CipherSuites() {
ashutosh-narkar marked this conversation as resolved.
Show resolved Hide resolved
cipherSuitesMap[c.Name] = c
}

for _, c := range tls.InsecureCipherSuites() {
cipherSuitesMap[c.Name] = c
}

cipherSuitesIds := []uint16{}
for _, c := range cipherSuites {
val, ok := cipherSuitesMap[c]
if !ok {
return nil, fmt.Errorf("invalid cipher suite %v", c)
}

// verify no TLS 1.3 cipher suites as they are not configurable
for _, ver := range val.SupportedVersions {
if ver == tls.VersionTLS13 {
return nil, fmt.Errorf("TLS 1.3 cipher suite \"%v\" is not configurable", c)
}
}

cipherSuitesIds = append(cipherSuitesIds, val.ID)
}

return &cipherSuitesIds, nil
}

func historyPath() string {
home := os.Getenv("HOME")
if len(home) == 0 {
Expand Down
48 changes: 48 additions & 0 deletions cmd/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@ package cmd
import (
"bytes"
"context"
"crypto/tls"
"encoding/json"
"fmt"
"path/filepath"
"reflect"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -149,6 +152,51 @@ func TestInitRuntimeVerifyNonBundle(t *testing.T) {
}
}

func TestInitRuntimeCipherSuites(t *testing.T) {
ashutosh-narkar marked this conversation as resolved.
Show resolved Hide resolved
testCases := []struct {
name string
cipherSuites []string
expErr bool
expCipherSuites []uint16
}{
{"no cipher suites", []string{}, false, []uint16{}},
{"secure and insecure cipher suites", []string{"TLS_RSA_WITH_AES_128_CBC_SHA", "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", "TLS_RSA_WITH_RC4_128_SHA"}, false, []uint16{tls.TLS_RSA_WITH_AES_128_CBC_SHA, tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, tls.TLS_RSA_WITH_RC4_128_SHA}},
{"invalid cipher suites", []string{"foo"}, true, []uint16{}},
{"tls 1.3 cipher suite", []string{"TLS_AES_128_GCM_SHA256"}, true, []uint16{}},
{"tls 1.2-1.3 cipher suite", []string{"TLS_RSA_WITH_AES_128_GCM_SHA256", "TLS_AES_128_GCM_SHA256"}, true, []uint16{}},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {

params := newTestRunParams()

if len(tc.cipherSuites) != 0 {
params.cipherSuites = tc.cipherSuites
}

rt, err := initRuntime(context.Background(), params, nil, false)
fmt.Println(err)

if !tc.expErr && err != nil {
t.Fatal("Unexpected error occurred:", err)
} else if tc.expErr && err == nil {
t.Fatal("Expected error but got nil")
} else if err == nil {
if len(tc.expCipherSuites) > 0 {
if !reflect.DeepEqual(*rt.Params.CipherSuites, tc.expCipherSuites) {
t.Fatalf("expected cipher suites %v but got %v", tc.expCipherSuites, *rt.Params.CipherSuites)
}
} else {
if rt.Params.CipherSuites != nil {
t.Fatal("expected no value defined for cipher suites")
}
}
}
})
}
}

func TestInitRuntimeSkipKnownSchemaCheck(t *testing.T) {

fs := map[string]string{
Expand Down
4 changes: 4 additions & 0 deletions runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@ type Params struct {
// This flag allows users to opt-in to the new behavior and helps transition to the future release upon which
// the new behavior will be enabled by default.
V1Compatible bool

// CipherSuites specifies the list of enabled TLS 1.0–1.2 cipher suites
CipherSuites *[]uint16
}

// LoggingConfig stores the configuration for OPA's logging behaviour.
Expand Down Expand Up @@ -550,6 +553,7 @@ func (rt *Runtime) Serve(ctx context.Context) error {
WithRuntime(rt.Manager.Info).
WithMetrics(rt.metrics).
WithMinTLSVersion(rt.Params.MinTLSVersion).
WithCipherSuites(rt.Params.CipherSuites).
WithDistributedTracingOpts(rt.Params.DistributedTracingOpts)

// If decision_logging plugin enabled, check to see if we opted in to the ND builtins cache.
Expand Down
65 changes: 39 additions & 26 deletions server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ type Server struct {
distributedTracingOpts tracing.Options
ndbCacheEnabled bool
unixSocketPerm *string
cipherSuites *[]uint16
}

// Metrics defines the interface that the server requires for recording HTTP
Expand Down Expand Up @@ -400,6 +401,12 @@ func (s *Server) WithNDBCacheEnabled(ndbCacheEnabled bool) *Server {
return s
}

// WithCipherSuites sets the list of enabled TLS 1.0–1.2 cipher suites.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the supported suites accepted by verifyCipherSuites() are compatible with TLS 1.3: TLS_AES_128_GCM_SHA256, TLS_AES_256_GCM_SHA384, TLS_CHACHA20_POLY1305_SHA256. Is this description wrong, or is something else in play here that disqualifies 1.3?
If the former, maybe we should not be so specific in the description.
If the latter, then we should probably make sure the user can't select these suites in the first place.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, It's the latter. See my comment below 👇 .

func (s *Server) WithCipherSuites(cipherSuites *[]uint16) *Server {
s.cipherSuites = cipherSuites
return s
}

// WithUnixSocketPermission sets the permission for the Unix domain socket if used to listen for
// incoming connections. Applies to the sockets the server is listening on including diagnostic API's.
func (s *Server) WithUnixSocketPermission(unixSocketPerm *string) *Server {
Expand Down Expand Up @@ -635,38 +642,44 @@ func (s *Server) getListenerForHTTPSServer(u *url.URL, h http.Handler, t httpLis
return nil, nil, fmt.Errorf("TLS certificate required but not supplied")
}

httpsServer := http.Server{
Addr: u.Host,
Handler: h,
TLSConfig: &tls.Config{
GetCertificate: s.getCertificate,
// GetConfigForClient is used to ensure that a fresh config is provided containing the latest cert pool.
// This is not required, but appears to be how connect time updates config should be done:
// https://github.com/golang/go/issues/16066#issuecomment-250606132
GetConfigForClient: func(info *tls.ClientHelloInfo) (*tls.Config, error) {
s.tlsConfigMtx.Lock()
defer s.tlsConfigMtx.Unlock()

cfg := &tls.Config{
GetCertificate: s.getCertificate,
ClientCAs: s.certPool,
}
tlsConfig := tls.Config{
GetCertificate: s.getCertificate,
// GetConfigForClient is used to ensure that a fresh config is provided containing the latest cert pool.
// This is not required, but appears to be how connect time updates config should be done:
// https://github.com/golang/go/issues/16066#issuecomment-250606132
GetConfigForClient: func(info *tls.ClientHelloInfo) (*tls.Config, error) {
s.tlsConfigMtx.Lock()
defer s.tlsConfigMtx.Unlock()

if s.authentication == AuthenticationTLS {
cfg.ClientAuth = tls.RequireAndVerifyClientCert
}
cfg := &tls.Config{
GetCertificate: s.getCertificate,
ClientCAs: s.certPool,
}

if s.minTLSVersion != 0 {
cfg.MinVersion = s.minTLSVersion
} else {
cfg.MinVersion = defaultMinTLSVersion
}
if s.authentication == AuthenticationTLS {
cfg.ClientAuth = tls.RequireAndVerifyClientCert
}

return cfg, nil
},
if s.minTLSVersion != 0 {
cfg.MinVersion = s.minTLSVersion
} else {
cfg.MinVersion = defaultMinTLSVersion
}

if s.cipherSuites != nil {
cfg.CipherSuites = *s.cipherSuites
ashutosh-narkar marked this conversation as resolved.
Show resolved Hide resolved
}

return cfg, nil
},
}

httpsServer := http.Server{
Addr: u.Host,
Handler: h,
TLSConfig: &tlsConfig,
}

l := newHTTPListener(&httpsServer, t)

httpsLoop := func() error { return l.ListenAndServeTLS("", "") }
Expand Down
Loading