Make Azure key parsing failable

azkv/keysource.go

@@ -58,33 +58,36 @@ func NewMasterKey(vaultURL string, keyName string, keyVersion string) *MasterKey
 }
 
 // MasterKeysFromURLs takes a comma separated list of Azure Key Vault URLs and returns a slice of new MasterKeys for them
-func MasterKeysFromURLs(urls string) []*MasterKey {
+func MasterKeysFromURLs(urls string) ([]*MasterKey, error) {
 	var keys []*MasterKey
 	if urls == "" {
-		return keys
+		return keys, nil
 	}
 	for _, s := range strings.Split(urls, ",") {
-		keys = append(keys, NewMasterKeyFromURL(s))
+		k, err := NewMasterKeyFromURL(s)
+		if err != nil {
+			return nil, err
+		}
+		keys = append(keys, k)
 	}
-	return keys
+	return keys, nil
 }
 
 // NewMasterKeyFromResourceID takes an Azure Key Vault key URL and returns a new MasterKey
 // URL format is {vaultUrl}/keys/{key-name}/{key-version}
-func NewMasterKeyFromURL(url string) *MasterKey {
+func NewMasterKeyFromURL(url string) (*MasterKey, error) {
 	k := &MasterKey{}
 	re := regexp.MustCompile("^(https://[^/]+)/keys/([^/]+)/([^/]+)$")
 	parts := re.FindStringSubmatch(url)
 	if parts == nil || len(parts) < 2 {
-		log.Error("No match!")
-		// !?
+		return nil, fmt.Errorf("Could not parse valid key from %q", url)
 	}
 
 	k.VaultURL = parts[1]
 	k.Name = parts[2]
 	k.Version = parts[3]
 	k.CreationDate = time.Now().UTC()
-	return k
+	return k, nil
 }
 
 // EncryptedDataKey returns the encrypted data key this master key holds

azkv/keysource_test.go

@@ -12,12 +12,14 @@ func TestAzureKeySourceFromUrl(t *testing.T) {
 	cases := []struct {
 		name              string
 		input             string
+		expectSuccess     bool
 		expectedFoundKeys int
 		expectedKeys      []MasterKey
 	}{
 		{
 			name:              "Single url",
 			input:             "https://test.vault.azure.net/keys/test-key/a2a690a4fcc04166b739da342a912c90",
+			expectSuccess:     true,
 			expectedFoundKeys: 1,
 			expectedKeys: []MasterKey{
 				{
@@ -30,6 +32,7 @@ func TestAzureKeySourceFromUrl(t *testing.T) {
 		{
 			name:              "Multiple url",
 			input:             "https://test.vault.azure.net/keys/test-key/a2a690a4fcc04166b739da342a912c90,https://test2.vault.azure.net/keys/another-test-key/cf0021e8b743453bae758e7fbf71b60e",
+			expectSuccess:     true,
 			expectedFoundKeys: 2,
 			expectedKeys: []MasterKey{
 				{
@@ -44,11 +47,22 @@ func TestAzureKeySourceFromUrl(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:          "Single malformed url",
+			input:         "https://test.vault.azure.net/no-keys-here/test-key/a2a690a4fcc04166b739da342a912c90",
+			expectSuccess: false,
+		},
 	}
 
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
-			keys := MasterKeysFromURLs(c.input)
+			keys, err := MasterKeysFromURLs(c.input)
+			if err != nil && c.expectSuccess {
+				t.Fatalf("Unexpected error %v", err)
+			} else if err == nil && !c.expectSuccess {
+				t.Fatal("Expected error, but no error was returned")
+			}
+
 			if c.expectedFoundKeys != len(keys) {
 				t.Errorf("Unexpected number of keys returned, expected %d, got %d", c.expectedFoundKeys, len(keys))
 			}
@@ -87,8 +101,11 @@ func TestRoundtrip(t *testing.T) {
 
 	input := []byte("test-string")
 
-	key := NewMasterKeyFromURL(*azureKeyAcceptanceTestUrl)
-	err := key.Encrypt(input)
+	key, err := NewMasterKeyFromURL(*azureKeyAcceptanceTestUrl)
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = key.Encrypt(input)
 	if err != nil {
 		t.Fatal(err)
 	}

cmd/sops/main.go

@@ -92,7 +92,7 @@ func main() {
    To use multiple KMS or PGP keys, separate them by commas. For example:
        $ sops -p "10F2...0A, 85D...B3F21" file.yaml
 
-   The -p, -k and --gcp-kms flags are only used to encrypt new documents. Editing
+   The -p, -k, --gcp-kms and --azure-kv flags are only used to encrypt new documents. Editing
    or decrypting existing documents can be done with "sops file" or
    "sops -d file" respectively. The KMS and PGP keys listed in the encrypted
    documents are used then. To manage master keys in existing documents, use
@@ -188,7 +188,8 @@ func main() {
 					Action: func(c *cli.Context) error {
 						pgpFps := c.StringSlice("pgp")
 						kmsArns := c.StringSlice("kms")
-						azkvs := c.StringSlice("azkv")
+						gcpKmses := c.StringSlice("gcp-kms")
+						azkvs := c.StringSlice("azure-kv")
 						var group sops.KeyGroup
 						for _, fp := range pgpFps {
 							group = append(group, pgp.NewMasterKeyFromFingerprint(fp))
@@ -196,9 +197,16 @@ func main() {
 						for _, arn := range kmsArns {
 							group = append(group, kms.NewMasterKeyFromArn(arn, kms.ParseKMSContext(c.String("encryption-context"))))
 						}
-						// NOTE: Why isn't GCP here?
+						for _, kms := range gcpKmses {
+							group = append(group, gcpkms.NewMasterKeyFromResourceID(kms))
+						}
 						for _, url := range azkvs {
-							group = append(group, azkv.NewMasterKeyFromURL(url))
+							k, err := azkv.NewMasterKeyFromURL(url)
+							if err != nil {
+								log.WithError(err).Error("Failed to add key")
+								continue
+							}
+							group = append(group, k)
 						}
 						return groups.Add(groups.AddOpts{
 							InputPath:      c.String("file"),
@@ -499,7 +507,11 @@ func main() {
 			for _, k := range gcpkms.MasterKeysFromResourceIDString(c.String("add-gcp-kms")) {
 				addMasterKeys = append(addMasterKeys, k)
 			}
-			for _, k := range azkv.MasterKeysFromURLs(c.String("add-azure-kv")) {
+			azureKeys, err := azkv.MasterKeysFromURLs(c.String("add-azure-kv"))
+			if err != nil {
+				return err
+			}
+			for _, k := range azureKeys {
 				addMasterKeys = append(addMasterKeys, k)
 			}
 
@@ -513,7 +525,11 @@ func main() {
 			for _, k := range gcpkms.MasterKeysFromResourceIDString(c.String("rm-gcp-kms")) {
 				rmMasterKeys = append(rmMasterKeys, k)
 			}
-			for _, k := range azkv.MasterKeysFromURLs(c.String("rm-azure-kv")) {
+			azureKeys, err = azkv.MasterKeysFromURLs(c.String("rm-azure-kv"))
+			if err != nil {
+				return err
+			}
+			for _, k := range azureKeys {
 				rmMasterKeys = append(rmMasterKeys, k)
 			}
 			output, err = rotate(rotateOpts{
@@ -726,7 +742,11 @@ func keyGroups(c *cli.Context, file string) ([]sops.KeyGroup, error) {
 		}
 	}
 	if c.String("azure-kv") != "" {
-		for _, k := range azkv.MasterKeysFromURLs(c.String("azure-kv")) {
+		azureKeys, err := azkv.MasterKeysFromURLs(c.String("azure-kv"))
+		if err != nil {
+			return nil, err
+		}
+		for _, k := range azureKeys {
 			azkvKeys = append(azkvKeys, k)
 		}
 	}

config/config.go

@@ -181,7 +181,11 @@ func loadForFileFromBytes(confBytes []byte, filePath string, kmsEncryptionContex
 		for _, k := range gcpkms.MasterKeysFromResourceIDString(rule.GCPKMS) {
 			keyGroup = append(keyGroup, k)
 		}
-		for _, k := range azkv.MasterKeysFromURLs(rule.AzureKeyVault) {
+		azureKeys, err := azkv.MasterKeysFromURLs(rule.AzureKeyVault)
+		if err != nil {
+			return nil, err
+		}
+		for _, k := range azureKeys {
 			keyGroup = append(keyGroup, k)
 		}
 		groups = append(groups, keyGroup)