- 
                Notifications
    
You must be signed in to change notification settings  - Fork 2.5k
 
feat: add TLS URL parameters for rediss:// URLs #3475
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
base: master
Are you sure you want to change the base?
Changes from 4 commits
c1e788b
              2e2225e
              185f6ab
              3ba4a9a
              8c57646
              85cfa2d
              a070b72
              1cfe757
              a443622
              2614ca0
              62a56aa
              8ff9a76
              835d6ef
              5060993
              7add47d
              56829d4
              15872f5
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| 
          
            
          
           | 
    @@ -10,6 +10,27 @@ import ( | |
| ) | ||
| 
     | 
||
| func TestParseURL(t *testing.T) { | ||
| certPem := []byte(`-----BEGIN CERTIFICATE----- | ||
| MIIBhTCCASugAwIBAgIQIRi6zePL6mKjOipn+dNuaTAKBggqhkjOPQQDAjASMRAw | ||
| DgYDVQQKEwdBY21lIENvMB4XDTE3MTAyMDE5NDMwNloXDTE4MTAyMDE5NDMwNlow | ||
| EjEQMA4GA1UEChMHQWNtZSBDbzBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABD0d | ||
| 7VNhbWvZLWPuj/RtHFjvtJBEwOkhbN/BnnE8rnZR8+sbwnc/KhCk3FhnpHZnQz7B | ||
| 5aETbbIgmuvewdjvSBSjYzBhMA4GA1UdDwEB/wQEAwICpDATBgNVHSUEDDAKBggr | ||
| BgEFBQcDATAPBgNVHRMBAf8EBTADAQH/MCkGA1UdEQQiMCCCDmxvY2FsaG9zdDo1 | ||
| NDUzgg4xMjcuMC4wLjE6NTQ1MzAKBggqhkjOPQQDAgNIADBFAiEA2zpJEPQyz6/l | ||
| Wf86aX6PepsntZv2GYlA5UpabfT2EZICICpJ5h/iI+i341gBmLiAFQOyTDT+/wQc | ||
| 6MF9+Yw1Yy0t | ||
| -----END CERTIFICATE-----`) | ||
| keyPem := []byte(`-----BEGIN EC PRIVATE KEY----- | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security control: Secret Detection Trufflehog Privatekey PrivateKey (Unverified) Severity: HIGH Jit Bot commands and options (e.g., ignore issue)You can trigger Jit actions by commenting on this PR review: 
  | 
||
| MHcCAQEEIIrYSSNQFaA2Hwf1duRSxKtLYX5CB04fSeQ6tF1aY/PuoAoGCCqGSM49 | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security control: Secret Detection Trufflehog Privatekey PrivateKey (Unverified) Severity: HIGH Jit Bot commands and options (e.g., ignore issue)You can trigger Jit actions by commenting on this PR review: 
  | 
||
| AwEHoUQDQgAEPR3tU2Fta9ktY+6P9G0cWO+0kETA6SFs38GecTyudlHz6xvCdz8q | ||
| EKTcWGekdmdDPsHloRNtsiCa697B2O9IFA== | ||
| -----END EC PRIVATE KEY-----`) | ||
| testCert, err := tls.X509KeyPair(certPem, keyPem) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| 
     | 
||
| cases := []struct { | ||
| url string | ||
| o *Options // expected value | ||
| 
        
          
        
         | 
    @@ -29,10 +50,27 @@ func TestParseURL(t *testing.T) { | |
| o: &Options{Addr: "12345:6379"}, | ||
| }, { | ||
| url: "rediss://localhost:123", | ||
| o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ /* no deep comparison */ }}, | ||
| o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "localhost", MinVersion: tls.VersionTLS12}}, | ||
| }, { | ||
| url: "rediss://localhost:123?tls_server_name=abc&tls_min_version=1&tls_max_version=3&skip_verify=true", | ||
| o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "abc", MinVersion: 1, MaxVersion: 3, InsecureSkipVerify: true}}, | ||
| }, { | ||
| url: "rediss://localhost:123?tls_cert_file=./testdata/testcert.pem&tls_key_file=./testdata/testkey.pem", | ||
| o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "localhost", MinVersion: tls.VersionTLS12, Certificates: []tls.Certificate{testCert}}}, | ||
| }, { | ||
| url: "rediss://localhost:123?tls_cert_file=./testdata/doesnotexist.pem&tls_key_file=./testdata/testkey.pem", | ||
| o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "abc"}}, | ||
| err: errors.New("redis: error loading TLS certificate: open ./testdata/doesnotexist.pem: no such file or directory"), | ||
| }, { | ||
| url: "rediss://localhost:123?tls_cert_file=./testdata/testcert.pem", | ||
| o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "abc"}}, | ||
| err: errors.New("redis: tls_cert_file and tls_key_file URL parameters must be both set or both omitted"), | ||
| }, { | ||
| url: "rediss://localhost:123?tls_key_file=./testdata/testkey.pem", | ||
| err: errors.New("redis: tls_cert_file and tls_key_file URL parameters must be both set or both omitted"), | ||
| }, { | ||
| url: "rediss://localhost:123/?skip_verify=true", | ||
| o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{InsecureSkipVerify: true}}, | ||
| o: &Options{Addr: "localhost:123", TLSConfig: &tls.Config{ServerName: "localhost", MinVersion: tls.VersionTLS12, InsecureSkipVerify: true}}, | ||
| }, { | ||
| url: "redis://:bar@localhost:123", | ||
| o: &Options{Addr: "localhost:123", Password: "bar"}, | ||
| 
          
            
          
           | 
    @@ -197,6 +235,39 @@ func comprareOptions(t *testing.T, actual, expected *Options) { | |
| if actual.ConnMaxLifetime != expected.ConnMaxLifetime { | ||
| t.Errorf("ConnMaxLifetime: got %v, expected %v", actual.ConnMaxLifetime, expected.ConnMaxLifetime) | ||
| } | ||
| 
     | 
||
| if (actual.TLSConfig == nil) != (expected.TLSConfig == nil) { | ||
| t.Errorf("TLSConfig nil: got %v, expected %v", actual.TLSConfig == nil, expected.TLSConfig == nil) | ||
| } | ||
| 
     | 
||
| if (actual.TLSConfig != nil) && (expected.TLSConfig != nil) { | ||
| if actual.TLSConfig.MinVersion != expected.TLSConfig.MinVersion { | ||
| t.Errorf("TLSConfig.MinVersion: got %v, expected %v", actual.TLSConfig.MinVersion, expected.TLSConfig.MinVersion) | ||
| } | ||
| 
     | 
||
| if actual.TLSConfig.MaxVersion != expected.TLSConfig.MaxVersion { | ||
| t.Errorf("TLSConfig.MaxVersion: got %v, expected %v", actual.TLSConfig.MaxVersion, expected.TLSConfig.MaxVersion) | ||
| } | ||
| 
     | 
||
| if actual.TLSConfig.ServerName != expected.TLSConfig.ServerName { | ||
| t.Errorf("TLSConfig.ServerName: got %v, expected %v", actual.TLSConfig.ServerName, expected.TLSConfig.ServerName) | ||
| } | ||
| 
     | 
||
| if actual.TLSConfig.InsecureSkipVerify != expected.TLSConfig.InsecureSkipVerify { | ||
| t.Errorf("TLSConfig.InsecureSkipVerify: got %v, expected %v", actual.TLSConfig.InsecureSkipVerify, expected.TLSConfig.InsecureSkipVerify) | ||
| } | ||
| 
     | 
||
| if len(actual.TLSConfig.Certificates) != len(expected.TLSConfig.Certificates) { | ||
| t.Errorf("TLSConfig.Certificates: got %v, expected %v", actual.TLSConfig.Certificates, expected.TLSConfig.Certificates) | ||
| } | ||
| 
     | 
||
| for i, actualCert := range actual.TLSConfig.Certificates { | ||
| expectedCert := expected.TLSConfig.Certificates[i] | ||
| if !actualCert.Leaf.Equal(expectedCert.Leaf) { | ||
| t.Errorf("TLSConfig.Certificates[%d].Leaf: got %v, expected %v", i, actual.TLSConfig.Certificates, expected.TLSConfig.Certificates) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| 
     | 
||
| // Test ReadTimeout option initialization, including special values -1 and 0. | ||
| 
          
            
          
           | 
    ||
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| -----BEGIN CERTIFICATE----- | ||
| MIIBhTCCASugAwIBAgIQIRi6zePL6mKjOipn+dNuaTAKBggqhkjOPQQDAjASMRAw | ||
| DgYDVQQKEwdBY21lIENvMB4XDTE3MTAyMDE5NDMwNloXDTE4MTAyMDE5NDMwNlow | ||
| EjEQMA4GA1UEChMHQWNtZSBDbzBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABD0d | ||
| 7VNhbWvZLWPuj/RtHFjvtJBEwOkhbN/BnnE8rnZR8+sbwnc/KhCk3FhnpHZnQz7B | ||
| 5aETbbIgmuvewdjvSBSjYzBhMA4GA1UdDwEB/wQEAwICpDATBgNVHSUEDDAKBggr | ||
| BgEFBQcDATAPBgNVHRMBAf8EBTADAQH/MCkGA1UdEQQiMCCCDmxvY2FsaG9zdDo1 | ||
| NDUzgg4xMjcuMC4wLjE6NTQ1MzAKBggqhkjOPQQDAgNIADBFAiEA2zpJEPQyz6/l | ||
| Wf86aX6PepsntZv2GYlA5UpabfT2EZICICpJ5h/iI+i341gBmLiAFQOyTDT+/wQc | ||
| 6MF9+Yw1Yy0t | ||
| -----END CERTIFICATE----- | 
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| -----BEGIN EC PRIVATE KEY----- | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security control: Secret Detection Trufflehog Privatekey PrivateKey (Unverified) Severity: HIGH Jit Bot commands and options (e.g., ignore issue)You can trigger Jit actions by commenting on this PR review: 
  | 
||
| MHcCAQEEIIrYSSNQFaA2Hwf1duRSxKtLYX5CB04fSeQ6tF1aY/PuoAoGCCqGSM49 | ||
| 
         There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security control: Static Code Analysis Semgrep Pro Generic.Secrets.Security.Detected-Private-Key.Detected-Private-Key Private Key detected. This is a sensitive credential and should not be hardcoded here. Instead, store this in a separate, private file. Severity: HIGH Jit Bot commands and options (e.g., ignore issue)You can trigger Jit actions by commenting on this PR review: 
  | 
||
| AwEHoUQDQgAEPR3tU2Fta9ktY+6P9G0cWO+0kETA6SFs38GecTyudlHz6xvCdz8q | ||
| EKTcWGekdmdDPsHloRNtsiCa697B2O9IFA== | ||
| -----END EC PRIVATE KEY----- | ||
Uh oh!
There was an error while loading. Please reload this page.