[TT-13629]: OAS upstream SSL configuration

[kofoworola]

User description

Description

TT-13629

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

• I ensured that the documentation is up to date
• I explained why this PR updates go.mod in detail with reasoning why it's required
• I would like a code coverage CI quality gate exception and have explained why

---

PR Type

Enhancement, Tests, Documentation

---

Description

• Added TLS transport configuration to the upstream settings.

• Introduced new TLSTransport and Proxy structs with fill and extract methods.

• Updated schema definitions to include TLS transport and proxy configurations.

• Enhanced test coverage for TLS transport and proxy settings.

---

Changes walkthrough 📝

	Relevant files
Tests	linter_test.go Update linter tests for TLS transport settings                      apidef/oas/linter_test.go Added TLS transport settings to upstream configuration in test cases. Updated test logic to include TLS version and cipher settings. +4/-0      upstream_test.go Add tests for TLS transport and proxy settings                      apidef/oas/upstream_test.go Added test cases for TLSTransport and Proxy configurations. Validated fill and extract methods for TLS transport and proxy. +51/-0
Enhancement	upstream.go Add TLS transport and proxy configuration logic                    apidef/oas/upstream.go Introduced TLSTransport and Proxy structs for upstream. Added methods to fill and extract TLS transport and proxy settings. Implemented utility functions for TLS version conversion. +133/-0
Documentation	x-tyk-api-gateway.json Update schema for TLS transport and proxy                                apidef/oas/schema/x-tyk-api-gateway.json Updated schema to include tlsTransport and proxy in upstream. Added required fields and enumerations for TLS settings. Simplified and reformatted schema requirements. +159/-332

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 5 🔵🔵🔵🔵🔵

🧪 No relevant tests

🔒 Security concerns TLS Configuration: The X-Tyk-TLSTransport struct allows for insecure configurations, such as InsecureSkipVerify. Ensure that these settings are not enabled in production environments to avoid potential security vulnerabilities.

