Skip to content

Commit 3012412

Browse files
authored
fix: handles key vault name with double quotes. (#102)
* fix: handles vault name with double quotes. Signed-off-by: Nilekh Chaudhari <1626598+nilekhc@users.noreply.github.com> * ping * chore: gofmt * fix: adds sanitize util method * fix: updates tests
1 parent 12ad347 commit 3012412

File tree

5 files changed

+67
-2
lines changed

5 files changed

+67
-2
lines changed

docs/manual-install.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ This guide demonstrates steps required to enable the KMS Plugin for Key Vault in
7373
imagePullPolicy: IfNotPresent
7474
args:
7575
- --listen-addr=unix:///opt/azurekms.socket # [OPTIONAL] gRPC listen address. Default is unix:///opt/azurekms.socket
76-
- --keyvault-name=${KV_NAME} # [REQUIRED] Name of the keyvault
76+
- --keyvault-name=${KV_NAME} # [REQUIRED] Name of the keyvault. Must match criteria specified at https://docs.microsoft.com/en-us/azure/key-vault/general/about-keys-secrets-certificates#vault-name-and-object-name
7777
- --key-name=${KEY_NAME} # [REQUIRED] Name of the keyvault key used for encrypt/decrypt
7878
- --key-version=${KEY_VERSION} # [REQUIRED] Version of the key to use
7979
- --log-format-json=false # [OPTIONAL] Set log formatter to json. Default is false.

pkg/plugin/keyvault.go

+7
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313

1414
"github.com/Azure/kubernetes-kms/pkg/auth"
1515
"github.com/Azure/kubernetes-kms/pkg/config"
16+
"github.com/Azure/kubernetes-kms/pkg/utils"
1617
"github.com/Azure/kubernetes-kms/pkg/version"
1718

1819
kv "github.com/Azure/azure-sdk-for-go/services/keyvault/2016-10-01/keyvault"
@@ -38,6 +39,11 @@ type keyVaultClient struct {
3839

3940
// NewKeyVaultClient returns a new key vault client to use for kms operations
4041
func newKeyVaultClient(config *config.AzureConfig, vaultName, keyName, keyVersion string) (*keyVaultClient, error) {
42+
//Sanitize vaultName, keyName, keyVersion. (https://github.com/Azure/kubernetes-kms/issues/85)
43+
vaultName = utils.SanitizeString(vaultName)
44+
keyName = utils.SanitizeString(keyName)
45+
keyVersion = utils.SanitizeString(keyVersion)
46+
4147
// this should be the case for bring your own key, clusters bootstrapped with
4248
// aks-engine or aks and standalone kms plugin deployments
4349
if len(vaultName) == 0 || len(keyName) == 0 || len(keyVersion) == 0 {
@@ -115,6 +121,7 @@ func getVaultURL(vaultName string, azureEnvironment *azure.Environment) (vaultUR
115121
if len(vaultName) < 3 || len(vaultName) > 24 {
116122
return nil, fmt.Errorf("invalid vault name: %q, must be between 3 and 24 chars", vaultName)
117123
}
124+
118125
// See docs for validation spec: https://docs.microsoft.com/en-us/azure/key-vault/about-keys-secrets-and-certificates#objects-identifiers-and-versioning
119126
isValid := regexp.MustCompile(`^[-A-Za-z0-9]+$`).MatchString
120127
if !isValid(vaultName) {

pkg/plugin/keyvault_test.go

+9-1
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,14 @@ func TestNewKeyVaultClient(t *testing.T) {
5757
keyVersion: "262067a9e8ba401aa8a746c5f1a7e147",
5858
expectedErr: false,
5959
},
60+
{
61+
desc: "no error with double quotes",
62+
config: &config.AzureConfig{ClientID: "clientid", ClientSecret: "clientsecret"},
63+
vaultName: "\"testkv\"",
64+
keyName: "\"key1\"",
65+
keyVersion: "\"262067a9e8ba401aa8a746c5f1a7e147\"",
66+
expectedErr: false,
67+
},
6068
}
6169

6270
for _, test := range tests {
@@ -118,7 +126,7 @@ func TestGetVaultURL(t *testing.T) {
118126
if test.expectedErr && err == nil || !test.expectedErr && err != nil {
119127
t.Fatalf("expected error: %v, got error: %v", test.expectedErr, err)
120128
}
121-
expectedURL := "https://" + test.vaultName + "." + vaultDNSSuffix[idx] + "/"
129+
expectedURL := "https://" + strings.Trim(test.vaultName, "\"") + "." + vaultDNSSuffix[idx] + "/"
122130
if !test.expectedErr && expectedURL != *vaultURL {
123131
t.Fatalf("expected vault url: %s, got: %s", expectedURL, *vaultURL)
124132
}

pkg/utils/sanitize.go

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package utils
2+
3+
import "strings"
4+
5+
// SanitizeString returns a string that does not have white spaces and double quotes
6+
func SanitizeString(s string) string {
7+
return strings.TrimSpace(strings.Trim(strings.TrimSpace(s), "\""))
8+
}

pkg/utils/sanitize_test.go

+42
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package utils
2+
3+
import "testing"
4+
5+
func TestSanitizeString(t *testing.T) {
6+
testCases := []struct {
7+
name string
8+
input string
9+
expectedOutput string
10+
}{
11+
{
12+
name: "With_White_Spaces",
13+
input: " hello ",
14+
expectedOutput: "hello",
15+
},
16+
{
17+
name: "With_Double_Quotes",
18+
input: "\"hello\"",
19+
expectedOutput: "hello",
20+
},
21+
{
22+
name: "With_White_Spaces_And_Double_Quotes",
23+
input: " \"hello\" ",
24+
expectedOutput: "hello",
25+
},
26+
{
27+
name: "With_Double_Quotes_And_White_Spaces",
28+
input: "\" hello \"",
29+
expectedOutput: "hello",
30+
},
31+
}
32+
33+
for _, testCase := range testCases {
34+
t.Run(testCase.name, func(t *testing.T) {
35+
sanitizedString := SanitizeString(testCase.input)
36+
37+
if sanitizedString != testCase.expectedOutput {
38+
t.Fatalf("expected output: '%s', found: '%s'", testCase.expectedOutput, sanitizedString)
39+
}
40+
})
41+
}
42+
}

0 commit comments

Comments
 (0)