Fix transit byok tool, add docs, tests

changelog/19373.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+cli/transit: Fix import, import-version command invocation
+```

command/transit_import_key.go

@@ -8,6 +8,7 @@ import (
 	"encoding/base64"
 	"encoding/pem"
 	"fmt"
+	"os"
 	"regexp"
 	"strings"
 
@@ -38,17 +39,20 @@ func (c *TransitImportCommand) Help() string {
 Usage: vault transit import PATH KEY [options...]
 
   Using the Transit or Transform key wrapping system, imports key material from
-  the base64 encoded KEY, into a new key whose API path is PATH.  To import a new version
-  into an existing key, use import_version.  The remaining options after KEY (key=value style) are passed
-  on to the transit/transform create key endpoint.  If your system or device natively supports
-  the RSA AES key wrap mechanism, you should use it directly rather than this command. 
+  the base64 encoded KEY (either directly on the CLI or via @path notation),
+  into a new key whose API path is PATH.  To import a new version into an
+  existing key, use import_version.  The remaining options after KEY (key=value
+  style) are passed on to the transit/transform create key endpoint.  If your
+  system or device natively supports the RSA AES key wrap mechanism (such as
+  the PKCS#11 mechanism CKM_RSA_AES_KEY_WRAP), you should use it directly
+  rather than this command.
 ` + c.Flags().Help()
 
 	return strings.TrimSpace(helpText)
 }
 
 func (c *TransitImportCommand) Flags() *FlagSets {
-	return c.flagSet(FlagSetHTTP | FlagSetOutputField | FlagSetOutputFormat)
+	return c.flagSet(FlagSetHTTP)
 }
 
 func (c *TransitImportCommand) AutocompleteArgs() complete.Predictor {
@@ -60,13 +64,20 @@ func (c *TransitImportCommand) AutocompleteFlags() complete.Flags {
 }
 
 func (c *TransitImportCommand) Run(args []string) int {
-	return importKey(c.BaseCommand, "import", args)
+	return importKey(c.BaseCommand, "import", c.Flags(), args)
 }
 
 // error codes: 1: user error, 2: internal computation error, 3: remote api call error
-func importKey(c *BaseCommand, operation string, args []string) int {
-	if len(args) != 2 {
-		c.UI.Error(fmt.Sprintf("Incorrect argument count (expected 2, got %d)", len(args)))
+func importKey(c *BaseCommand, operation string, flags *FlagSets, args []string) int {
+	// Parse and validate the arguments.
+	if err := flags.Parse(args); err != nil {
+		c.UI.Error(err.Error())
+		return pkiRetUsage
+	}
+
+	args = flags.Args()
+	if len(args) < 2 {
+		c.UI.Error(fmt.Sprintf("Incorrect argument count (expected 2+, got %d). Wanted PATH to import into and KEY material.", len(args)))
 		return 1
 	}
 
@@ -89,7 +100,18 @@ func importKey(c *BaseCommand, operation string, args []string) int {
 	path := parts[1]
 	keyName := parts[2]
 
-	key, err := base64.StdEncoding.DecodeString(args[1])
+	keyMaterial := args[1]
+	if keyMaterial[0] == '@' {
+		keyMaterialBytes, err := os.ReadFile(keyMaterial[1:])
+		if err != nil {
+			c.UI.Error(fmt.Sprintf("error reading key material file: %v", err))
+			return 1
+		}
+
+		keyMaterial = string(keyMaterialBytes)
+	}
+
+	key, err := base64.StdEncoding.DecodeString(keyMaterial)
 	if err != nil {
 		c.UI.Error(fmt.Sprintf("error base64 decoding source key material: %v", err))
 		return 1
@@ -126,15 +148,19 @@ func importKey(c *BaseCommand, operation string, args []string) int {
 	}
 	combinedCiphertext := append(wrappedAESKey, wrappedTargetKey...)
 	importCiphertext := base64.StdEncoding.EncodeToString(combinedCiphertext)
+
 	// Parse all the key options
-	data := map[string]interface{}{
-		"ciphertext": importCiphertext,
+	data, err := parseArgsData(os.Stdin, args[2:])
+	if err != nil {
+		c.UI.Error(fmt.Sprintf("Failed to parse extra K=V data: %s", err))
+		return 1
 	}
-	for _, v := range args[2:] {
-		parts := strings.Split(v, "=")
-		data[parts[0]] = parts[1]
+	if data == nil {
+		data = make(map[string]interface{}, 1)
 	}
 
+	data["ciphertext"] = importCiphertext
+
 	c.UI.Output("Submitting wrapped key to Vault transit.")
 	// Finally, call import
 	_, err = client.Logical().Write(path+"/keys/"+keyName+"/"+operation, data)

command/transit_import_key_test.go

@@ -0,0 +1,185 @@
+package command
+
+import (
+	"bytes"
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/x509"
+	"encoding/base64"
+	"testing"
+
+	"github.com/hashicorp/vault/api"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestTransitImport(t *testing.T) {
+	t.Parallel()
+
+	client, closer := testVaultServer(t)
+	defer closer()
+
+	if err := client.Sys().Mount("transit", &api.MountInput{
+		Type: "transit",
+	}); err != nil {
+		t.Fatalf("transit mount error: %#v", err)
+	}
+
+	rsa1, rsa2, aes128, aes256 := generateKeys(t)
+
+	type testCase struct {
+		variant    string
+		path       string
+		key        []byte
+		args       []string
+		shouldFail bool
+	}
+	tests := []testCase{
+		{
+			"import",
+			"transit/keys/rsa1",
+			rsa1,
+			[]string{"type=rsa-2048"},
+			false, /* first import */
+		},
+		{
+			"import",
+			"transit/keys/rsa1",
+			rsa2,
+			[]string{"type=rsa-2048"},
+			true, /* already exists */
+		},
+		{
+			"import-version",
+			"transit/keys/rsa1",
+			rsa2,
+			[]string{"type=rsa-2048"},
+			false, /* new version */
+		},
+		{
+			"import",
+			"transit/keys/rsa2",
+			rsa2,
+			[]string{"type=rsa-4096"},
+			true, /* wrong type */
+		},
+		{
+			"import",
+			"transit/keys/rsa2",
+			rsa2,
+			[]string{"type=rsa-2048"},
+			false, /* new name */
+		},
+		{
+			"import",
+			"transit/keys/aes1",
+			aes128,
+			[]string{"type=aes128-gcm96"},
+			false, /* first import */
+		},
+		{
+			"import",
+			"transit/keys/aes1",
+			aes256,
+			[]string{"type=aes256-gcm96"},
+			true, /* already exists */
+		},
+		{
+			"import-version",
+			"transit/keys/aes1",
+			aes256,
+			[]string{"type=aes256-gcm96"},
+			true, /* new version, different type */
+		},
+		{
+			"import-version",
+			"transit/keys/aes1",
+			aes128,
+			[]string{"type=aes128-gcm96"},
+			false, /* new version */
+		},
+		{
+			"import",
+			"transit/keys/aes2",
+			aes256,
+			[]string{"type=aes128-gcm96"},
+			true, /* wrong type */
+		},
+		{
+			"import",
+			"transit/keys/aes2",
+			aes256,
+			[]string{"type=aes256-gcm96"},
+			false, /* new name */
+		},
+	}
+
+	for index, tc := range tests {
+		t.Logf("Running test case %d: %v", index, tc)
+		execTransitImport(t, client, tc.variant, tc.path, tc.key, tc.args, tc.shouldFail)
+	}
+}
+
+func execTransitImport(t *testing.T, client *api.Client, method string, path string, key []byte, data []string, expectFailure bool) {
+	t.Helper()
+
+	keyBase64 := base64.StdEncoding.EncodeToString(key)
+
+	var args []string
+	args = append(args, "transit")
+	args = append(args, method)
+	args = append(args, path)
+	args = append(args, keyBase64)
+	args = append(args, data...)
+
+	stdout := bytes.NewBuffer(nil)
+	stderr := bytes.NewBuffer(nil)
+	runOpts := &RunOptions{
+		Stdout: stdout,
+		Stderr: stderr,
+		Client: client,
+	}
+
+	code := RunCustom(args, runOpts)
+	combined := stdout.String() + stderr.String()
+
+	if code != 0 {
+		if !expectFailure {
+			t.Fatalf("Got unexpected failure from test (ret %d): %v", code, combined)
+		}
+	} else {
+		if expectFailure {
+			t.Fatalf("Expected failure, got success from test (ret %d): %v", code, combined)
+		}
+	}
+}
+
+func generateKeys(t *testing.T) (rsa1 []byte, rsa2 []byte, aes128 []byte, aes256 []byte) {
+	t.Helper()
+
+	priv1, err := rsa.GenerateKey(rand.Reader, 2048)
+	require.NotNil(t, priv1, "failed generating RSA 1 key")
+	require.NoError(t, err, "failed generating RSA 1 key")
+
+	rsa1, err = x509.MarshalPKCS8PrivateKey(priv1)
+	require.NotNil(t, rsa1, "failed marshaling RSA 1 key")
+	require.NoError(t, err, "failed marshaling RSA 1 key")
+
+	priv2, err := rsa.GenerateKey(rand.Reader, 2048)
+	require.NotNil(t, priv2, "failed generating RSA 2 key")
+	require.NoError(t, err, "failed generating RSA 2 key")
+
+	rsa2, err = x509.MarshalPKCS8PrivateKey(priv2)
+	require.NotNil(t, rsa2, "failed marshaling RSA 2 key")
+	require.NoError(t, err, "failed marshaling RSA 2 key")
+
+	aes128 = make([]byte, 128/8)
+	_, err = rand.Read(aes128)
+	require.NoError(t, err, "failed generating AES 128 key")
+
+	aes256 = make([]byte, 256/8)
+	_, err = rand.Read(aes256)
+	require.NoError(t, err, "failed generating AES 256 key")
+
+	return
+}

command/transit_import_key_version.go

@@ -22,21 +22,23 @@ func (c *TransitImportVersionCommand) Synopsis() string {
 
 func (c *TransitImportVersionCommand) Help() string {
 	helpText := `
-Usage: vault transit import-version PATH KEY
+Usage: vault transit import-version PATH KEY [...]
 
   Using the Transit or Transform key wrapping system, imports key material from
-  the base64 encoded KEY, into a new key whose API path is PATH.  To import a new transit/transform key,
-  use import.  The remaining options after KEY (key=value style) are passed on to the transit/transform create key 
-  endpoint. 
-  If your system or device natively supports the RSA AES key wrap mechanism, you should use it directly 
-  rather than this command.
+  the base64 encoded KEY (either directly on the CLI or via @path notation),
+  into a new key whose API path is PATH.  To import a new transit/transform
+  key, use the import command instead.  The remaining options after KEY
+  (key=value style) are passed on to the transit/transform create key endpoint.
+  If your system or device natively supports the RSA AES key wrap mechanism
+  (such as the PKCS#11 mechanism CKM_RSA_AES_KEY_WRAP), you should use it
+  directly rather than this command.
 ` + c.Flags().Help()
 
 	return strings.TrimSpace(helpText)
 }
 
 func (c *TransitImportVersionCommand) Flags() *FlagSets {
-	return c.flagSet(FlagSetHTTP | FlagSetOutputField | FlagSetOutputFormat)
+	return c.flagSet(FlagSetHTTP)
 }
 
 func (c *TransitImportVersionCommand) AutocompleteArgs() complete.Predictor {
@@ -48,5 +50,5 @@ func (c *TransitImportVersionCommand) AutocompleteFlags() complete.Flags {
 }
 
 func (c *TransitImportVersionCommand) Run(args []string) int {
-	return importKey(c.BaseCommand, "import_version", args)
+	return importKey(c.BaseCommand, "import_version", c.Flags(), args)
 }

website/content/docs/commands/transit/import.mdx

@@ -0,0 +1,62 @@
+---
+layout: docs
+page_title: transit import and transit import-version - Command
+description: |-
+  The "transit import" and "transit import-version" commands import the
+  specified key into Transit, via the Transit BYOK mechanism.
+---
+
+# transit import and transit import-version
+
+The `transit import` and `transit import-version` commands import the
+specified key into Transit, via the [Transit BYOK
+mechanism](/vault/docs/secrets/transit#bring-your-own-key-byok). The former
+imports this key as a new key, failing if it already exists, whereas the
+latter will only update an existing key in Transit to a new version of the
+key material.
+
+This needs access to read the transit mount's wrapping key (at
+`transit/wrapping_key`) and the ability to write to either import
+endpoints (either `transit/keys/:name/import` or
+`transit/keys/:name/import_version`).
+
+## Examples
+
+Imports a 2048-bit RSA key as a new key:
+
+```
+$ vault transit import transit/keys/test-key @test-key type=rsa-2048
+Retrieving transit wrapping key.
+Wrapping source key with ephemeral key.
+Encrypting ephemeral key with transit wrapping key.
+Submitting wrapped key to Vault transit.
+Success!
+```
+
+Imports a new version of an existing key:
+
+```
+$ vault transit import-version transit/keys/test-key @test-key-updated
+Retrieving transit wrapping key.
+Wrapping source key with ephemeral key.
+Encrypting ephemeral key with transit wrapping key.
+Submitting wrapped key to Vault transit.
+Success!
+```
+
+## Usage
+
+This command does not have any unique flags and respects core Vault CLI
+commands. See `vault transit import -help` for more information.
+
+This command requires two positional arguments:
+
+ 1. `PATH`, the path to the transit key to import in the format of
+    `<mount>/keys/<key-name>`, where `<mount>` is the path to the mount
+    (using `-namespace=<ns>` to specify any namespaces), and `<key-name>`
+    is the desired name of the key.
+ 2. `KEY`, the key material to import in Standard Base64 encoding (either
+    of a raw key in the case of symmetric keys such as AES, or of the DER
+    encoded format for asymmetric keys such as RSA). If the value for `KEY`
+    begins with an `@`, the CLI argument is assumed to be a path to a file
+    on disk to be read.