Added ability to specify allowed TLS cipher suites.

openstandia · Sep 1, 2022 · ebacc2d · ebacc2d
1 parent a1ff878
commit ebacc2d
Show file tree

Hide file tree

Showing 9 changed files with 148 additions and 11 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,7 @@ N/A
 - [#1669](https://github.com/oauth2-proxy/oauth2-proxy/pull/1699) Fix method deprecated error in lint (@t-katsumura)
 
 - [#1709](https://github.com/oauth2-proxy/oauth2-proxy/pull/1709) Show an alert message when basic auth credentials are invalid (@aiciobanu)
+- [#1723](https://github.com/oauth2-proxy/oauth2-proxy/pull/1723) Added ability to specify allowed TLS cipher suites. (@crbednarz)
 
 - [#1720](https://github.com/oauth2-proxy/oauth2-proxy/pull/1720) Extract roles from authToken, to allow using allowed roles with Keycloak.
 

diff --git a/docs/docs/configuration/alpha_config.md b/docs/docs/configuration/alpha_config.md
@@ -478,6 +478,7 @@ as well as an optional minimal TLS version that is acceptable.
 | `Key` | _[SecretSource](#secretsource)_ | Key is the TLS key data to use.<br/>Typically this will come from a file. |
 | `Cert` | _[SecretSource](#secretsource)_ | Cert is the TLS certificate data to use.<br/>Typically this will come from a file. |
 | `MinVersion` | _string_ | MinVersion is the minimal TLS version that is acceptable.<br/>E.g. Set to "TLS1.3" to select TLS version 1.3 |
+| `CipherSuites` | _[]string_ | CipherSuites is a list of TLS cipher suites that are allowed.<br/>E.g.:<br/>- TLS_RSA_WITH_RC4_128_SHA<br/>- TLS_RSA_WITH_AES_256_GCM_SHA384<br/>If not specified, the default Go safe cipher list is used.<br/>List of valid cipher suites can be found in the [crypto/tls documentation](https://pkg.go.dev/crypto/tls#pkg-constants). |
 
 ### URLParameterRule
 

diff --git a/docs/docs/configuration/overview.md b/docs/docs/configuration/overview.md
@@ -196,6 +196,7 @@ An example [oauth2-proxy.cfg](https://github.com/oauth2-proxy/oauth2-proxy/blob/
 | `--standard-logging` | bool | Log standard runtime information | true |
 | `--standard-logging-format` | string | Template for standard log lines | see [Logging Configuration](#logging-configuration) |
 | `--tls-cert-file` | string | path to certificate file | |
+| `--tls-cipher-suite` | string \| list | Restricts TLS cipher suites used by server to those listed (e.g. TLS_RSA_WITH_RC4_128_SHA) (may be given multiple times). If not specified, the default Go safe cipher list is used. List of valid cipher suites can be found in the [crypto/tls documentation](https://pkg.go.dev/crypto/tls#pkg-constants). | |
 | `--tls-key-file` | string | path to private key file | |
 | `--tls-min-version` | string | minimum TLS version that is acceptable, either `"TLS1.2"` or `"TLS1.3"` | `"TLS1.2"` |
 | `--upstream` | string \| list | the http url(s) of the upstream endpoint, file:// paths for static files or `static://<status_code>` for static response. Routing is based on the path | |

diff --git a/docs/docs/configuration/tls.md b/docs/docs/configuration/tls.md
@@ -32,8 +32,9 @@ There are two recommended configurations:
     The defaults set `TLS1.2` as the minimal version. 
     Regardless of the minimum version configured, `TLS1.3` is currently always used as the maximal version.
 
-    The server side cipher suites are the defaults from [`crypto/tls`](https://pkg.go.dev/crypto/tls#CipherSuites) of 
-    the currently used `go` version for building `oauth2-proxy`.
+    TLS server side cipher suites can be specified with `--tls-cipher-suite=TLS_RSA_WITH_RC4_128_SHA`.
+    If not specified, the defaults from [`crypto/tls`](https://pkg.go.dev/crypto/tls#CipherSuites) of the currently used `go` version for building `oauth2-proxy` will be used.
+    A complete list of valid TLS cipher suite names can be found in [`crypto/tls`](https://pkg.go.dev/crypto/tls#pkg-constants).
 
 ### Terminate TLS at Reverse Proxy, e.g. Nginx
 

diff --git a/pkg/apis/options/legacy_options.go b/pkg/apis/options/legacy_options.go
@@ -447,15 +447,16 @@ func getXAuthRequestAccessTokenHeader() Header {
 }
 
 type LegacyServer struct {
-	MetricsAddress       string `flag:"metrics-address" cfg:"metrics_address"`
-	MetricsSecureAddress string `flag:"metrics-secure-address" cfg:"metrics_secure_address"`
-	MetricsTLSCertFile   string `flag:"metrics-tls-cert-file" cfg:"metrics_tls_cert_file"`
-	MetricsTLSKeyFile    string `flag:"metrics-tls-key-file" cfg:"metrics_tls_key_file"`
-	HTTPAddress          string `flag:"http-address" cfg:"http_address"`
-	HTTPSAddress         string `flag:"https-address" cfg:"https_address"`
-	TLSCertFile          string `flag:"tls-cert-file" cfg:"tls_cert_file"`
-	TLSKeyFile           string `flag:"tls-key-file" cfg:"tls_key_file"`
-	TLSMinVersion        string `flag:"tls-min-version" cfg:"tls_min_version"`
+	MetricsAddress       string   `flag:"metrics-address" cfg:"metrics_address"`
+	MetricsSecureAddress string   `flag:"metrics-secure-address" cfg:"metrics_secure_address"`
+	MetricsTLSCertFile   string   `flag:"metrics-tls-cert-file" cfg:"metrics_tls_cert_file"`
+	MetricsTLSKeyFile    string   `flag:"metrics-tls-key-file" cfg:"metrics_tls_key_file"`
+	HTTPAddress          string   `flag:"http-address" cfg:"http_address"`
+	HTTPSAddress         string   `flag:"https-address" cfg:"https_address"`
+	TLSCertFile          string   `flag:"tls-cert-file" cfg:"tls_cert_file"`
+	TLSKeyFile           string   `flag:"tls-key-file" cfg:"tls_key_file"`
+	TLSMinVersion        string   `flag:"tls-min-version" cfg:"tls_min_version"`
+	TLSCipherSuites      []string `flag:"tls-cipher-suite" cfg:"tls_cipher_suites"`
 }
 
 func legacyServerFlagset() *pflag.FlagSet {
@@ -470,6 +471,7 @@ func legacyServerFlagset() *pflag.FlagSet {
 	flagSet.String("tls-cert-file", "", "path to certificate file")
 	flagSet.String("tls-key-file", "", "path to private key file")
 	flagSet.String("tls-min-version", "", "minimal TLS version for HTTPS clients (either \"TLS1.2\" or \"TLS1.3\")")
+	flagSet.StringSlice("tls-cipher-suite", []string{}, "restricts TLS cipher suites to those listed (e.g. TLS_RSA_WITH_RC4_128_SHA) (may be given multiple times)")
 
 	return flagSet
 }
@@ -600,6 +602,9 @@ func (l LegacyServer) convert() (Server, Server) {
 			},
 			MinVersion: l.TLSMinVersion,
 		}
+		if len(l.TLSCipherSuites) != 0 {
+			appServer.TLS.CipherSuites = l.TLSCipherSuites
+		}
 		// Preserve backwards compatibility, only run one server
 		appServer.BindAddress = ""
 	} else {

diff --git a/pkg/apis/options/legacy_options_test.go b/pkg/apis/options/legacy_options_test.go
@@ -804,6 +804,7 @@ var _ = Describe("Legacy Options", func() {
 			keyPath             = "tls.key"
 			minVersion          = "TLS1.3"
 		)
+		cipherSuites := []string{"TLS_RSA_WITH_AES_128_GCM_SHA256", "TLS_RSA_WITH_AES_256_GCM_SHA384"}
 
 		var tlsConfig = &TLS{
 			Cert: &SecretSource{
@@ -820,6 +821,15 @@ var _ = Describe("Legacy Options", func() {
 			MinVersion: minVersion,
 		}
 
+		var tlsConfigCipherSuites = &TLS{
+			Cert: tlsConfig.Cert,
+			Key:  tlsConfig.Key,
+			CipherSuites: []string{
+				"TLS_RSA_WITH_AES_128_GCM_SHA256",
+				"TLS_RSA_WITH_AES_256_GCM_SHA384",
+			},
+		}
+
 		DescribeTable("should convert to app and metrics servers",
 			func(in legacyServersTableInput) {
 				appServer, metricsServer := in.legacyServer.convert()
@@ -860,6 +870,19 @@ var _ = Describe("Legacy Options", func() {
 					TLS:               tlsConfigMinVersion,
 				},
 			}),
+			Entry("with TLS options specified with CipherSuites", legacyServersTableInput{
+				legacyServer: LegacyServer{
+					HTTPAddress:     insecureAddr,
+					HTTPSAddress:    secureAddr,
+					TLSKeyFile:      keyPath,
+					TLSCertFile:     crtPath,
+					TLSCipherSuites: cipherSuites,
+				},
+				expectedAppServer: Server{
+					SecureBindAddress: secureAddr,
+					TLS:               tlsConfigCipherSuites,
+				},
+			}),
 			Entry("with metrics HTTP and HTTPS addresses", legacyServersTableInput{
 				legacyServer: LegacyServer{
 					HTTPAddress:          insecureAddr,

diff --git a/pkg/apis/options/server.go b/pkg/apis/options/server.go
@@ -29,4 +29,12 @@ type TLS struct {
 	// MinVersion is the minimal TLS version that is acceptable.
 	// E.g. Set to "TLS1.3" to select TLS version 1.3
 	MinVersion string
+
+	// CipherSuites is a list of TLS cipher suites that are allowed.
+	// E.g.:
+	// - TLS_RSA_WITH_RC4_128_SHA
+	// - TLS_RSA_WITH_AES_256_GCM_SHA384
+	// If not specified, the default Go safe cipher list is used.
+	// List of valid cipher suites can be found in the [crypto/tls documentation](https://pkg.go.dev/crypto/tls#pkg-constants).
+	CipherSuites []string
 }
diff --git a/pkg/http/server.go b/pkg/http/server.go
@@ -81,6 +81,27 @@ func (s *server) setupListener(opts Opts) error {
 	return nil
 }
 
+func parseCipherSuites(names []string) ([]uint16, error) {
+	cipherNameMap := make(map[string]uint16)
+
+	for _, cipherSuite := range tls.CipherSuites() {
+		cipherNameMap[cipherSuite.Name] = cipherSuite.ID
+	}
+	for _, cipherSuite := range tls.InsecureCipherSuites() {
+		cipherNameMap[cipherSuite.Name] = cipherSuite.ID
+	}
+
+	result := make([]uint16, len(names))
+	for i, name := range names {
+		id, present := cipherNameMap[name]
+		if !present {
+			return nil, fmt.Errorf("unknown TLS cipher suite name specified %q", name)
+		}
+		result[i] = id
+	}
+	return result, nil
+}
+
 // setupTLSListener sets the server TLS listener if the HTTPS server is enabled.
 // The HTTPS server can be disabled by setting the SecureBindAddress to "-" or by
 // leaving it empty.
@@ -104,6 +125,14 @@ func (s *server) setupTLSListener(opts Opts) error {
 	}
 	config.Certificates = []tls.Certificate{cert}
 
+	if len(opts.TLS.CipherSuites) > 0 {
+		cipherSuites, err := parseCipherSuites(opts.TLS.CipherSuites)
+		if err != nil {
+			return fmt.Errorf("could not parse cipher suites: %v", err)
+		}
+		config.CipherSuites = cipherSuites
+	}
+
 	if len(opts.TLS.MinVersion) > 0 {
 		switch opts.TLS.MinVersion {
 		case "TLS1.2":

diff --git a/pkg/http/server_test.go b/pkg/http/server_test.go
@@ -261,6 +261,40 @@ var _ = Describe("Server", func() {
 				expectHTTPListener: false,
 				expectTLSListener:  true,
 			}),
+			Entry("with an ipv4 valid https bind address, and valid TLS config with CipherSuites", &newServerTableInput{
+				opts: Opts{
+					Handler:           handler,
+					SecureBindAddress: "127.0.0.1:0",
+					TLS: &options.TLS{
+						Key:  &ipv4KeyDataSource,
+						Cert: &ipv4CertDataSource,
+						CipherSuites: []string{
+							"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
+							"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
+						},
+					},
+				},
+				expectedErr:        nil,
+				expectHTTPListener: false,
+				expectTLSListener:  true,
+			}),
+			Entry("with an ipv4 valid https bind address, and invalid TLS config with unknown CipherSuites", &newServerTableInput{
+				opts: Opts{
+					Handler:           handler,
+					SecureBindAddress: "127.0.0.1:0",
+					TLS: &options.TLS{
+						Key:  &ipv4KeyDataSource,
+						Cert: &ipv4CertDataSource,
+						CipherSuites: []string{
+							"TLS_RSA_WITH_RC4_64_SHA",
+							"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
+						},
+					},
+				},
+				expectedErr:        errors.New("error setting up TLS listener: could not parse cipher suites: unknown TLS cipher suite name specified \"TLS_RSA_WITH_RC4_64_SHA\""),
+				expectHTTPListener: false,
+				expectTLSListener:  true,
+			}),
 			Entry("with an ipv6 valid http bind address", &newServerTableInput{
 				opts: Opts{
 					Handler:     handler,
@@ -454,6 +488,40 @@ var _ = Describe("Server", func() {
 				expectHTTPListener: false,
 				expectTLSListener:  true,
 			}),
+			Entry("with an ipv6 valid https bind address, and valid TLS config with CipherSuites", &newServerTableInput{
+				opts: Opts{
+					Handler:           handler,
+					SecureBindAddress: "[::1]:0",
+					TLS: &options.TLS{
+						Key:  &ipv4KeyDataSource,
+						Cert: &ipv4CertDataSource,
+						CipherSuites: []string{
+							"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
+							"TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA",
+						},
+					},
+				},
+				expectedErr:        nil,
+				expectHTTPListener: false,
+				expectTLSListener:  true,
+			}),
+			Entry("with an ipv6 valid https bind address, and invalid TLS config with unknown CipherSuites", &newServerTableInput{
+				opts: Opts{
+					Handler:           handler,
+					SecureBindAddress: "[::1]:0",
+					TLS: &options.TLS{
+						Key:  &ipv4KeyDataSource,
+						Cert: &ipv4CertDataSource,
+						CipherSuites: []string{
+							"TLS_RSA_WITH_RC4_64_SHA",
+							"TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA",
+						},
+					},
+				},
+				expectedErr:        errors.New("error setting up TLS listener: could not parse cipher suites: unknown TLS cipher suite name specified \"TLS_RSA_WITH_RC4_64_SHA\""),
+				expectHTTPListener: false,
+				expectTLSListener:  true,
+			}),
 		)
 	})