From dd9eef31ff6c3ed8fd71e22532512a948b56ea71 Mon Sep 17 00:00:00 2001 From: Jim Wang Date: Mon, 7 Dec 2020 14:26:33 -0700 Subject: [PATCH 1/5] feat(security): implement secrets-config proxy tls Add implementation for secrets-config: proxy tls subcommand It uploads the user-provided Kong TLS certificate/key pair to proxy server. Closes: #2866 Signed-off-by: Jim Wang --- .../config/command/proxy/adduser/command.go | 12 +- .../config/command/proxy/common/common.go | 11 + .../config/command/proxy/tls/command.go | 181 ++++++++++++++- .../config/command/proxy/tls/command_test.go | 213 +++++++++++++++++- .../command/proxy/tls/testdata/testCert.pem | 32 +++ .../command/proxy/tls/testdata/testCert.prkey | 52 +++++ 6 files changed, 481 insertions(+), 20 deletions(-) create mode 100644 internal/security/config/command/proxy/common/common.go create mode 100644 internal/security/config/command/proxy/tls/testdata/testCert.pem create mode 100644 internal/security/config/command/proxy/tls/testdata/testCert.prkey diff --git a/internal/security/config/command/proxy/adduser/command.go b/internal/security/config/command/proxy/adduser/command.go index d3a7405f03..0e6db7d116 100644 --- a/internal/security/config/command/proxy/adduser/command.go +++ b/internal/security/config/command/proxy/adduser/command.go @@ -18,6 +18,7 @@ import ( "strings" "github.com/edgexfoundry/edgex-go/internal" + "github.com/edgexfoundry/edgex-go/internal/security/config/command/proxy/common" "github.com/edgexfoundry/edgex-go/internal/security/config/interfaces" "github.com/edgexfoundry/edgex-go/internal/security/proxy/config" "github.com/edgexfoundry/edgex-go/internal/security/secretstoreclient" @@ -27,8 +28,7 @@ import ( ) const ( - CommandName string = "adduser" - urlEncodedForm string = "application/x-www-form-urlencoded" + CommandName string = "adduser" ) type cmd struct { @@ -134,7 +134,7 @@ func (c *cmd) createConsumer() error { if err != nil { return fmt.Errorf("Failed to prepare new consumer request %s: %w", c.username, err) } - req.Header.Add(clients.ContentType, urlEncodedForm) + req.Header.Add(clients.ContentType, common.UrlEncodedForm) resp, err := c.client.Do(req) if err != nil { return fmt.Errorf("Failed to send new consumer request %s: %w", c.username, err) @@ -174,7 +174,7 @@ func (c *cmd) addUserToGroup() error { if err != nil { return fmt.Errorf("Failed to build request to associate consumer %s to group %s: %w", c.username, c.group, err) } - req.Header.Add(clients.ContentType, urlEncodedForm) + req.Header.Add(clients.ContentType, common.UrlEncodedForm) resp, err := c.client.Do(req) if err != nil { return fmt.Errorf("Failed to submit request to associate consumer %s to group %s: %w", c.username, c.group, err) @@ -234,7 +234,7 @@ func (c *cmd) ExecuteAddJwt() (int, error) { if err != nil { return interfaces.StatusCodeExitWithError, fmt.Errorf("Failed to prepare request to associate JWT to user %s: %w", c.username, err) } - req.Header.Add(clients.ContentType, urlEncodedForm) + req.Header.Add(clients.ContentType, common.UrlEncodedForm) resp, err := c.client.Do(req) if err != nil { return interfaces.StatusCodeExitWithError, fmt.Errorf("Failed to send request to associate JWT to user %s: %w", c.username, err) @@ -298,7 +298,7 @@ func (c *cmd) ExecuteAddOAuth2() (statusCode int, err error) { if err != nil { return interfaces.StatusCodeExitWithError, fmt.Errorf("Failed to prepare request to create oauth application %s: %w", c.username, err) } - req.Header.Add(clients.ContentType, urlEncodedForm) + req.Header.Add(clients.ContentType, common.UrlEncodedForm) resp, err := c.client.Do(req) if err != nil { return interfaces.StatusCodeExitWithError, fmt.Errorf("Failed to send request to create oauth application %s: %w", c.username, err) diff --git a/internal/security/config/command/proxy/common/common.go b/internal/security/config/command/proxy/common/common.go new file mode 100644 index 0000000000..d493578484 --- /dev/null +++ b/internal/security/config/command/proxy/common/common.go @@ -0,0 +1,11 @@ +// +// Copyright (c) 2020 Intel Corporation +// +// SPDX-License-Identifier: Apache-2.0' +// + +package common + +const ( + UrlEncodedForm = "application/x-www-form-urlencoded" +) diff --git a/internal/security/config/command/proxy/tls/command.go b/internal/security/config/command/proxy/tls/command.go index 9467316e39..edd19d62bc 100644 --- a/internal/security/config/command/proxy/tls/command.go +++ b/internal/security/config/command/proxy/tls/command.go @@ -7,14 +7,24 @@ package tls import ( + "bytes" + "encoding/json" "flag" "fmt" + "io/ioutil" + "net/http" + "net/url" "os" "strings" + "github.com/edgexfoundry/edgex-go/internal" + "github.com/edgexfoundry/edgex-go/internal/security/config/command/proxy/common" "github.com/edgexfoundry/edgex-go/internal/security/config/interfaces" "github.com/edgexfoundry/edgex-go/internal/security/proxy/config" + "github.com/edgexfoundry/edgex-go/internal/security/secretstoreclient" + bootstrapConfig "github.com/edgexfoundry/go-mod-bootstrap/config" + "github.com/edgexfoundry/go-mod-core-contracts/clients" "github.com/edgexfoundry/go-mod-core-contracts/clients/logger" ) @@ -24,6 +34,7 @@ const ( type cmd struct { loggingClient logger.LoggingClient + client internal.HttpCaller configuration *config.ConfigurationStruct certificatePath string privateKeyPath string @@ -36,6 +47,7 @@ func NewCommand( cmd := cmd{ loggingClient: lc, + client: secretstoreclient.NewRequestor(lc).Insecure(), configuration: configuration, } var dummy string @@ -61,9 +73,172 @@ func NewCommand( } func (c *cmd) Execute() (statusCode int, err error) { - fmt.Println("TODO: Configure inbound TLS certificate.") + fmt.Println("Configure inbound proxy TLS certificate.") fmt.Printf("--incert %s\n", c.certificatePath) fmt.Printf("--inkey %s\n", c.privateKeyPath) - err = fmt.Errorf("tls command is unimplemented") - return interfaces.StatusCodeExitWithError, err + + if err := c.uploadProxyTlsCert(); err != nil { + return interfaces.StatusCodeExitWithError, err + } + + return +} + +func (c *cmd) readCertKeyPairFromFiles() (*bootstrapConfig.CertKeyPair, error) { + certPem, err := ioutil.ReadFile(c.certificatePath) + if err != nil { + return nil, fmt.Errorf("Failed to read TLS certificate from file %s: %w", c.certificatePath, err) + } + prvKey, err := ioutil.ReadFile(c.privateKeyPath) + if err != nil { + return nil, fmt.Errorf("Failed to read private key from file %s: %w", c.privateKeyPath, err) + } + return &bootstrapConfig.CertKeyPair{Cert: string(certPem), Key: string(prvKey)}, nil +} + +func (c *cmd) uploadProxyTlsCert() error { + // try to read both files and make sure they are existing + certKeyPair, err := c.readCertKeyPairFromFiles() + if err != nil { + return err + } + + // to see if any proxy certificates already exists + // if yes, then delete them all first + // and then upload the new TLS certificate + parsedCertData, err := c.listKongTLSCertificates() + if err != nil { + return err + } + + c.loggingClient.Debug(fmt.Sprintf("number of existing Kong tls certs = %d", len(parsedCertData))) + + if len(parsedCertData) > 0 { + // delete the existing certs first + // ideally, it should only have one certificate to be deleted + // Disclaimer: Kong TLS certificate should only be uploaded via secret-config utility + for _, certMap := range parsedCertData { + certId := fmt.Sprintf("%s", certMap["id"]) + if err := c.deleteKongTLSCertificateById(certId); err != nil { + return err + } + } + } + + // post the certKeyPair as the new one + if err := c.postKongTLSCertificate(certKeyPair); err != nil { + return err + } + + return nil +} + +func (c *cmd) listKongTLSCertificates() ([]map[string]interface{}, error) { + // list certificates first to see if any already exists + certKongURL := strings.Join([]string{c.configuration.KongURL.GetProxyBaseURL(), "certificates"}, "/") + c.loggingClient.Info(fmt.Sprintf("list tls certificates on the endpoint of %s", certKongURL)) + req, err := http.NewRequest(http.MethodGet, certKongURL, http.NoBody) + if err != nil { + return nil, fmt.Errorf("Failed to prepare request to list Kong tls certs: %w", err) + } + resp, err := c.client.Do(req) + if err != nil { + return nil, fmt.Errorf("Failed to send request to list Kong tls certs: %w", err) + } + defer resp.Body.Close() + + responseBody, err := ioutil.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("Failed to read response body to list Kong tls certs: %w", err) + } + + var parsedResponse map[string]interface{} + + switch resp.StatusCode { + case http.StatusOK: + if err := json.NewDecoder(bytes.NewReader(responseBody)).Decode(&parsedResponse); err != nil { + return nil, fmt.Errorf("Unable to parse response from list certificate: %w", err) + } + default: + return nil, fmt.Errorf("List Kong tls certificates request failed with code: %d", resp.StatusCode) + } + + var jsonData []byte + jsonData, err = json.Marshal(parsedResponse["data"]) + if err != nil { + return nil, fmt.Errorf("Failed to json marshal parsed response data: %w", err) + } + + outputData := fmt.Sprintf("%s", jsonData) + + // the list certificate get API returns the array of certificates + var parsedCertData []map[string]interface{} + if err := json.NewDecoder(bytes.NewReader([]byte(outputData))).Decode(&parsedCertData); err != nil { + return nil, fmt.Errorf("Unable to parse response for parsed cert data: %w", err) + } + + return parsedCertData, nil +} + +func (c *cmd) deleteKongTLSCertificateById(certId string) error { + delCertKongURL := strings.Join([]string{c.configuration.KongURL.GetProxyBaseURL(), + "certificates", certId}, "/") + c.loggingClient.Info(fmt.Sprintf("deleting tls certificate on the endpoint of %s", delCertKongURL)) + req, err := http.NewRequest(http.MethodDelete, delCertKongURL, http.NoBody) + if err != nil { + return fmt.Errorf("Failed to prepare request to delete Kong tls cert: %w", err) + } + resp, err := c.client.Do(req) + if err != nil { + return fmt.Errorf("Failed to send request to delete Kong tls cert: %w", err) + } + defer resp.Body.Close() + + switch resp.StatusCode { + case http.StatusNoContent: + c.loggingClient.Info("Successfully deleted Kong tls cert") + case http.StatusNotFound: + // not able to find this certId but should be ok to proceed and post a new certificate + c.loggingClient.Warn(fmt.Sprintf("Unable to delete Kong tls cert because the certificate Id %s not found", certId)) + default: + return fmt.Errorf("Delete Kong tls certificate request failed with code: %d", resp.StatusCode) + } + return nil +} + +func (c *cmd) postKongTLSCertificate(certKeyPair *bootstrapConfig.CertKeyPair) error { + postCertKongURL := strings.Join([]string{c.configuration.KongURL.GetProxyBaseURL(), + "certificates"}, "/") + c.loggingClient.Info(fmt.Sprintf("posting tls certificate on the endpoint of %s", postCertKongURL)) + + form := url.Values{ + "cert": []string{certKeyPair.Cert}, + "key": []string{certKeyPair.Key}, + } + formVal := form.Encode() + req, err := http.NewRequest(http.MethodPost, postCertKongURL, strings.NewReader(formVal)) + if err != nil { + return fmt.Errorf("Failed to prepare request to post Kong tls cert: %w", err) + } + + req.Header.Add(clients.ContentType, common.UrlEncodedForm) + resp, err := c.client.Do(req) + if err != nil { + return fmt.Errorf("Failed to send request to post Kong tls cert: %w", err) + } + defer resp.Body.Close() + responseBody, err := ioutil.ReadAll(resp.Body) + if err != nil { + return fmt.Errorf("Failed to read response body: %v", err) + } + + switch resp.StatusCode { + case http.StatusCreated: + c.loggingClient.Info("Successfully posted Kong tls cert") + case http.StatusBadRequest: + return fmt.Errorf("BadRequest as unable to post Kong tls cert due to error: %v", responseBody) + default: + return fmt.Errorf("Post Kong tls certificate request failed with code: %d", resp.StatusCode) + } + return nil } diff --git a/internal/security/config/command/proxy/tls/command_test.go b/internal/security/config/command/proxy/tls/command_test.go index a575939e3c..cf49469395 100644 --- a/internal/security/config/command/proxy/tls/command_test.go +++ b/internal/security/config/command/proxy/tls/command_test.go @@ -7,6 +7,12 @@ package tls import ( + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strconv" "testing" "github.com/edgexfoundry/edgex-go/internal/security/config/interfaces" @@ -18,6 +24,12 @@ import ( "github.com/stretchr/testify/require" ) +const ( + emptyCertCase = iota + oneCertCase + twoOrMoreCertCase +) + // TestTLSBagArguments tests command line errors func TestTLSBagArguments(t *testing.T) { // Arrange @@ -27,6 +39,7 @@ func TestTLSBagArguments(t *testing.T) { {}, // missing arg --in {"-badarg"}, // invalid arg {"--incert", "somefile"}, // missing --inkey + {"--inkey", "keyfile"}, // missing --incert } for _, args := range badArgTestcases { @@ -40,21 +53,199 @@ func TestTLSBagArguments(t *testing.T) { } // TestTLSErrorStub tests the tls error stub -func TestTLSErrorStub(t *testing.T) { +func TestTLSErrorFileNotFound(t *testing.T) { + // Arrange + lc := logger.MockLogger{} + config := &config.ConfigurationStruct{} + fileNotFoundTestcases := [][]string{ + {"--incert", "missingcertificate", "--inkey", "missingprivatekey"}, // both files missing + {"--incert", "testdata/testCert.pem", "--inkey", "missingprivatekey"}, // key file missing + {"--incert", "missingcertificate", "--inkey", "testdata/testCert.prkey"}, // both files missing + } + + for _, args := range fileNotFoundTestcases { + // Act + command, err := NewCommand(lc, config, args) + require.NoError(t, err) + code, err := command.Execute() + + // Assert + require.Error(t, err) + require.Equal(t, interfaces.StatusCodeExitWithError, code) + } +} + +func TestTLSAddNewCertificate(t *testing.T) { // Arrange lc := logger.MockLogger{} config := &config.ConfigurationStruct{} - args := []string{ - "--incert", "somecertificate", - "--inkey", "someprivatekey", + + tests := []struct { + name string + certCase int + listCertOk bool + deleteCertOk bool + postCertOk bool + expectedErr bool + }{ + {"Good: Add new kong cert when cert in server is empty", emptyCertCase, true, true, true, false}, + {"Good: Add new kong cert when one cert is in server", oneCertCase, true, true, true, false}, + {"Good: Add new kong cert when two or more cert is in server", twoOrMoreCertCase, true, true, true, false}, + {"Ok: Add new kong cert when delete certificate API failed but list cert being empty", emptyCertCase, true, false, true, false}, + {"Bad: Add new kong cert when list certificate API failed", oneCertCase, false, true, true, true}, + {"Bad: Add new kong cert when delete certificate API failed", oneCertCase, true, false, true, true}, + {"Bad: Add new kong cert when post certificate API failed", oneCertCase, true, true, false, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := getTlsCertificateTestServer(tt.certCase, tt.listCertOk, tt.deleteCertOk, tt.postCertOk, t) + defer ts.Close() + + tsURL, err := url.Parse(ts.URL) + require.NoError(t, err) + + config.KongURL.Server = tsURL.Hostname() + config.KongURL.AdminPort, _ = strconv.Atoi(tsURL.Port()) + + args := []string{ + "--incert", "testdata/testCert.pem", "--inkey", "testdata/testCert.prkey", + } + + // Act + command, err := NewCommand(lc, config, args) + require.NoError(t, err) + code, err := command.Execute() + + // Assert + if tt.expectedErr { + require.Error(t, err) + require.Equal(t, interfaces.StatusCodeExitWithError, code) + } else { + require.NoError(t, err) + require.Equal(t, interfaces.StatusCodeExitNormal, code) + } + }) } +} + +func getTlsCertificateTestServer(listCertCase int, listCertOk bool, deleteCertOk bool, postCertOk bool, + t *testing.T) *httptest.Server { + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + urlPath := r.URL.EscapedPath() + switch r.Method { + case http.MethodGet: + if urlPath == "/certificates" { + if listCertOk { + w.WriteHeader(http.StatusOK) + } else { + w.WriteHeader(http.StatusBadRequest) + } + var jsonResponse map[string]interface{} + switch listCertCase { + default: + case emptyCertCase: + jsonResponse = map[string]interface{}{ + "data": []string{}, + "next": nil, + } + case oneCertCase: + jsonResponse = map[string]interface{}{ + "data": []map[string]interface{}{ + { + "id": "fake-cert-id-01", + "createdAt": 1425366534, + "cert": "-----BEGIN CERTIFICATE-----...1E0MEFE=-----END CERTIFICATE-----", + "key": "-----BEGIN PRIVATE KEY-----...xmS8qXA==-----END PRIVATE KEY-----", + "snis": []string{}, + }, + }, + "next": "https://localhost:8001/certificates?offset=xxxxxxxxxxx", + } + case twoOrMoreCertCase: + jsonResponse = map[string]interface{}{ + "data": []map[string]interface{}{ + { + "id": "fake-cert-id-01", + "createdAt": 1425366534, + "cert": "-----BEGIN CERTIFICATE-----...1E0MEFE=-----END CERTIFICATE-----", + "key": "-----BEGIN PRIVATE KEY-----...xmS8qXA==-----END PRIVATE KEY-----", + "snis": []string{}, + }, + { + "id": "fake-cert-id-02", + "createdAt": 1438366534, + "cert": "-----BEGIN CERTIFICATE-----...j5GO0XQ=-----END CERTIFICATE-----", + "key": "-----BEGIN PRIVATE KEY-----...XtEiYK==-----END PRIVATE KEY-----", + "snis": []string{}, + }, + }, + "next": "https://localhost:8001/certificates?offset=xxxxxxxxxxx", + } + } - // Act - command, err := NewCommand(lc, config, args) - require.NoError(t, err) - code, err := command.Execute() + if respErr := json.NewEncoder(w).Encode(jsonResponse); respErr != nil { + t.Fatalf("Unexpected error %v", respErr) + } + } + case http.MethodPost: + if urlPath == "/certificates" { + if postCertOk { + w.WriteHeader(http.StatusCreated) + } else { + w.WriteHeader(http.StatusBadRequest) + } + var jsonResponse map[string]interface{} + switch listCertCase { + default: + case emptyCertCase: + jsonResponse = map[string]interface{}{ + "data": []map[string]interface{}{ + { + "id": "fake-cert-id-01", + "createdAt": 1425366534, + "cert": "-----BEGIN CERTIFICATE-----...1E0MEFE=-----END CERTIFICATE-----", + "key": "-----BEGIN PRIVATE KEY-----...xmS8qXA==-----END PRIVATE KEY-----", + "snis": []string{}, + }, + }, + "next": "https://localhost:8001/certificates?offset=xxxxxxxxxxx", + } + case oneCertCase: + jsonResponse = map[string]interface{}{ + "data": []map[string]interface{}{ + { + "id": "fake-cert-id-01", + "createdAt": 1425366534, + "cert": "-----BEGIN CERTIFICATE-----...1E0MEFE=-----END CERTIFICATE-----", + "key": "-----BEGIN PRIVATE KEY-----...xmS8qXA==-----END PRIVATE KEY-----", + "snis": []string{}, + }, + { + "id": "fake-cert-id-02", + "createdAt": 1438366534, + "cert": "-----BEGIN CERTIFICATE-----...j5GO0XQ=-----END CERTIFICATE-----", + "key": "-----BEGIN PRIVATE KEY-----...XtEiYK==-----END PRIVATE KEY-----", + "snis": []string{}, + }, + }, + "next": "https://localhost:8001/certificates?offset=xxxxxxxxxxx", + } + } - // Assert - require.Error(t, err) - require.Equal(t, interfaces.StatusCodeExitWithError, code) + if respErr := json.NewEncoder(w).Encode(jsonResponse); respErr != nil { + t.Fatalf("Unexpected error %v", respErr) + } + } + case http.MethodDelete: + if listCertCase > emptyCertCase && deleteCertOk && + (urlPath == "/certificates/fake-cert-id-01" || urlPath == "/certificates/fake-cert-id-02") { + w.WriteHeader(http.StatusNoContent) + } else { // other case with non-existing certificate id + w.WriteHeader(http.StatusBadRequest) + } + default: + t.Fatal(fmt.Sprintf("Unexpected http method %s call to URL %s", r.Method, urlPath)) + } + })) } diff --git a/internal/security/config/command/proxy/tls/testdata/testCert.pem b/internal/security/config/command/proxy/tls/testdata/testCert.pem new file mode 100644 index 0000000000..81c3775404 --- /dev/null +++ b/internal/security/config/command/proxy/tls/testdata/testCert.pem @@ -0,0 +1,32 @@ +-----BEGIN CERTIFICATE----- +MIIFlTCCA32gAwIBAgIJAMQkV3m0F4jHMA0GCSqGSIb3DQEBCwUAMGExCzAJBgNV +BAYTAlVTMQswCQYDVQQIDAJBWjERMA8GA1UEBwwIQ2hhbmRsZXIxDjAMBgNVBAoM +BUxvY2FsMQ0wCwYDVQQLDARIb21lMRMwEQYDVQQDDAplZGdleC1rb25nMB4XDTIw +MTIwNDAxNTA1OVoXDTIxMTIwNDAxNTA1OVowYTELMAkGA1UEBhMCVVMxCzAJBgNV +BAgMAkFaMREwDwYDVQQHDAhDaGFuZGxlcjEOMAwGA1UECgwFTG9jYWwxDTALBgNV +BAsMBEhvbWUxEzARBgNVBAMMCmVkZ2V4LWtvbmcwggIiMA0GCSqGSIb3DQEBAQUA +A4ICDwAwggIKAoICAQC93eYKSZ3RHeMjJkNb08wFWflN5NAGDmckwzc03XRpbNrg +XS6SK7Vp9J6klfIRF1K+wBmBMrpLW54mujaVd8ZqNta8Aj3OQUHz1P/uFpBeIhX1 +Ou+i4QZnYAMFdOZwTGI0Zi6IYUe6Q5pK2gN10KZp1CrVaeBflYCCC8sX0AhuB53f +Ijc2R0s9/gTzNIWmXao3XK7KnbBf25wj5oceTetpvTTOZRU+8sdvVyoiIc10Kix6 +2xYU4sPfyb4584Di/KK6k5vjhOrXhqVZ4jZ0rzynnkVeq5JXq0sAvSVL2z4hGsb6 +ed4HwZ0jNVMTd52VIgc/U38rnnlihbqInosfaP90rnIAok/FSJgF1EdQqzY7JW8k +CAfHXJkJS7YlMzdONHrsV5DHYbFr86APGCl4sXgso+i/aJTFOKFj3eTOW33WZmJX +YvF0H7y0UVzU33hZPBI1q5lr5zR4joIk011YXx/XhPrlGiO+WLOAslKgKEn3+yTp +VAQIZEhCNUOpGCzJRIN3CQPpYFByRUPkkJaqXBQr6B4z+OqAT/BVM754kLi7DoF+ +GeTsZoRzZXAYc1W5rjdNI5hmiMH+XKAic+maHK5vHYPc4XX7o6OhqMnggQ5gbaoE +3mo3ym8+Rj4f4tkpk8xiHUl5FHDF3p3mtqofNBN+1axiBBsP1xHoNEzI2IZhpQID +AQABo1AwTjAdBgNVHQ4EFgQUU4Dd+z+fWbEG50Cfa5y8CboYYc0wHwYDVR0jBBgw +FoAUU4Dd+z+fWbEG50Cfa5y8CboYYc0wDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0B +AQsFAAOCAgEAIqoY9aAvJvXTmNEwOQq/vuxmGie8+a6BWdkzz+NXLXbGv6DbNDBB +XqgiNrxvbEw4WJCpJ4+O28EZ5hC1rCR+4yoomdtk0Q6tS9EoRssrMxIXHJGGbyPL +fkxzMvApn9M8dffWqmXKjwXVcqEUP2bYa/oVwxXSytZhF9u88jt2hgkniu2HFJG9 +9L0GlKWvhjW7LAYGK0kFZsgDXyRXgxgBZeDtwoCIJzayJkgepOD+MxJeHGMPPeCt +0McUvXNqMiWyrtI9t9xDQs13igGp5q3peITZ7J8wi/IXO3Yb5OFHQAnX9dSNb529 +Eiwb2Dam2RkgAu1OFWOWVmIRfLUlFpoumEwmB8+eCuAg344SOlkHNpccsWwVKUUH +jnoUVrMyslJamtbJ9uN+Mg/X83tNq8pspAj1ekWjb6PWiJJiqNSmw3ULyZFkJk66 +aTXgRPqGA90q7xQIZvPdllFXZVG+qoN0+KhQ4Um4hLwBbraHweBjMjCX+raouQ7X +uFk6aNSd4r9H5hOm8+8bGG3ak5tM14xtFb+B4yzDpKDOqcPxB81uCN32WJwDbJPW +aw5+a20q70W9juIQC7Qkx1hY90PAr9Anhc6vYHOfrRd6FXU9HBRsips/IHgXYETv +ospIvh3VcnZFMO2RWaPSPG9j5GO0XQbd0H3HL1aokGM1lKsW1E0MEFE= +-----END CERTIFICATE----- diff --git a/internal/security/config/command/proxy/tls/testdata/testCert.prkey b/internal/security/config/command/proxy/tls/testdata/testCert.prkey new file mode 100644 index 0000000000..6d5f892879 --- /dev/null +++ b/internal/security/config/command/proxy/tls/testdata/testCert.prkey @@ -0,0 +1,52 @@ +-----BEGIN PRIVATE KEY----- +MIIJQgIBADANBgkqhkiG9w0BAQEFAASCCSwwggkoAgEAAoICAQC93eYKSZ3RHeMj +JkNb08wFWflN5NAGDmckwzc03XRpbNrgXS6SK7Vp9J6klfIRF1K+wBmBMrpLW54m +ujaVd8ZqNta8Aj3OQUHz1P/uFpBeIhX1Ou+i4QZnYAMFdOZwTGI0Zi6IYUe6Q5pK +2gN10KZp1CrVaeBflYCCC8sX0AhuB53fIjc2R0s9/gTzNIWmXao3XK7KnbBf25wj +5oceTetpvTTOZRU+8sdvVyoiIc10Kix62xYU4sPfyb4584Di/KK6k5vjhOrXhqVZ +4jZ0rzynnkVeq5JXq0sAvSVL2z4hGsb6ed4HwZ0jNVMTd52VIgc/U38rnnlihbqI +nosfaP90rnIAok/FSJgF1EdQqzY7JW8kCAfHXJkJS7YlMzdONHrsV5DHYbFr86AP +GCl4sXgso+i/aJTFOKFj3eTOW33WZmJXYvF0H7y0UVzU33hZPBI1q5lr5zR4joIk +011YXx/XhPrlGiO+WLOAslKgKEn3+yTpVAQIZEhCNUOpGCzJRIN3CQPpYFByRUPk +kJaqXBQr6B4z+OqAT/BVM754kLi7DoF+GeTsZoRzZXAYc1W5rjdNI5hmiMH+XKAi +c+maHK5vHYPc4XX7o6OhqMnggQ5gbaoE3mo3ym8+Rj4f4tkpk8xiHUl5FHDF3p3m +tqofNBN+1axiBBsP1xHoNEzI2IZhpQIDAQABAoICAEm+G5C3UvJbd1K5k8PgmQcj +EO8uBZW0ll7y60g/Eu23d7NYvbPMAiPq/IrUKjZ1JpArsDw75ZtLNDu6a2TWJlwu +fGx9OmAXfHQlsFlarivBdWHPyC8s0v3njoTaCH5pTGMppL0Xe3Jeu1iDIIDrzxaf +bxuAicLWIBliVzkidYO2tZxqf6M3QYQMWwPSzG33RxtFhiXv5Qb2Fsjiymv8u4hQ +EmhvXjcauFcINbcYBeIuVyRIX2UxRf5vLtD3QIHouZNZrVxKzaN0CljEwX3Eafx4 +Pl0sqiQexXy9+fRibNkSu2GhX5kHm+6G0YNoG+5GTyZLWcMpQ9/+LJrPaR2D/GM6 +9v8xFjkd2yzSFEvPQ82F7RCTWB0yuAqYA1EveJJ1J0acq+3FAVQlkHSPQGpin21a +75VJNgMOEsKNo6HvDa84ZXsjfJBPr2E97JRQb41ZvlLwaMqVS/F8E26nZ5zCgYjz +WafTCudNQve1B8SPuS0taOLg1u6CAEcQWZvHzUCGsOcm/f1K9VDaUnoTcmP9mVJW +N1NOXwa/2pvmLTss6bgrSuqqUjZMVMs6zzLIRhS07+37V1APj2/Gv1RLalE0mKXK +5yLqJPbTgL79H6+kdx59I0aKjcjv9YIrf7PXCEMm+wId3hPfeCxl1/LbhwOmQ0q2 +atFeBi4j6XZdWd2Pe4ZBAoIBAQD1csUUJNoSU4ro5eMXviiEiNXFCvaPCNsvBRYO +U3UNHSYqrkbwQQmg+FEAz4A7HWImDTGxBDcV6HAlrlpy2t374M6HmtQWeN+V9XqQ +jq8PaeD5uEhoppRF/k3+ODgjzYMLcCB+zOiZLHeecIM/pZy52BDP/mbgNAlVsPWK +ENl1lOIvh5NGiQkauwWqVxrGq3+oSJcCXivqP/FKZT1Yt/BcgaT/Y4ieNqpAxdy1 +9bxQl5ktjp/z0Ei2XwI4TOcyn0hKTq2xyYK+puijCrQVyN7HtLl6VwvC9+GdTVev +gjFLr823u3smrq3KAp8NgWw4YeVEfwS6n+ShL0sUfiuvJfYJAoIBAQDGB3AYGaNh +rBBrZkbcESNJRiW0z0+JlUar7e8xqlIlhLf0atW4WjnnRt5rI7mNm/FZgDsEeGUP +fqIXDT30REmjcZabA7bdKBA34BlY/deKS3l1CP78OcjEIX2yJIp5+dnOa4nDpEsT +XOfO1a+mloIr0ShQRMw1yNyZzva5XZFZphbutyD33AKa14PmtMuLs0yRe8P38+3n +ZSnpswKDkKlVvg2hd2Lzx+gVaekdIefkoJ53Pfq7AZtbjq2C2i7utQioUFdNXUAv +t7ltEbyZjD2EY5tZP/j7B+euxm1VwUY2tfal9cQvFx7Mc5tNk/WHqZ/n+hgao3T4 +YN795DAWCRW9AoIBAQC6c0yyWKwvb7b95GP2DUXKKAf7frB9R0T5Guv+RfeM1q+/ +jFPm/gDKftdJvlaykUeVkEBHL+SIh+FpPmEHDqvCQqug50RlLbzqtWc9mKXzF4MH +L64RkTjhUqT8kMhUqjD551tH93BZqyXS/bU2DCBaLH/ZAHA1sFHG/n4HO3V1lHud +j2eZFVANNjS/iRuV/4Eh3MKZ0d+aOqiY0v8e/dtg6jrkpj+JsAz011kqAFnk/sQl +j0qCc55IqzOMgR6/na/Ugp//hgwDt9bQw7i5M7XIDsk23hjZKmQAklghwsyfqhSj +lY+feuIZqpQYNlB3JCZ9Od/lxMBwkPR/xLh6fGPBAoIBAFGtia93C6tig9c4dSuZ +qfs0AqNkCoNN3btWRR0wCffNmO4oDoSeOlnJIj4AmyzsUAzBVhZO7igI7CQj4xTY +AaN3W04OpyLqvl29gdbxxDAXVz5NepZf4w36XlTWu4L56bs5IbZfElQnMrld67gD +Rid6em05Ix1f/pU2Bw+Hp0bZuYW2ZNO2nCBvmcjTc0zopEExUi/4HX0efb4Vhojr +ZtylqguaKWcxYelLKMpKTNJA6Xt3Re5SCFkoLSrWgRsV8j8x/AA94RaNad6xvR97 +93eeednDCBfKN+Yfk8MWF7bDMLtc9hESMTLU0A6cY14UOY892SlLmBhGJFOGBctg +7eUCggEAIvfgOf6Llw/v2i+g9AgzFev6ETZ53WO0rd5pJNtI6ELqCfj6iRzaIm6G +zR/ZkCZo4ScPkCV92UFM2LCfYK1sRM2qNnHOVZWQzNDIJCWXhWWWvwHYo9ocbYdJ +xBJEFnJvohfzAj0XmU5yvgNNhyOyC7+hBNvfo4LjzaKFRGkeWKYqFbBcx8NpRNwY +8B4V7E5HOEuORouDj8zF8k01ekjh6YWGys5AM8f91BuJz4GxQfVSZrsYwubEykOc +FIeqdtF3saTRXtEiYKDSR6iVxrpG2omTxbQZH0jdNA9sNjXUumPhkYnEpqfc6HdR +J8vwM9u4cs0tANWCY9hb9RVxmS8qXA== +-----END PRIVATE KEY----- From aab1748c5da10e2115d4bf3fab6e2bce6cac0e1a Mon Sep 17 00:00:00 2001 From: Jim Wang Date: Tue, 8 Dec 2020 10:07:43 -0700 Subject: [PATCH 2/5] feat(security): Implement secrets-configure proxy tls Add SNIS association with the user specified certificates. Also addressed PR feedback. Signed-off-by: Jim Wang --- .../config/command/proxy/common/common.go | 1 + .../config/command/proxy/tls/command.go | 75 ++++++++++++++----- .../config/command/proxy/tls/command_test.go | 41 ++++++++-- 3 files changed, 93 insertions(+), 24 deletions(-) diff --git a/internal/security/config/command/proxy/common/common.go b/internal/security/config/command/proxy/common/common.go index d493578484..c5e8ce0708 100644 --- a/internal/security/config/command/proxy/common/common.go +++ b/internal/security/config/command/proxy/common/common.go @@ -7,5 +7,6 @@ package common const ( + // UrlEncodedForm is the content type for web form data being encoded UrlEncodedForm = "application/x-www-form-urlencoded" ) diff --git a/internal/security/config/command/proxy/tls/command.go b/internal/security/config/command/proxy/tls/command.go index edd19d62bc..7e76875d55 100644 --- a/internal/security/config/command/proxy/tls/command.go +++ b/internal/security/config/command/proxy/tls/command.go @@ -30,14 +30,17 @@ import ( const ( CommandName = "tls" + + builtinSNISList = "localhost,kong" ) type cmd struct { - loggingClient logger.LoggingClient - client internal.HttpCaller - configuration *config.ConfigurationStruct - certificatePath string - privateKeyPath string + loggingClient logger.LoggingClient + client internal.HttpCaller + configuration *config.ConfigurationStruct + certificatePath string + privateKeyPath string + serveNameIndictionsList string } func NewCommand( @@ -57,6 +60,8 @@ func NewCommand( flagSet.StringVar(&cmd.certificatePath, "incert", "", "Path to PEM-encoded leaf certificate") flagSet.StringVar(&cmd.privateKeyPath, "inkey", "", "Path to PEM-encoded private key") + flagSet.StringVar(&cmd.serveNameIndictionsList, "snis", "", + "[Optional] comma-separated extra server name indications list to associate with this certificate") err := flagSet.Parse(args) if err != nil { @@ -73,9 +78,6 @@ func NewCommand( } func (c *cmd) Execute() (statusCode int, err error) { - fmt.Println("Configure inbound proxy TLS certificate.") - fmt.Printf("--incert %s\n", c.certificatePath) - fmt.Printf("--inkey %s\n", c.privateKeyPath) if err := c.uploadProxyTlsCert(); err != nil { return interfaces.StatusCodeExitWithError, err @@ -106,20 +108,19 @@ func (c *cmd) uploadProxyTlsCert() error { // to see if any proxy certificates already exists // if yes, then delete them all first // and then upload the new TLS certificate - parsedCertData, err := c.listKongTLSCertificates() + existingCertIDs, err := c.listKongTLSCertificates() if err != nil { return err } - c.loggingClient.Debug(fmt.Sprintf("number of existing Kong tls certs = %d", len(parsedCertData))) + c.loggingClient.Debug(fmt.Sprintf("number of existing Kong tls certs = %d", len(existingCertIDs))) - if len(parsedCertData) > 0 { + if len(existingCertIDs) > 0 { // delete the existing certs first // ideally, it should only have one certificate to be deleted // Disclaimer: Kong TLS certificate should only be uploaded via secret-config utility - for _, certMap := range parsedCertData { - certId := fmt.Sprintf("%s", certMap["id"]) - if err := c.deleteKongTLSCertificateById(certId); err != nil { + for _, certID := range existingCertIDs { + if err := c.deleteKongTLSCertificateById(certID); err != nil { return err } } @@ -133,7 +134,7 @@ func (c *cmd) uploadProxyTlsCert() error { return nil } -func (c *cmd) listKongTLSCertificates() ([]map[string]interface{}, error) { +func (c *cmd) listKongTLSCertificates() ([]string, error) { // list certificates first to see if any already exists certKongURL := strings.Join([]string{c.configuration.KongURL.GetProxyBaseURL(), "certificates"}, "/") c.loggingClient.Info(fmt.Sprintf("list tls certificates on the endpoint of %s", certKongURL)) @@ -177,7 +178,12 @@ func (c *cmd) listKongTLSCertificates() ([]map[string]interface{}, error) { return nil, fmt.Errorf("Unable to parse response for parsed cert data: %w", err) } - return parsedCertData, nil + certIDs := make([]string, len(parsedCertData)) + for i, certMap := range parsedCertData { + certIDs[i] = fmt.Sprintf("%s", certMap["id"]) + } + + return certIDs, nil } func (c *cmd) deleteKongTLSCertificateById(certId string) error { @@ -214,7 +220,9 @@ func (c *cmd) postKongTLSCertificate(certKeyPair *bootstrapConfig.CertKeyPair) e form := url.Values{ "cert": []string{certKeyPair.Cert}, "key": []string{certKeyPair.Key}, + "snis": getServerNameIndications(c.serveNameIndictionsList), } + formVal := form.Encode() req, err := http.NewRequest(http.MethodPost, postCertKongURL, strings.NewReader(formVal)) if err != nil { @@ -236,9 +244,42 @@ func (c *cmd) postKongTLSCertificate(certKeyPair *bootstrapConfig.CertKeyPair) e case http.StatusCreated: c.loggingClient.Info("Successfully posted Kong tls cert") case http.StatusBadRequest: - return fmt.Errorf("BadRequest as unable to post Kong tls cert due to error: %v", responseBody) + return fmt.Errorf("BadRequest as unable to post Kong tls cert due to error: %s", string(responseBody)) default: return fmt.Errorf("Post Kong tls certificate request failed with code: %d", resp.StatusCode) } return nil } + +func getServerNameIndications(snisList string) []string { + // this will parse out the internal default server name indications and extra ones from input if given + var snis []string + snis = append(snis, parseAndTrimSpaces(builtinSNISList)...) + + uniqueSnisMap := make(map[string]bool) + for _, s := range snis { + uniqueSnisMap[s] = true + } + + if len(snisList) > 0 { + splitSnis := parseAndTrimSpaces(snisList) + for _, sni := range splitSnis { + if _, exists := uniqueSnisMap[sni]; !exists { + uniqueSnisMap[sni] = true + snis = append(snis, sni) + } + } + } + return snis +} + +func parseAndTrimSpaces(commaSep string) (ret []string) { + splitStrs := strings.Split(commaSep, ",") + for _, s := range splitStrs { + trimmed := strings.TrimSpace(s) + if len(trimmed) > 0 { // effective only it is non-empty string + ret = append(ret, trimmed) + } + } + return +} diff --git a/internal/security/config/command/proxy/tls/command_test.go b/internal/security/config/command/proxy/tls/command_test.go index cf49469395..299cb00ee2 100644 --- a/internal/security/config/command/proxy/tls/command_test.go +++ b/internal/security/config/command/proxy/tls/command_test.go @@ -60,7 +60,7 @@ func TestTLSErrorFileNotFound(t *testing.T) { fileNotFoundTestcases := [][]string{ {"--incert", "missingcertificate", "--inkey", "missingprivatekey"}, // both files missing {"--incert", "testdata/testCert.pem", "--inkey", "missingprivatekey"}, // key file missing - {"--incert", "missingcertificate", "--inkey", "testdata/testCert.prkey"}, // both files missing + {"--incert", "missingcertificate", "--inkey", "testdata/testCert.prkey"}, // cert file missing } for _, args := range fileNotFoundTestcases { @@ -129,8 +129,35 @@ func TestTLSAddNewCertificate(t *testing.T) { } } +func TestGetServerNameIndications(t *testing.T) { + builtinSnis := []string{"localhost", "kong"} + tests := []struct { + name string + snisInputStr string + expectedSnis []string + }{ + {"Empty SNIS input", "", builtinSnis}, + {"One SNIS input", "test1.com", append(builtinSnis, "test1.com")}, + {"Two SNIS input", "test1.com,test2.com", append(builtinSnis, []string{"test1.com", "test2.com"}...)}, + {"Two or more SNIS with spaces", "test1.com, test2.com, test3.com", + append(builtinSnis, []string{"test1.com", "test2.com", "test3.com"}...)}, + {"Mixed with empty entries", ", test1.com, ", append(builtinSnis, "test1.com")}, + {"Equivalent empty entries", ",,", builtinSnis}, + {"Duplicate with internal entries", "kong, localhost", builtinSnis}, + {"Mixed with some duplicating internal entries", "kong, test1.com, test2.com,localhost,kong", + append(builtinSnis, []string{"test1.com", "test2.com"}...)}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.expectedSnis, getServerNameIndications(tt.snisInputStr)) + }) + } +} + func getTlsCertificateTestServer(listCertCase int, listCertOk bool, deleteCertOk bool, postCertOk bool, t *testing.T) *httptest.Server { + builtinSnis := []string{"localhost", "kong"} return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { urlPath := r.URL.EscapedPath() switch r.Method { @@ -157,7 +184,7 @@ func getTlsCertificateTestServer(listCertCase int, listCertOk bool, deleteCertOk "createdAt": 1425366534, "cert": "-----BEGIN CERTIFICATE-----...1E0MEFE=-----END CERTIFICATE-----", "key": "-----BEGIN PRIVATE KEY-----...xmS8qXA==-----END PRIVATE KEY-----", - "snis": []string{}, + "snis": builtinSnis, }, }, "next": "https://localhost:8001/certificates?offset=xxxxxxxxxxx", @@ -170,14 +197,14 @@ func getTlsCertificateTestServer(listCertCase int, listCertOk bool, deleteCertOk "createdAt": 1425366534, "cert": "-----BEGIN CERTIFICATE-----...1E0MEFE=-----END CERTIFICATE-----", "key": "-----BEGIN PRIVATE KEY-----...xmS8qXA==-----END PRIVATE KEY-----", - "snis": []string{}, + "snis": builtinSnis, }, { "id": "fake-cert-id-02", "createdAt": 1438366534, "cert": "-----BEGIN CERTIFICATE-----...j5GO0XQ=-----END CERTIFICATE-----", "key": "-----BEGIN PRIVATE KEY-----...XtEiYK==-----END PRIVATE KEY-----", - "snis": []string{}, + "snis": builtinSnis, }, }, "next": "https://localhost:8001/certificates?offset=xxxxxxxxxxx", @@ -206,7 +233,7 @@ func getTlsCertificateTestServer(listCertCase int, listCertOk bool, deleteCertOk "createdAt": 1425366534, "cert": "-----BEGIN CERTIFICATE-----...1E0MEFE=-----END CERTIFICATE-----", "key": "-----BEGIN PRIVATE KEY-----...xmS8qXA==-----END PRIVATE KEY-----", - "snis": []string{}, + "snis": builtinSnis, }, }, "next": "https://localhost:8001/certificates?offset=xxxxxxxxxxx", @@ -219,14 +246,14 @@ func getTlsCertificateTestServer(listCertCase int, listCertOk bool, deleteCertOk "createdAt": 1425366534, "cert": "-----BEGIN CERTIFICATE-----...1E0MEFE=-----END CERTIFICATE-----", "key": "-----BEGIN PRIVATE KEY-----...xmS8qXA==-----END PRIVATE KEY-----", - "snis": []string{}, + "snis": builtinSnis, }, { "id": "fake-cert-id-02", "createdAt": 1438366534, "cert": "-----BEGIN CERTIFICATE-----...j5GO0XQ=-----END CERTIFICATE-----", "key": "-----BEGIN PRIVATE KEY-----...XtEiYK==-----END PRIVATE KEY-----", - "snis": []string{}, + "snis": builtinSnis, }, }, "next": "https://localhost:8001/certificates?offset=xxxxxxxxxxx", From 0f334072fc1b34e624dc20b353904d004c4fda5b Mon Sep 17 00:00:00 2001 From: Jim Wang Date: Tue, 8 Dec 2020 12:16:07 -0700 Subject: [PATCH 3/5] docs(security): Update the secrets-config README Add user's guide documentation for a new optional argument of proxy tls subcommand: --snis [list of names in comma separated] Signed-off-by: Jim Wang --- cmd/secrets-config/README.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/cmd/secrets-config/README.md b/cmd/secrets-config/README.md index 22931660ee..b8adfb5b31 100644 --- a/cmd/secrets-config/README.md +++ b/cmd/secrets-config/README.md @@ -39,6 +39,15 @@ Proxy configuration commands (listed below) require access to the secret store m Path to TLS private key (PEM-encoded). + * **--snis** _comma_separated_list_for_server_names_ (optional) + + A comma separated extra server DNS names in addition to the built-in + server name indications. The built-in names are "localhost,kong". + These names will be associated with the user-provided certificate for Kong's TLS to use. + Based on the specification [RFC4366](https://tools.ietf.org/html/rfc4366): + "Currently, the only server names supported are DNS hostnames", + so the IP address-based input is not allowed. + * **adduser** Create an API gateway user using specified token type. Requires additional arguments: From 35953d63dbb6324954242bc6432706fab40e1eb5 Mon Sep 17 00:00:00 2001 From: Jim Wang Date: Tue, 8 Dec 2020 17:10:53 -0700 Subject: [PATCH 4/5] feat(security): Change the list certificates algorithm With Bryon's suggestion not using /certificates API, now it changes to /snis and only deletes those certificates associated with the snis names, including the buitlin ones. Signed-off-by: Jim Wang --- .../config/command/proxy/tls/command.go | 72 +++++++++++++------ .../config/command/proxy/tls/command_test.go | 68 +++++++++++++----- 2 files changed, 104 insertions(+), 36 deletions(-) diff --git a/internal/security/config/command/proxy/tls/command.go b/internal/security/config/command/proxy/tls/command.go index 7e76875d55..813b8a95e2 100644 --- a/internal/security/config/command/proxy/tls/command.go +++ b/internal/security/config/command/proxy/tls/command.go @@ -34,13 +34,18 @@ const ( builtinSNISList = "localhost,kong" ) +type certificateIDs []string + type cmd struct { loggingClient logger.LoggingClient client internal.HttpCaller configuration *config.ConfigurationStruct certificatePath string privateKeyPath string - serveNameIndictionsList string + serverNameIndicatorList string + + // states related to certificates and snis + serverNameToCertIDs map[string]certificateIDs } func NewCommand( @@ -60,7 +65,7 @@ func NewCommand( flagSet.StringVar(&cmd.certificatePath, "incert", "", "Path to PEM-encoded leaf certificate") flagSet.StringVar(&cmd.privateKeyPath, "inkey", "", "Path to PEM-encoded private key") - flagSet.StringVar(&cmd.serveNameIndictionsList, "snis", "", + flagSet.StringVar(&cmd.serverNameIndicatorList, "snis", "", "[Optional] comma-separated extra server name indications list to associate with this certificate") err := flagSet.Parse(args) @@ -135,22 +140,22 @@ func (c *cmd) uploadProxyTlsCert() error { } func (c *cmd) listKongTLSCertificates() ([]string, error) { - // list certificates first to see if any already exists - certKongURL := strings.Join([]string{c.configuration.KongURL.GetProxyBaseURL(), "certificates"}, "/") - c.loggingClient.Info(fmt.Sprintf("list tls certificates on the endpoint of %s", certKongURL)) + // list snis certificates association to see if any already exists + certKongURL := strings.Join([]string{c.configuration.KongURL.GetProxyBaseURL(), "snis"}, "/") + c.loggingClient.Info(fmt.Sprintf("list snis tls certificates on the endpoint of %s", certKongURL)) req, err := http.NewRequest(http.MethodGet, certKongURL, http.NoBody) if err != nil { - return nil, fmt.Errorf("Failed to prepare request to list Kong tls certs: %w", err) + return nil, fmt.Errorf("Failed to prepare request to list Kong snis tls certs: %w", err) } resp, err := c.client.Do(req) if err != nil { - return nil, fmt.Errorf("Failed to send request to list Kong tls certs: %w", err) + return nil, fmt.Errorf("Failed to send request to list Kong snis tls certs: %w", err) } defer resp.Body.Close() responseBody, err := ioutil.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("Failed to read response body to list Kong tls certs: %w", err) + return nil, fmt.Errorf("Failed to read response body to list Kong snis tls certs: %w", err) } var parsedResponse map[string]interface{} @@ -158,10 +163,10 @@ func (c *cmd) listKongTLSCertificates() ([]string, error) { switch resp.StatusCode { case http.StatusOK: if err := json.NewDecoder(bytes.NewReader(responseBody)).Decode(&parsedResponse); err != nil { - return nil, fmt.Errorf("Unable to parse response from list certificate: %w", err) + return nil, fmt.Errorf("Unable to parse response from list snis certificate: %w", err) } default: - return nil, fmt.Errorf("List Kong tls certificates request failed with code: %d", resp.StatusCode) + return nil, fmt.Errorf("List Kong tls snis certificates request failed with code: %d", resp.StatusCode) } var jsonData []byte @@ -170,17 +175,43 @@ func (c *cmd) listKongTLSCertificates() ([]string, error) { return nil, fmt.Errorf("Failed to json marshal parsed response data: %w", err) } - outputData := fmt.Sprintf("%s", jsonData) + // the list snis get API returns an array of snis-certs association + var parsedSnisCertData []map[string]interface{} + if err := json.NewDecoder(bytes.NewReader(jsonData)).Decode(&parsedSnisCertData); err != nil { + return nil, fmt.Errorf("Unable to parse response for parsed SNIS cert data: %w", err) + } + + c.serverNameToCertIDs = make(map[string]certificateIDs) + for _, sniCerts := range parsedSnisCertData { + // extract sni name and certificate id from parsed json data + sni := fmt.Sprintf("%s", sniCerts["name"]) + certID := fmt.Sprintf("%s", sniCerts["certificate"].(map[string]interface{})["id"]) + + if certIDs, exists := c.serverNameToCertIDs[sni]; !exists { + // initialize for a new sni name + c.serverNameToCertIDs[sni] = []string{certID} + } else { + // update the existing certificateID array + certIDs = append(certIDs, certID) + c.serverNameToCertIDs[sni] = certIDs + } + } - // the list certificate get API returns the array of certificates - var parsedCertData []map[string]interface{} - if err := json.NewDecoder(bytes.NewReader([]byte(outputData))).Decode(&parsedCertData); err != nil { - return nil, fmt.Errorf("Unable to parse response for parsed cert data: %w", err) + uniqueSnisFromInput := getServerNameIndicators(c.serverNameIndicatorList) + // collect the unique certificate ids that match the given snis from the input to be deleted + uniqueCertIDs := make(map[string]bool) + for _, sniFromInput := range uniqueSnisFromInput { + if certIDs, exists := c.serverNameToCertIDs[sniFromInput]; exists { + for _, certID := range certIDs { + uniqueCertIDs[certID] = true + } + } } - certIDs := make([]string, len(parsedCertData)) - for i, certMap := range parsedCertData { - certIDs[i] = fmt.Sprintf("%s", certMap["id"]) + // get the unique certificate ids + certIDs := make([]string, 0, len(uniqueCertIDs)) + for certID := range uniqueCertIDs { + certIDs = append(certIDs, certID) } return certIDs, nil @@ -220,7 +251,7 @@ func (c *cmd) postKongTLSCertificate(certKeyPair *bootstrapConfig.CertKeyPair) e form := url.Values{ "cert": []string{certKeyPair.Cert}, "key": []string{certKeyPair.Key}, - "snis": getServerNameIndications(c.serveNameIndictionsList), + "snis": getServerNameIndicators(c.serverNameIndicatorList), } formVal := form.Encode() @@ -251,7 +282,7 @@ func (c *cmd) postKongTLSCertificate(certKeyPair *bootstrapConfig.CertKeyPair) e return nil } -func getServerNameIndications(snisList string) []string { +func getServerNameIndicators(snisList string) []string { // this will parse out the internal default server name indications and extra ones from input if given var snis []string snis = append(snis, parseAndTrimSpaces(builtinSNISList)...) @@ -261,6 +292,7 @@ func getServerNameIndications(snisList string) []string { uniqueSnisMap[s] = true } + // sanitize the user given list if len(snisList) > 0 { splitSnis := parseAndTrimSpaces(snisList) for _, sni := range splitSnis { diff --git a/internal/security/config/command/proxy/tls/command_test.go b/internal/security/config/command/proxy/tls/command_test.go index 299cb00ee2..2f63dfae24 100644 --- a/internal/security/config/command/proxy/tls/command_test.go +++ b/internal/security/config/command/proxy/tls/command_test.go @@ -52,7 +52,7 @@ func TestTLSBagArguments(t *testing.T) { } } -// TestTLSErrorStub tests the tls error stub +// TestTLSErrorFileNotFound tests the tls error regarding file not found issues func TestTLSErrorFileNotFound(t *testing.T) { // Arrange lc := logger.MockLogger{} @@ -129,7 +129,7 @@ func TestTLSAddNewCertificate(t *testing.T) { } } -func TestGetServerNameIndications(t *testing.T) { +func TestGetServerNameIndicatros(t *testing.T) { builtinSnis := []string{"localhost", "kong"} tests := []struct { name string @@ -150,7 +150,7 @@ func TestGetServerNameIndications(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - require.Equal(t, tt.expectedSnis, getServerNameIndications(tt.snisInputStr)) + require.Equal(t, tt.expectedSnis, getServerNameIndicators(tt.snisInputStr)) }) } } @@ -162,7 +162,7 @@ func getTlsCertificateTestServer(listCertCase int, listCertOk bool, deleteCertOk urlPath := r.URL.EscapedPath() switch r.Method { case http.MethodGet: - if urlPath == "/certificates" { + if urlPath == "/snis" { if listCertOk { w.WriteHeader(http.StatusOK) } else { @@ -180,11 +180,20 @@ func getTlsCertificateTestServer(listCertCase int, listCertOk bool, deleteCertOk jsonResponse = map[string]interface{}{ "data": []map[string]interface{}{ { - "id": "fake-cert-id-01", + "id": "fake-snis-id-01", "createdAt": 1425366534, - "cert": "-----BEGIN CERTIFICATE-----...1E0MEFE=-----END CERTIFICATE-----", - "key": "-----BEGIN PRIVATE KEY-----...xmS8qXA==-----END PRIVATE KEY-----", - "snis": builtinSnis, + "name": builtinSnis[0], + "certificate": map[string]interface{}{ + "id": "fake-cert-id-01", + }, + }, + { + "id": "fake-snis-id-02", + "createdAt": 1425366534, + "name": builtinSnis[1], + "certificate": map[string]interface{}{ + "id": "fake-cert-id-01", + }, }, }, "next": "https://localhost:8001/certificates?offset=xxxxxxxxxxx", @@ -193,18 +202,45 @@ func getTlsCertificateTestServer(listCertCase int, listCertOk bool, deleteCertOk jsonResponse = map[string]interface{}{ "data": []map[string]interface{}{ { - "id": "fake-cert-id-01", + "id": "fake-snis-id-01", "createdAt": 1425366534, - "cert": "-----BEGIN CERTIFICATE-----...1E0MEFE=-----END CERTIFICATE-----", - "key": "-----BEGIN PRIVATE KEY-----...xmS8qXA==-----END PRIVATE KEY-----", - "snis": builtinSnis, + "name": builtinSnis[0], + "certificate": map[string]interface{}{ + "id": "fake-cert-id-01", + }, }, { - "id": "fake-cert-id-02", + "id": "fake-snis-id-02", "createdAt": 1438366534, - "cert": "-----BEGIN CERTIFICATE-----...j5GO0XQ=-----END CERTIFICATE-----", - "key": "-----BEGIN PRIVATE KEY-----...XtEiYK==-----END PRIVATE KEY-----", - "snis": builtinSnis, + "name": builtinSnis[0], + "certificate": map[string]interface{}{ + "id": "fake-cert-id-02", + }, + }, + { + "id": "fake-snis-id-03", + "createdAt": 1425366534, + "name": builtinSnis[1], + "certificate": map[string]interface{}{ + "id": "fake-cert-id-01", + }, + }, + { + "id": "fake-snis-id-04", + "createdAt": 1438366534, + "name": builtinSnis[1], + "certificate": map[string]interface{}{ + "id": "fake-cert-id-02", + }, + }, + // to make tests interesting, add another public cert somehow uploaded with different snis name + { + "id": "fake-snis-id-05", + "createdAt": 1438369534, + "name": "test.domain.com", + "certificate": map[string]interface{}{ + "id": "fake-cert-id-03", + }, }, }, "next": "https://localhost:8001/certificates?offset=xxxxxxxxxxx", From a41decdaea853968b8fbd73c5ef4065e301df088 Mon Sep 17 00:00:00 2001 From: Jim Wang Date: Wed, 9 Dec 2020 14:39:39 -0700 Subject: [PATCH 5/5] refactor(security): Use user-defined structs for json unmarshal User-defined structs defined locally and are used to do json unmarshal so that only decoding once Addressed per Lenny's request for changes. Signed-off-by: Jim Wang --- .../config/command/proxy/tls/command.go | 95 ++++++++++--------- .../config/command/proxy/tls/command_test.go | 2 +- 2 files changed, 49 insertions(+), 48 deletions(-) diff --git a/internal/security/config/command/proxy/tls/command.go b/internal/security/config/command/proxy/tls/command.go index 813b8a95e2..42ad89b938 100644 --- a/internal/security/config/command/proxy/tls/command.go +++ b/internal/security/config/command/proxy/tls/command.go @@ -43,9 +43,6 @@ type cmd struct { certificatePath string privateKeyPath string serverNameIndicatorList string - - // states related to certificates and snis - serverNameToCertIDs map[string]certificateIDs } func NewCommand( @@ -122,7 +119,6 @@ func (c *cmd) uploadProxyTlsCert() error { if len(existingCertIDs) > 0 { // delete the existing certs first - // ideally, it should only have one certificate to be deleted // Disclaimer: Kong TLS certificate should only be uploaded via secret-config utility for _, certID := range existingCertIDs { if err := c.deleteKongTLSCertificateById(certID); err != nil { @@ -139,7 +135,24 @@ func (c *cmd) uploadProxyTlsCert() error { return nil } -func (c *cmd) listKongTLSCertificates() ([]string, error) { +func (c *cmd) listKongTLSCertificates() (certificateIDs, error) { + // definition of Kong snis related objects + // KongCertObj is the structure for part of certificate object returned from Kong's API /snis + type KongCertObj struct { + CertId string `json:"id"` + } + + // SniCertObj is the structure for part of snis object returned from Kong's API /snis + type SniCertObj struct { + SnisName string `json:"name"` + KongCert KongCertObj `json:"certificate"` + } + + // KongSnisObj is the top level structure for part of json data object returned from Kong's API /snis + type KongSnisObj struct { + Entries []SniCertObj `json:"data,omitempty"` + } + // list snis certificates association to see if any already exists certKongURL := strings.Join([]string{c.configuration.KongURL.GetProxyBaseURL(), "snis"}, "/") c.loggingClient.Info(fmt.Sprintf("list snis tls certificates on the endpoint of %s", certKongURL)) @@ -158,63 +171,31 @@ func (c *cmd) listKongTLSCertificates() ([]string, error) { return nil, fmt.Errorf("Failed to read response body to list Kong snis tls certs: %w", err) } - var parsedResponse map[string]interface{} + snisCerts := KongSnisObj{} switch resp.StatusCode { case http.StatusOK: - if err := json.NewDecoder(bytes.NewReader(responseBody)).Decode(&parsedResponse); err != nil { + if err := json.NewDecoder(bytes.NewReader(responseBody)).Decode(&snisCerts); err != nil { return nil, fmt.Errorf("Unable to parse response from list snis certificate: %w", err) } default: return nil, fmt.Errorf("List Kong tls snis certificates request failed with code: %d", resp.StatusCode) } - var jsonData []byte - jsonData, err = json.Marshal(parsedResponse["data"]) - if err != nil { - return nil, fmt.Errorf("Failed to json marshal parsed response data: %w", err) - } - - // the list snis get API returns an array of snis-certs association - var parsedSnisCertData []map[string]interface{} - if err := json.NewDecoder(bytes.NewReader(jsonData)).Decode(&parsedSnisCertData); err != nil { - return nil, fmt.Errorf("Unable to parse response for parsed SNIS cert data: %w", err) - } - - c.serverNameToCertIDs = make(map[string]certificateIDs) - for _, sniCerts := range parsedSnisCertData { - // extract sni name and certificate id from parsed json data - sni := fmt.Sprintf("%s", sniCerts["name"]) - certID := fmt.Sprintf("%s", sniCerts["certificate"].(map[string]interface{})["id"]) - - if certIDs, exists := c.serverNameToCertIDs[sni]; !exists { + serverNameToCertIDs := make(map[string]certificateIDs) + for _, entry := range snisCerts.Entries { + if certIDs, exists := serverNameToCertIDs[entry.SnisName]; !exists { // initialize for a new sni name - c.serverNameToCertIDs[sni] = []string{certID} + serverNameToCertIDs[entry.SnisName] = []string{entry.KongCert.CertId} } else { // update the existing certificateID array - certIDs = append(certIDs, certID) - c.serverNameToCertIDs[sni] = certIDs - } - } - - uniqueSnisFromInput := getServerNameIndicators(c.serverNameIndicatorList) - // collect the unique certificate ids that match the given snis from the input to be deleted - uniqueCertIDs := make(map[string]bool) - for _, sniFromInput := range uniqueSnisFromInput { - if certIDs, exists := c.serverNameToCertIDs[sniFromInput]; exists { - for _, certID := range certIDs { - uniqueCertIDs[certID] = true - } + certIDs = append(certIDs, entry.KongCert.CertId) + serverNameToCertIDs[entry.SnisName] = certIDs } } - // get the unique certificate ids - certIDs := make([]string, 0, len(uniqueCertIDs)) - for certID := range uniqueCertIDs { - certIDs = append(certIDs, certID) - } - - return certIDs, nil + // only get to-be-deleted certificates with which unique certIDs that match the server name indicators + return c.getUniqueCertIDsMatchServerNames(serverNameToCertIDs), nil } func (c *cmd) deleteKongTLSCertificateById(certId string) error { @@ -282,6 +263,26 @@ func (c *cmd) postKongTLSCertificate(certKeyPair *bootstrapConfig.CertKeyPair) e return nil } +func (c *cmd) getUniqueCertIDsMatchServerNames(snisToCertIDs map[string]certificateIDs) certificateIDs { + uniqueSnisFromInput := getServerNameIndicators(c.serverNameIndicatorList) + // collect the unique certificate ids that match the given snis from the input to be deleted + uniqueCertIDs := make(map[string]bool) + for _, sniFromInput := range uniqueSnisFromInput { + if certIDs, exists := snisToCertIDs[sniFromInput]; exists { + for _, certID := range certIDs { + uniqueCertIDs[certID] = true + } + } + } + + // get the unique certificate ids + certIDs := make([]string, 0, len(uniqueCertIDs)) + for certID := range uniqueCertIDs { + certIDs = append(certIDs, certID) + } + return certIDs +} + func getServerNameIndicators(snisList string) []string { // this will parse out the internal default server name indications and extra ones from input if given var snis []string diff --git a/internal/security/config/command/proxy/tls/command_test.go b/internal/security/config/command/proxy/tls/command_test.go index 2f63dfae24..a2ad1eab72 100644 --- a/internal/security/config/command/proxy/tls/command_test.go +++ b/internal/security/config/command/proxy/tls/command_test.go @@ -129,7 +129,7 @@ func TestTLSAddNewCertificate(t *testing.T) { } } -func TestGetServerNameIndicatros(t *testing.T) { +func TestGetServerNameIndicators(t *testing.T) { builtinSnis := []string{"localhost", "kong"} tests := []struct { name string