Skip to content

Commit b98521b

Browse files
bugerclaude
andcommitted
Fix: Add mutual TLS support for dedicated rate limiter Redis connection
## Problem The dedicated Redis connection for the rate limiter (when configured with `rate_limiter.storage.type: redis`) was not properly implementing mutual TLS authentication. When `UseSSL` was enabled, the code only set `InsecureSkipVerify` and ignored the client certificate configuration (`CertFile`, `KeyFile`, `CAFile`). This created a security vulnerability where even if administrators configured mTLS for all Redis connections, the rate limiter's dedicated connection would bypass these security controls. ## Solution Enhanced the `NewStorage` function in `internal/rate/storage.go` to properly handle TLS configuration with full mutual TLS support: - Added `createTLSConfig` helper function that loads CA certificates, client certificates, and keys - Added support for TLS version configuration (`TLSMinVersion`, `TLSMaxVersion`) - Properly handles certificate loading errors with appropriate logging - Maintains backward compatibility with existing configurations ## Testing - Added comprehensive unit tests for all TLS configuration scenarios - Tests cover: basic SSL, mTLS, CA certificates, TLS versions, and error cases - All existing rate limiter tests continue to pass ## Impact This fix ensures that when administrators configure mutual TLS for Redis connections, the rate limiter's dedicated connection will properly authenticate using client certificates, maintaining consistent security across all Redis connections. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent e7dd30f commit b98521b

File tree

2 files changed

+315
-3
lines changed

2 files changed

+315
-3
lines changed

internal/rate/storage.go

Lines changed: 68 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@ package rate
22