⚡ Recommended focus areas for review Possible Issue The tlsVersionFromString and tlsVersionToString functions do not handle invalid or unexpected input gracefully. Consider adding error handling or logging for unsupported versions. func tlsVersionFromString(v string) uint16 { switch v { case "1.0": return tls.VersionTLS10 case "1.1": return tls.VersionTLS11 case "1.2": return tls.VersionTLS12 case "1.3": return tls.VersionTLS13 default: return 0 } } // tlsVersionFromString converts v from version into to the form 1.0/1.1 func tlsVersionToString(v uint16) string { switch v { case tls.VersionTLS10: return "1.0" case tls.VersionTLS11: return "1.1" case tls.VersionTLS12: return "1.2" case tls.VersionTLS13: return "1.3" default: return "" Test Coverage The test TestTLSTransportProxy does not fully validate the Proxy struct's behavior. Ensure that the Proxy struct's Fill and ExtractTo methods are adequately tested. t.Run("proxy settings", func(t *testing.T) { proxyTransport := Proxy{ URL: "proxy-url", } var convertedAPI apidef.APIDefinition var resultProxy Proxy convertedAPI.SetDisabledFlags() proxyTransport.Fill(convertedAPI) assert.Equal(t, proxyTransport, resultProxy) }) Schema Validation The added X-Tyk-TLSTransport schema includes a large list of cipher suites. Ensure that this list is comprehensive and aligns with the intended security requirements. "X-Tyk-TLSTransport": { "type": "object", "properties": { "insecureSkipVerify": { "type": "boolean" }, "ciphers": { "type": "array", "items": { "type": "string", "enum": [ "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", "TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384", "TLS_CHACHA20_POLY1305_SHA256", "TLS_FALLBACK_SCSV", "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305", "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305" ] } }, "minVersion": { "type": "string", "enum": ["1.0", "1.1", "1.2", "1.3"] }, "maxVersion": { "type": "string", "enum": ["1.0", "1.1", "1.2", "1.3"] }, "forceCommonNameCheck": { "type": "boolean" } } },

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Security	Replace insecure cipher suite Avoid using the insecure cipher suite "TLS_RSA_WITH_RC4_128_SHA" as it is considered outdated and vulnerable; replace it with a more secure option. apidef/oas/linter_test.go [96] -settings.Upstream.TLSTransport.Ciphers = []string{"TLS_RSA_WITH_RC4_128_SHA"} +settings.Upstream.TLSTransport.Ciphers = []string{"TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"} Suggestion importance[1-10]: 9 Why: The suggestion addresses a critical security issue by replacing an outdated and vulnerable cipher suite with a more secure option. This change significantly enhances the security of the TLS configuration.	9
	Remove insecure cipher suites from ciphers Ensure that the ciphers array in the X-Tyk-TLSTransport definition includes only secure and modern cipher suites, as some listed ciphers (e.g., TLS_RSA_WITH_RC4_128_SHA) are considered outdated and insecure. apidef/oas/schema/x-tyk-api-gateway.json [2032-2066] "ciphers": { "type": "array", "items": { "type": "string", "enum": [ - "TLS_RSA_WITH_RC4_128_SHA", - "TLS_RSA_WITH_3DES_EDE_CBC_SHA", - ... + "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", + "TLS_AES_128_GCM_SHA256", + "TLS_AES_256_GCM_SHA384", + "TLS_CHACHA20_POLY1305_SHA256" ] } } Suggestion importance[1-10]: 9 Why: The suggestion addresses a critical security concern by removing outdated and insecure cipher suites from the ciphers array. This change ensures the use of modern and secure cipher suites, which is essential for maintaining robust encryption and protecting against vulnerabilities.	9
	Enforce secure TLS versions in configuration Validate that the minVersion and maxVersion fields in the X-Tyk-TLSTransport definition are correctly configured to enforce secure TLS versions, ideally setting minVersion to "1.2" or higher. apidef/oas/schema/x-tyk-api-gateway.json [2068-2074] "minVersion": { "type": "string", - "enum": ["1.0", "1.1", "1.2", "1.3"] + "enum": ["1.2", "1.3"] }, "maxVersion": { "type": "string", - "enum": ["1.0", "1.1", "1.2", "1.3"] + "enum": ["1.2", "1.3"] } Suggestion importance[1-10]: 8 Why: The suggestion improves security by enforcing a minimum TLS version of 1.2, which is widely regarded as the baseline for secure communications. This change reduces the risk of using deprecated and insecure TLS versions.	8
	Ensure forceCommonNameCheck is secure Add validation or documentation to ensure that the forceCommonNameCheck field in the X-Tyk-TLSTransport definition is set appropriately, as disabling it could lead to security vulnerabilities. apidef/oas/schema/x-tyk-api-gateway.json [2076-2078] "forceCommonNameCheck": { - "type": "boolean" + "type": "boolean", + "default": true, + "description": "Enforces validation of the server's common name in the certificate." } Suggestion importance[1-10]: 7 Why: Adding a default value and description for forceCommonNameCheck enhances security by encouraging proper configuration and providing clarity about its purpose. However, the suggestion is less impactful than the previous ones as it does not enforce a specific secure default behavior.	7
