Fix transit byok tool, add docs, tests (#19373)

* Fix Vault Transit BYOK helper argument parsing

This commit fixes the following issues with the importer:

 - More than two arguments were not supported, causing the CLI to error
   out and resulting in a failure to import RSA keys.
 - The @file notation support was not accepted for KEY, meaning
   unencrypted keys had to be manually specified on the CLI.
 - Parsing of additional argument data was done in a non-standard way.
 - Fix parsing of command line options and ensure only relevant
   options are included.

Additionally, some error messages and help text was clarified.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add missing documentation on Transit CLI to website

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add tests for Transit BYOK vault subcommand

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add changelog

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Appease CI

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

---------

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

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
+  the base64 encoded KEY (either directly on the CLI or via @path notation),
-  into an existing key, use import_version.  The remaining options after KEY (key=value style) are passed
+  into a new key whose API path is PATH.  To import a new version into an
-  on to the transit/transform create key endpoint.  If your system or device natively supports
+  existing key, use import_version.  The remaining options after KEY (key=value
-  the RSA AES key wrap mechanism, you should use it directly rather than this command. 
+  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 {
+func importKey(c *BaseCommand, operation string, flags *FlagSets, args []string) int {
-	if len(args) != 2 {
+	// Parse and validate the arguments.
-		c.UI.Error(fmt.Sprintf("Incorrect argument count (expected 2, got %d)", len(args)))
+	if err := flags.Parse(args); err != nil {
+		c.UI.Error(err.Error())
+		return 1
+	}
+
+	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{}{
+	data, err := parseArgsData(os.Stdin, args[2:])
-		"ciphertext": importCiphertext,
+	if err != nil {
+		c.UI.Error(fmt.Sprintf("Failed to parse extra K=V data: %s", err))
+		return 1
 	}
-	for _, v := range args[2:] {
+	if data == nil {
-		parts := strings.Split(v, "=")
+		data = make(map[string]interface{}, 1)
-		data[parts[0]] = parts[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,186 @@
+package command
+
+import (
+	"bytes"
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/x509"
+	"encoding/base64"
+	"testing"
+
+	"github.com/hashicorp/vault/api"
+
+	"github.com/stretchr/testify/require"
+)
+
+// Validate the `vault transit import` command works.
+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,
+  the base64 encoded KEY (either directly on the CLI or via @path notation),
-  use import.  The remaining options after KEY (key=value style) are passed on to the transit/transform create key 
+  into a new key whose API path is PATH.  To import a new transit/transform
-  endpoint. 
+  key, use the import command instead.  The remaining options after KEY
-  If your system or device natively supports the RSA AES key wrap mechanism, you should use it directly 
+  (key=value style) are passed on to the transit/transform create key endpoint.
-  rather than this command.
+  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.

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

@@ -0,0 +1,32 @@
+---
+layout: docs
+page_title: transit - Command
+description: |-
+  The "transit" command groups subcommands for interacting with Vault's Transit
+  secrets engine.
+---
+
+# transit
+
+The `transit` command groups subcommands for interacting with Vault's
+[Transit Secrets Engine](/vault/docs/secrets/transit).
+
+## Syntax
+
+Option flags for a given subcommand are provided after the subcommand, but before the arguments.
+
+## Examples
+
+To [import](/vault/docs/commands/transit/import) keys into a mount via the
+[Transit BYOK](/vault/docs/secrets/transit#bring-your-own-key-byok)
+mechanism, use the `vault transit import <path> <key>` or
+`vault transit import-version <path> <key>` commands:
+
+```
+$ 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!
+```

website/data/docs-nav-data.json

@@ -832,6 +832,19 @@
           }
         ]
       },
+      {
+        "title": "<code>transit</code>",
+        "routes": [
+          {
+            "title": "Overview",
+            "path": "commands/transit"
+          },
+          {
+            "title": "<code>import</code> and <code>import-version</code>",
+            "path": "commands/transit/import"
+          }
+        ]
+      },
       {
         "title": "<code>unwrap</code>",
         "path": "commands/unwrap"