33
import (
44
"crypto/tls"
5+
"crypto/x509"
6+
"os"
57
"time"
68

9+
"github.com/sirupsen/logrus"
710
"github.com/TykTechnologies/tyk/config"
811
"github.com/TykTechnologies/tyk/internal/redis"
912
)
@@ -25,9 +28,7 @@ func NewStorage(cfg *config.StorageOptionsConf) redis.UniversalClient {
2528
var tlsConfig *tls.Config
2629

2730
if cfg.UseSSL {
28-
tlsConfig = &tls.Config{
29-
InsecureSkipVerify: cfg.SSLInsecureSkipVerify,
30-
}
31+
tlsConfig = createTLSConfig(cfg)
3132
}
3233

3334
opts := &redis.UniversalOptions{
@@ -55,3 +56,67 @@ func NewStorage(cfg *config.StorageOptionsConf) redis.UniversalClient {
5556

5657
return redis.NewClient(opts.Simple())
5758
}
59+
60+
// createTLSConfig creates a TLS configuration with proper mTLS support
61+
func createTLSConfig(cfg *config.StorageOptionsConf) *tls.Config {
62+
tlsConfig := &tls.Config{
63+
InsecureSkipVerify: cfg.SSLInsecureSkipVerify,
64+
}
65+
66+
// Set TLS versions if configured
67+
if cfg.TLSMinVersion != "" {
68+
if version, ok := getTLSVersion(cfg.TLSMinVersion); ok {
69+
tlsConfig.MinVersion = version
70+
}
71+
}
72+
if cfg.TLSMaxVersion != "" {
73+
if version, ok := getTLSVersion(cfg.TLSMaxVersion); ok {
74+
tlsConfig.MaxVersion = version
75+
}
76+
}
77+
78+
// Load CA certificate if provided
79+
if cfg.CAFile != "" {
80+
caCert, err := os.ReadFile(cfg.CAFile)
81+
if err != nil {
82+
logrus.WithError(err).Error("Failed to load CA certificate for rate limiter Redis")
83+
return tlsConfig
84+
}
85+
86+
caCertPool := x509.NewCertPool()
87+
if !caCertPool.AppendCertsFromPEM(caCert) {
88+
logrus.Error("Failed to parse CA certificate for rate limiter Redis")
89+
return tlsConfig
90+
}
91+
tlsConfig.RootCAs = caCertPool
92+
}
93+
94+
// Load client certificate and key for mutual TLS
95+
if cfg.CertFile != "" && cfg.KeyFile != "" {
96+
cert, err := tls.LoadX509KeyPair(cfg.CertFile, cfg.KeyFile)
97+
if err != nil {
98+
logrus.WithError(err).Error("Failed to load client certificate and key for rate limiter Redis")
99+
return tlsConfig
100+
}
101+
tlsConfig.Certificates = []tls.Certificate{cert}
102+
}
103+
104+
return tlsConfig
105+
}
106+
107+
// getTLSVersion converts a string TLS version to the corresponding tls constant
108+
func getTLSVersion(version string) (uint16, bool) {
109+
switch version {
110+
case "1.0":
111+
return tls.VersionTLS10, true
112+
case "1.1":
113+
return tls.VersionTLS11, true
114+
case "1.2":
115+
return tls.VersionTLS12, true
116+
case "1.3":
117+
return tls.VersionTLS13, true
118+
default:
119+
logrus.Warnf("Unknown TLS version: %s", version)
120+
return 0, false
121+
}
122+
}

internal/rate/storage_tls_test.go

Lines changed: 247 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,247 @@
1+
package rate
2+
3+
import (
4+
"crypto/rand"
5+
"crypto/rsa"
6+
"crypto/tls"
7+
"crypto/x509"
8+
"crypto/x509/pkix"
9+
"encoding/pem"
10+
"math/big"
11+
"os"
12+
"path/filepath"
13+
"testing"
14+
"time"
15+
16+
"github.com/TykTechnologies/tyk/config"
17+
"github.com/stretchr/testify/assert"
18+
"github.com/stretchr/testify/require"
19+
)
20+
21+
func TestCreateTLSConfig(t *testing.T) {
22+
t.Run("No SSL configured", func(t *testing.T) {
23+
cfg := &config.StorageOptionsConf{
24+
UseSSL: false,
25+
}
26+
tlsConfig := createTLSConfig(cfg)
27+
assert.NotNil(t, tlsConfig)
28+
assert.False(t, tlsConfig.InsecureSkipVerify)
29+
})
30+
31+
t.Run("SSL with InsecureSkipVerify", func(t *testing.T) {
32+
cfg := &config.StorageOptionsConf{
33+
UseSSL: true,
34+
SSLInsecureSkipVerify: true,
35+
}
36+
tlsConfig := createTLSConfig(cfg)
37+
assert.NotNil(t, tlsConfig)
38+
assert.True(t, tlsConfig.InsecureSkipVerify)
39+
})
40+
41+
t.Run("SSL with TLS versions", func(t *testing.T) {
42+
cfg := &config.StorageOptionsConf{
43+
UseSSL: true,
44+
TLSMinVersion: "1.2",
45+
TLSMaxVersion: "1.3",
46+
}
47+
tlsConfig := createTLSConfig(cfg)
48+
assert.NotNil(t, tlsConfig)
49+
assert.Equal(t, uint16(tls.VersionTLS12), tlsConfig.MinVersion)
50+
assert.Equal(t, uint16(tls.VersionTLS13), tlsConfig.MaxVersion)
51+
})
52+
53+
t.Run("SSL with invalid TLS version", func(t *testing.T) {
54+
cfg := &config.StorageOptionsConf{
55+
UseSSL: true,
56+
TLSMinVersion: "invalid",
57+
}
58+
tlsConfig := createTLSConfig(cfg)
59+
assert.NotNil(t, tlsConfig)
60+
assert.Equal(t, uint16(0), tlsConfig.MinVersion) // Should be zero for invalid version
61+
})
62+
63+
t.Run("SSL with CA certificate", func(t *testing.T) {
64+
// Create a temporary CA certificate
65+
tempDir := t.TempDir()
66+
caFile := filepath.Join(tempDir, "ca.crt")
67+
createTestCACert(t, caFile)
68+
69+
cfg := &config.StorageOptionsConf{
70+
UseSSL: true,
71+
CAFile: caFile,
72+
}
73+
tlsConfig := createTLSConfig(cfg)
74+
assert.NotNil(t, tlsConfig)
75+
assert.NotNil(t, tlsConfig.RootCAs)
76+
})
77+
78+
t.Run("SSL with invalid CA certificate path", func(t *testing.T) {
79+
cfg := &config.StorageOptionsConf{
80+
UseSSL: true,
81+
CAFile: "/nonexistent/ca.crt",
82+
}
83+
tlsConfig := createTLSConfig(cfg)
84+
assert.NotNil(t, tlsConfig)
85+
assert.Nil(t, tlsConfig.RootCAs) // Should be nil when CA loading fails
86+
})
87+
88+
t.Run("SSL with client certificate and key", func(t *testing.T) {
89+
// Create temporary client certificate and key
90+
tempDir := t.TempDir()
91+
certFile := filepath.Join(tempDir, "client.crt")
92+
keyFile := filepath.Join(tempDir, "client.key")
93+
createTestCertAndKey(t, certFile, keyFile)
94+
95+
cfg := &config.StorageOptionsConf{
96+
UseSSL: true,
97+
CertFile: certFile,
98+
KeyFile: keyFile,
99+
}
100+
tlsConfig := createTLSConfig(cfg)
101+
assert.NotNil(t, tlsConfig)
102+
assert.Len(t, tlsConfig.Certificates, 1)
103+
})
104+
105+
t.Run("SSL with only cert file (missing key)", func(t *testing.T) {
106+
tempDir := t.TempDir()
107+
certFile := filepath.Join(tempDir, "client.crt")
108+
createTestCACert(t, certFile) // Just create a cert file
109+
110+
cfg := &config.StorageOptionsConf{
111+
UseSSL: true,
112+
CertFile: certFile,
113+
}
114+
tlsConfig := createTLSConfig(cfg)
115+
assert.NotNil(t, tlsConfig)
116+
assert.Len(t, tlsConfig.Certificates, 0) // Should be empty when key is missing
117+
})
118+
119+
t.Run("SSL with invalid client certificate path", func(t *testing.T) {
120+
cfg := &config.StorageOptionsConf{
121+
UseSSL: true,
122+
CertFile: "/nonexistent/client.crt",
123+
KeyFile: "/nonexistent/client.key",
124+
}
125+
tlsConfig := createTLSConfig(cfg)
126+
assert.NotNil(t, tlsConfig)
127+
assert.Len(t, tlsConfig.Certificates, 0) // Should be empty when loading fails
128+
})
129+
130+
t.Run("SSL with full mTLS configuration", func(t *testing.T) {
131+
// Create all certificates
132+
tempDir := t.TempDir()
133+
caFile := filepath.Join(tempDir, "ca.crt")
134+
certFile := filepath.Join(tempDir, "client.crt")
135+
keyFile := filepath.Join(tempDir, "client.key")
136+
137+
createTestCACert(t, caFile)
138+
createTestCertAndKey(t, certFile, keyFile)
139+
140+
cfg := &config.StorageOptionsConf{
141+
UseSSL: true,
142+
SSLInsecureSkipVerify: false,
143+
CAFile: caFile,
144+
CertFile: certFile,
145+
KeyFile: keyFile,
146+
TLSMinVersion: "1.2",
147+
TLSMaxVersion: "1.3",
148+
}
149+
150+
tlsConfig := createTLSConfig(cfg)
151+
assert.NotNil(t, tlsConfig)
152+
assert.False(t, tlsConfig.InsecureSkipVerify)
153+
assert.NotNil(t, tlsConfig.RootCAs)
154+
assert.Len(t, tlsConfig.Certificates, 1)
155+
assert.Equal(t, uint16(tls.VersionTLS12), tlsConfig.MinVersion)
156+
assert.Equal(t, uint16(tls.VersionTLS13), tlsConfig.MaxVersion)
157+
})
158+
}
159+
160+
func TestGetTLSVersion(t *testing.T) {
161+
tests := []struct {
162+
input string
163+
expected uint16
164+
ok bool
165+
}{
166+
{"1.0", tls.VersionTLS10, true},
167+
{"1.1", tls.VersionTLS11, true},
168+
{"1.2", tls.VersionTLS12, true},
169+
{"1.3", tls.VersionTLS13, true},
170+
{"invalid", 0, false},
171+
{"", 0, false},
172+
}
173+
174+
for _, tt := range tests {
175+
t.Run(tt.input, func(t *testing.T) {
176+
version, ok := getTLSVersion(tt.input)
177+
assert.Equal(t, tt.expected, version)
178+
assert.Equal(t, tt.ok, ok)
179+
})
180+
}
181+
}
182+
183+
// Helper function to create a test CA certificate
184+
func createTestCACert(t *testing.T, filename string) {
185+
template := x509.Certificate{
186+
SerialNumber: big.NewInt(1),
187+
Subject: pkix.Name{
188+
Organization: []string{"Test CA"},
189+
},
190+
NotBefore: time.Now(),
191+
NotAfter: time.Now().Add(365 * 24 * time.Hour),
192+
KeyUsage: x509.KeyUsageCertSign | x509.KeyUsageDigitalSignature,
193+
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
194+
BasicConstraintsValid: true,
195+
IsCA: true,
196+
}
197+
198+
priv, err := rsa.GenerateKey(rand.Reader, 2048)
199+
require.NoError(t, err)
200+
201+
certDER, err := x509.CreateCertificate(rand.Reader, &template, &template, &priv.PublicKey, priv)
202+
require.NoError(t, err)
203+
204+
certPEM := pem.EncodeToMemory(&pem.Block{
205+
Type: "CERTIFICATE",
206+
Bytes: certDER,
207+
})
208+
209+
err = os.WriteFile(filename, certPEM, 0644)
210+
require.NoError(t, err)
211+
}
212+
213+
// Helper function to create a test client certificate and key
214+
func createTestCertAndKey(t *testing.T, certFile, keyFile string) {
215+
priv, err := rsa.GenerateKey(rand.Reader, 2048)
216+
require.NoError(t, err)
217+
218+
template := x509.Certificate{
219+
SerialNumber: big.NewInt(1),
220+
Subject: pkix.Name{
221+
Organization: []string{"Test Client"},
222+
},
223+
NotBefore: time.Now(),
224+
NotAfter: time.Now().Add(365 * 24 * time.Hour),
225+
KeyUsage: x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
226+
ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
227+
}
228+
229+
certDER, err := x509.CreateCertificate(rand.Reader, &template, &template, &priv.PublicKey, priv)
230+
require.NoError(t, err)
231+
232+
certPEM := pem.EncodeToMemory(&pem.Block{
233+
Type: "CERTIFICATE",
234+
Bytes: certDER,
235+
})
236+
237+
keyPEM := pem.EncodeToMemory(&pem.Block{
238+
Type: "RSA PRIVATE KEY",
239+
Bytes: x509.MarshalPKCS1PrivateKey(priv),
240+
})
241+
242+
err = os.WriteFile(certFile, certPEM, 0644)
243+
require.NoError(t, err)
244+
245+
err = os.WriteFile(keyFile, keyPEM, 0600)
246+
require.NoError(t, err)
247+
}

0 commit comments

Comments
 (0)