General	Handle invalid TLS version input Ensure the tlsVersionFromString function handles invalid or unexpected input gracefully by logging or returning an error instead of defaulting to 0. apidef/oas/upstream.go [275-276] default: + log.Printf("Invalid TLS version: %s", v) return 0 Suggestion importance[1-10]: 7 Why: The suggestion improves error handling by logging invalid TLS version inputs, which enhances debugging and prevents silent failures. However, it does not fully address the issue of returning a potentially misleading default value (0).	7
	Correct typo in test name Fix the typo in the test case name "emmpty tls settings" to "empty tls settings" for better readability and consistency. apidef/oas/upstream_test.go [501] -t.Run("emmpty tls settings", func(t *testing.T) { +t.Run("empty tls settings", func(t *testing.T) { Suggestion importance[1-10]: 5 Why: Fixing the typo in the test case name improves code readability and consistency, which is beneficial for maintaining the test suite. However, the impact on functionality is minimal.	5

[github-actions]

API Changes

--- prev.txt	2025-01-23 08:27:53.416521359 +0000
+++ current.txt	2025-01-23 08:27:48.204537201 +0000
@@ -3624,6 +3624,23 @@
     Provider defines an issuer to validate and the Client ID to Policy ID
     mappings.
 
+type Proxy struct {
+	// Enabled determines if the proxy is active.
+	Enabled bool `bson:"enabled" json:"enabled"`
+
+	// URL specifies the URL of the internal proxy.
+	URL string `bson:"url" json:"url"`
+}
+    Proxy contains the configuration for an internal proxy.
+
+    Tyk classic API definition: `proxy.proxy_url`
+
+func (p *Proxy) ExtractTo(api *apidef.APIDefinition)
+    ExtractTo extracts *Proxy into *apidef.ServiceDiscoveryConfiguration.
+
+func (p *Proxy) Fill(api apidef.APIDefinition)
+    Fill fills *Proxy from apidef.ServiceDiscoveryConfiguration.
+
 type RateLimit struct {
 	// Enabled activates API level rate limiting for this API.
 	//
@@ -4011,6 +4028,58 @@
 func (s *State) Fill(api apidef.APIDefinition)
     Fill fills *State from apidef.APIDefinition.
 
+type TLSTransport struct {
+	// InsecureSkipVerify controls whether a client verifies the server's certificate chain and host name.
+	// If InsecureSkipVerify is true, crypto/tls accepts any certificate presented by the server and any host name in that certificate.
+	// In this mode, TLS is susceptible to machine-in-the-middle attacks unless custom verification is used.
+	// This should be used only for testing or in combination with VerifyConnection or VerifyPeerCertificate.
+	//
+	// Tyk classic API definition: `proxy.transport.ssl_insecure_skip_verify`
+	InsecureSkipVerify bool `bson:"insecureSkipVerify,omitempty" json:"insecureSkipVerify,omitempty"`
+
+	// Ciphers is a list of SSL ciphers to be used. If unset, the default ciphers will be used.
+	//
+	// Tyk classic API definition: `proxy.transport.ssl_ciphers`
+	Ciphers []string `bson:"ciphers,omitempty" json:"ciphers,omitempty"`
+
+	// MinVersion is the minimum SSL/TLS version that is acceptable.
+	// Tyk classic API definition: `proxy.transport.ssl_min_version`
+	MinVersion string `bson:"minVersion,omitempty" json:"minVersion,omitempty"`
+
+	// MaxVersion is the maximum SSL/TLS version that is acceptable.
+	MaxVersion string `bson:"maxVersion,omitempty" json:"maxVersion,omitempty"`
+
+	// ForceCommonNameCheck forces the validation of the hostname against the certificate Common Name.
+	//
+	// Tyk classic API definition: `proxy.transport.ssl_force_common_name_check`
+	ForceCommonNameCheck bool `bson:"forceCommonNameCheck,omitempty" json:"forceCommonNameCheck,omitempty"`
+}
+    TLSTransport contains the configuration for TLS transport settings. This
+    struct allows you to specify a custom proxy and set the minimum TLS versions
+    and any SSL ciphers.
+
+    Example:
+
+        {
+          "proxy_url": "http(s)://proxy.url:1234",
+          "minVersion": "1.0",
+          "maxVersion": "1.0",
+          "ciphers": [
+            "TLS_RSA_WITH_AES_128_GCM_SHA256",
+            "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA"
+          ],
+          "insecureSkipVerify": true,
+          "forceCommonNameCheck": false
+        }
+
+    Tyk classic API definition: `proxy.transport`
+
+func (t *TLSTransport) ExtractTo(api *apidef.APIDefinition)
+    ExtractTo extracts *TLSTransport into *apidef.ServiceDiscoveryConfiguration.
+
+func (t *TLSTransport) Fill(api apidef.APIDefinition)
+    Fill fills *TLSTransport from apidef.ServiceDiscoveryConfiguration.
+
 type Test struct {
 	// ServiceDiscovery contains the configuration related to test Service Discovery.
 	// Tyk classic API definition: `proxy.service_discovery`
@@ -4284,6 +4353,14 @@
 
 	// LoadBalancing contains configuration for load balancing between multiple upstream targets.
 	LoadBalancing *LoadBalancing `bson:"loadBalancing,omitempty" json:"loadBalancing,omitempty"`
+
+	// TLSTransport contains the configuration for TLS transport settings.
+	// Tyk classic API definition: `proxy.transport`
+	TLSTransport *TLSTransport `bson:"tlsTransport,omitempty" json:"tlsTransport,omitempty"`
+
+	// Proxy contains the configuration for an internal proxy.
+	// Tyk classic API definition: `proxy.proxy_url`
+	Proxy *Proxy `bson:"proxy,omitempty" json:"proxy,omitempty"`
 }
     Upstream holds configuration for the upstream server to which Tyk should
     proxy requests.

[titpetric]

Ciphers get added, removed, is there a way to not maintain a definite list which will be bitrot?
E.g. link to an implementation documentation page (crypto tls, other).

[titpetric]

This needs to be in internal/crypto or httputil (whereever more tls code lives, iirc crypto);
Edit: An alternative option is to do func (*TLSTransport) tlsVersion... and keep it here, also good.