Add support for openssh comments when generating a private key

Author: F21

.changes/unreleased/ENHANCEMENTS-20230803-094241.yaml

@@ -0,0 +1,8 @@
+kind: ENHANCEMENTS
+body: 'resource/tls_private_key: add openssh_comment attribute
+  data-source/tls_public_key: append openssh comment to public_key_openssh if private_key_openssh contains a comment
+  ephemeral/tls_private_key: add openssh_comment attribute
+  ephemeral/tls_public_key: append openssh comment to public_key_openssh if private_key_openssh contains a comment'
+time: 2023-08-03T09:42:41.390232535Z
+custom:
+  Issue: "395"

docs/ephemeral-resources/private_key.md

@@ -50,6 +50,7 @@ ephemeral "tls_private_key" "ed25519-example" {
 ### Optional
 
 - `ecdsa_curve` (String) When `algorithm` is `ECDSA`, the name of the elliptic curve to use. Currently-supported values are: `P224`, `P256`, `P384`, `P521`. (default: `P224`).
+- `openssh_comment` (String) Comment to add to the OpenSSH key (default: `""`).
 - `rsa_bits` (Number) When `algorithm` is `RSA`, the size of the generated RSA key, in bits (default: `2048`).
 
 ### Read-Only

docs/resources/private_key.md

@@ -56,6 +56,7 @@ resource "tls_private_key" "ed25519-example" {
 ### Optional
 
 - `ecdsa_curve` (String) When `algorithm` is `ECDSA`, the name of the elliptic curve to use. Currently-supported values are: `P224`, `P256`, `P384`, `P521`. (default: `P224`).
+- `openssh_comment` (String) Comment to add to the OpenSSH key (default: `""`).
 - `rsa_bits` (Number) When `algorithm` is `RSA`, the size of the generated RSA key, in bits (default: `2048`).
 
 ### Read-Only

internal/provider/common_key.go

@@ -13,7 +13,10 @@ import (
 	"crypto/rsa"
 	"crypto/x509"
 	"encoding/pem"
+	"errors"
 	"fmt"
+	"math/big"
+	"strings"
 
 	"github.com/hashicorp/terraform-plugin-framework/diag"
 	"github.com/hashicorp/terraform-plugin-framework/path"
@@ -118,18 +121,22 @@ func parsePrivateKeyPEM(keyPEMBytes []byte) (crypto.PrivateKey, Algorithm, error
 // parsePrivateKeyOpenSSHPEM takes a slide of bytes containing a private key
 // encoded in [OpenSSH PEM (RFC 4716)](https://datatracker.ietf.org/doc/html/rfc4716) format,
 // and returns a crypto.PrivateKey implementation, together with the Algorithm used by the key.
-func parsePrivateKeyOpenSSHPEM(keyOpenSSHPEMBytes []byte) (crypto.PrivateKey, Algorithm, error) {
+func parsePrivateKeyOpenSSHPEM(keyOpenSSHPEMBytes []byte) (crypto.PrivateKey, Algorithm, string, error) {
 	prvKey, err := ssh.ParseRawPrivateKey(keyOpenSSHPEMBytes)
 	if err != nil {
-		return nil, "", fmt.Errorf("failed to parse openssh private key: %w", err)
+		return nil, "", "", fmt.Errorf("failed to parse openssh private key: %w", err)
 	}
 
-	algorithm, err := privateKeyToAlgorithm(prvKey)
+	comment, err := getPrivateKeyComment(keyOpenSSHPEMBytes)
 	if err != nil {
-		return nil, "", fmt.Errorf("failed to determine key algorithm for private key of type %T: %w", prvKey, err)
+		return nil, "", "", fmt.Errorf("failed to get private key comment: %w", err)
 	}
 
-	return prvKey, algorithm, nil
+	algorithm, err := privateKeyToAlgorithm(prvKey)
+	if err != nil {
+		return nil, "", "", fmt.Errorf("failed to determine key algorithm for private key of type %T: %w", prvKey, err)
+	}
+	return prvKey, algorithm, comment, nil
 }
 
 // privateKeyToPublicKey takes a crypto.PrivateKey and extracts the corresponding crypto.PublicKey,
@@ -159,7 +166,7 @@ func privateKeyToAlgorithm(prvKey crypto.PrivateKey) (Algorithm, error) {
 
 // setPublicKeyAttributes takes a crypto.PrivateKey, extracts the corresponding crypto.PublicKey and then
 // encodes related attributes on the given *tfsdk.State.
-func setPublicKeyAttributes(ctx context.Context, s *tfsdk.State, prvKey crypto.PrivateKey) diag.Diagnostics {
+func setPublicKeyAttributes(ctx context.Context, s *tfsdk.State, prvKey crypto.PrivateKey, openSSHComment string) diag.Diagnostics {
 	var diags diag.Diagnostics
 
 	pubKey, err := privateKeyToPublicKey(prvKey)
@@ -199,8 +206,13 @@ func setPublicKeyAttributes(ctx context.Context, s *tfsdk.State, prvKey crypto.P
 	var pubKeySSH, pubKeySSHFingerprintMD5, pubKeySSHFingerprintSHA256 string
 	if err == nil {
 		sshPubKeyBytes := ssh.MarshalAuthorizedKey(sshPubKey)
-
 		pubKeySSH = string(sshPubKeyBytes)
+
+		// Manually add the comment as MarshalAuthorizedKeys ignores it: https://github.com/golang/go/issues/46870
+		if openSSHComment != "" {
+			pubKeySSH = fmt.Sprintf("%s %s\n", strings.TrimSuffix(pubKeySSH, "\n"), openSSHComment)
+		}
+
 		pubKeySSHFingerprintMD5 = ssh.FingerprintLegacyMD5(sshPubKey)
 		pubKeySSHFingerprintSHA256 = ssh.FingerprintSHA256(sshPubKey)
 	}
@@ -225,7 +237,7 @@ func setPublicKeyAttributes(ctx context.Context, s *tfsdk.State, prvKey crypto.P
 
 // setPublicKeyAttributes takes a crypto.PrivateKey, extracts the corresponding crypto.PublicKey and then
 // encodes related attributes on the given *tfsdk.EphemeralResultData.
-func setPublicKeyAttributesEphemeral(ctx context.Context, d *tfsdk.EphemeralResultData, prvKey crypto.PrivateKey) diag.Diagnostics {
+func setPublicKeyAttributesEphemeral(ctx context.Context, d *tfsdk.EphemeralResultData, prvKey crypto.PrivateKey, openSSHComment string) diag.Diagnostics {
 	var diags diag.Diagnostics
 
 	pubKey, err := privateKeyToPublicKey(prvKey)
@@ -265,8 +277,13 @@ func setPublicKeyAttributesEphemeral(ctx context.Context, d *tfsdk.EphemeralResu
 	var pubKeySSH, pubKeySSHFingerprintMD5, pubKeySSHFingerprintSHA256 string
 	if err == nil {
 		sshPubKeyBytes := ssh.MarshalAuthorizedKey(sshPubKey)
-
 		pubKeySSH = string(sshPubKeyBytes)
+
+		// Manually add the comment as MarshalAuthorizedKeys ignores it: https://github.com/golang/go/issues/46870
+		if openSSHComment != "" {
+			pubKeySSH = fmt.Sprintf("%s %s\n", strings.TrimSuffix(pubKeySSH, "\n"), openSSHComment)
+		}
+
 		pubKeySSHFingerprintMD5 = ssh.FingerprintLegacyMD5(sshPubKey)
 		pubKeySSHFingerprintSHA256 = ssh.FingerprintSHA256(sshPubKey)
 	}
@@ -288,3 +305,106 @@ func setPublicKeyAttributesEphemeral(ctx context.Context, d *tfsdk.EphemeralResu
 
 	return nil
 }
+
+// Note: The SSH package does not currently expose the comment in the private key, so an adapted version of
+// parseOpenSSHPrivateKey from https://github.com/golang/crypto/blob/master/ssh/keys.go#L1532
+const privateKeyAuthMagic = "openssh-key-v1\x00"
+
+type openSSHEncryptedPrivateKey struct {
+	CipherName   string
+	KdfName      string
+	KdfOpts      string
+	NumKeys      uint32
+	PubKey       []byte
+	PrivKeyBlock []byte
+}
+
+type openSSHPrivateKey struct {
+	Check1  uint32
+	Check2  uint32
+	Keytype string
+	Rest    []byte `ssh:"rest"`
+}
+
+type openSSHRSAPrivateKey struct {
+	N       *big.Int
+	E       *big.Int
+	D       *big.Int
+	Iqmp    *big.Int
+	P       *big.Int
+	Q       *big.Int
+	Comment string
+	Pad     []byte `ssh:"rest"`
+}
+
+type openSSHEd25519PrivateKey struct {
+	Pub     []byte
+	Priv    []byte
+	Comment string
+	Pad     []byte `ssh:"rest"`
+}
+
+type openSSHECDSAPrivateKey struct {
+	Curve   string
+	Pub     []byte
+	D       *big.Int
+	Comment string
+	Pad     []byte `ssh:"rest"`
+}
+
+func getPrivateKeyComment(pemBytes []byte) (string, error) {
+	block, _ := pem.Decode(pemBytes)
+
+	if block == nil {
+		return "", errors.New("ssh: no key found")
+	}
+
+	key := block.Bytes
+
+	if len(key) < len(privateKeyAuthMagic) || string(key[:len(privateKeyAuthMagic)]) != privateKeyAuthMagic {
+		return "", errors.New("ssh: invalid openssh private key format")
+	}
+	remaining := key[len(privateKeyAuthMagic):]
+
+	var w openSSHEncryptedPrivateKey
+	if err := ssh.Unmarshal(remaining, &w); err != nil {
+		return "", err
+	}
+	if w.NumKeys != 1 {
+		// We only support single key files, and so does OpenSSH.
+		// https://github.com/openssh/openssh-portable/blob/4103a3ec7/sshkey.c#L4171
+		return "", errors.New("ssh: multi-key files are not supported")
+	}
+
+	var pk1 openSSHPrivateKey
+	if err := ssh.Unmarshal(w.PrivKeyBlock, &pk1); err != nil || pk1.Check1 != pk1.Check2 {
+		if w.CipherName != "none" {
+			return "", x509.IncorrectPasswordError
+		}
+		return "", errors.New("ssh: malformed OpenSSH key")
+	}
+
+	switch pk1.Keytype {
+	case ssh.KeyAlgoRSA:
+		var key openSSHRSAPrivateKey
+		if err := ssh.Unmarshal(pk1.Rest, &key); err != nil {
+			return "", err
+		}
+
+		return key.Comment, nil
+	case ssh.KeyAlgoED25519:
+		var key openSSHEd25519PrivateKey
+		if err := ssh.Unmarshal(pk1.Rest, &key); err != nil {
+			return "", err
+		}
+		return key.Comment, nil
+	case ssh.KeyAlgoECDSA256, ssh.KeyAlgoECDSA384, ssh.KeyAlgoECDSA521:
+		var key openSSHECDSAPrivateKey
+		if err := ssh.Unmarshal(pk1.Rest, &key); err != nil {
+			return "", err
+		}
+		return key.Comment, nil
+	default:
+		return "", errors.New("ssh: unhandled key type")
+	}
+}

internal/provider/data_source_public_key.go

@@ -120,16 +120,18 @@ func (ds *publicKeyDataSource) Read(ctx context.Context, req datasource.ReadRequ
 	var prvKey crypto.PrivateKey
 	var algorithm Algorithm
 	var err error
+	var openSSHComment string
 
 	// Given the use of `ExactlyOneOf` in the Schema, we are guaranteed
 	// that either `private_key_pem` or `private_key_openssh` will be set.
 	var prvKeyArg types.String
+
 	if req.Config.GetAttribute(ctx, path.Root("private_key_pem"), &prvKeyArg); !prvKeyArg.IsNull() && !prvKeyArg.IsUnknown() {
 		tflog.Debug(ctx, "Parsing private key from PEM")
 		prvKey, algorithm, err = parsePrivateKeyPEM([]byte(prvKeyArg.ValueString()))
 	} else if req.Config.GetAttribute(ctx, path.Root("private_key_openssh"), &prvKeyArg); !prvKeyArg.IsNull() && !prvKeyArg.IsUnknown() {
 		tflog.Debug(ctx, "Parsing private key from OpenSSH PEM")
-		prvKey, algorithm, err = parsePrivateKeyOpenSSHPEM([]byte(prvKeyArg.ValueString()))
+		prvKey, algorithm, openSSHComment, err = parsePrivateKeyOpenSSHPEM([]byte(prvKeyArg.ValueString()))
 	}
 	if err != nil {
 		res.Diagnostics.AddError("Unable to parse private key", err.Error())
@@ -143,5 +145,5 @@ func (ds *publicKeyDataSource) Read(ctx context.Context, req datasource.ReadRequ
 	}
 
 	tflog.Debug(ctx, "Storing private key's public key info into the state")
-	res.Diagnostics.Append(setPublicKeyAttributes(ctx, &res.State, prvKey)...)
+	res.Diagnostics.Append(setPublicKeyAttributes(ctx, &res.State, prvKey, openSSHComment)...)
 }

internal/provider/data_source_public_key_test.go

@@ -240,6 +240,51 @@ func TestPublicKey_dataSource_PKCS8PEM(t *testing.T) {
 	})
 }
 
+func TestPublicKey_dataSource_OpenSSHComment(t *testing.T) {
+	r.UnitTest(t, r.TestCase{
+		ProtoV5ProviderFactories: protoV5ProviderFactories(),
+		Steps: []r.TestStep{
+			{
+				Config: `
+					resource "tls_private_key" "prvKey" {
+						algorithm = "RSA"
+						openssh_comment = "test@test"
+					}
+					data "tls_public_key" "pubKey" {
+						private_key_openssh = tls_private_key.prvKey.private_key_openssh
+					}
+				`,
+				Check: r.TestMatchResourceAttr("data.tls_public_key.pubKey", "public_key_openssh", regexp.MustCompile(` test@test\n$`)),
+			},
+			{
+				Config: `
+					resource "tls_private_key" "prvKey" {
+						algorithm = "ECDSA"
+						ecdsa_curve = "P384"
+						openssh_comment = "test@test"
+					}
+					data "tls_public_key" "pubKey" {
+						private_key_openssh = tls_private_key.prvKey.private_key_openssh
+					}
+				`,
+				Check: r.TestMatchResourceAttr("data.tls_public_key.pubKey", "public_key_openssh", regexp.MustCompile(` test@test\n$`)),
+			},
+			{
+				Config: `
+					resource "tls_private_key" "prvKey" {
+						algorithm = "ED25519"
+						openssh_comment = "test@test"
+					}
+					data "tls_public_key" "pubKey" {
+						private_key_openssh = tls_private_key.prvKey.private_key_openssh
+					}
+				`,
+				Check: r.TestMatchResourceAttr("data.tls_public_key.pubKey", "public_key_openssh", regexp.MustCompile(` test@test\n$`)),
+			},
+		},
+	})
+}
+
 func TestPublicKey_dataSource_errorCases(t *testing.T) {
 	r.UnitTest(t, r.TestCase{
 		ProtoV5ProviderFactories: protoV5ProviderFactories(),

internal/provider/ephemeral_private_key.go

@@ -50,6 +50,10 @@ func (p *privateKeyEphemeralResource) Schema(ctx context.Context, req ephemeral.
 			},
 
 			// Optional attributes
+			"openssh_comment": schema.StringAttribute{
+				Optional:            true,
+				MarkdownDescription: "Comment to add to the OpenSSH key (default: `\"\"`).",
+			},
 			"rsa_bits": schema.Int64Attribute{
 				Optional:            true,
 				Computed:            true,
@@ -216,7 +220,7 @@ func (p *privateKeyEphemeralResource) Open(ctx context.Context, req ephemeral.Op
 	tflog.Debug(ctx, "Marshalling private key to OpenSSH PEM (if supported)")
 	data.PrivateKeyOpenSSH = types.StringValue("")
 	if prvKeySupportsOpenSSHMarshalling(prvKey) {
-		openSSHKeyPemBlock, err := ssh.MarshalPrivateKey(prvKey, "")
+		openSSHKeyPemBlock, err := ssh.MarshalPrivateKey(prvKey, data.OpenSSHComment.ValueString())
 		if err != nil {
 			res.Diagnostics.AddError("Unable to marshal private key into OpenSSH format", err.Error())
 			return

internal/provider/ephemeral_private_key_test.go

@@ -180,6 +180,37 @@ func TestAccEphemeralPrivateKey_ED25519(t *testing.T) {
 	})
 }
 
+func TestAccEphemeralPrivateKey_OpenSSHComment(t *testing.T) {
+	r.UnitTest(t, r.TestCase{
+		// Ephemeral resources are only available in 1.10 and later
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.SkipBelow(tfversion.Version1_10_0),
+		},
+		ProtoV5ProviderFactories: protoV5ProviderFactories(),
+		ProtoV6ProviderFactories: map[string]func() (tfprotov6.ProviderServer, error){
+			"echo": echoprovider.NewProviderServer(),
+		},
+		Steps: []r.TestStep{
+			{
+				Config: ephemeralPrivateKeyWithEchoConfig(`ephemeral "tls_private_key" "test" {
+						algorithm = "ED25519"
+						openssh_comment = "test@test"
+					}`),
+				Check: r.ComposeAggregateTestCheckFunc(
+					tu.TestCheckPEMFormat("echo.tls_private_key_test", "data.private_key_pem", PreamblePrivateKeyPKCS8.String()),
+					tu.TestCheckPEMFormat("echo.tls_private_key_test", "data.public_key_pem", PreamblePublicKey.String()),
+					tu.TestCheckPEMFormat("echo.tls_private_key_test", "data.private_key_openssh", PreamblePrivateKeyOpenSSH.String()),
+					tu.TestCheckPEMFormat("echo.tls_private_key_test", "data.private_key_pem_pkcs8", PreamblePrivateKeyPKCS8.String()),
+					r.TestMatchResourceAttr("echo.tls_private_key_test", "data.public_key_openssh", regexp.MustCompile(`^ssh-ed25519 `)),
+					r.TestMatchResourceAttr("echo.tls_private_key_test", "data.public_key_openssh", regexp.MustCompile(` test@test\n$`)),
+					r.TestMatchResourceAttr("echo.tls_private_key_test", "data.public_key_fingerprint_md5", regexp.MustCompile(`^([abcdef\d]{2}:){15}[abcdef\d]{2}`)),
+					r.TestMatchResourceAttr("echo.tls_private_key_test", "data.public_key_fingerprint_sha256", regexp.MustCompile(`^SHA256:`)),
+				),
+			},
+		},
+	})
+}
+
 // Adds the test echo provider to enable using state checks with ephemeral resources.
 func ephemeralPrivateKeyWithEchoConfig(cfg string) string {
 	return fmt.Sprintf(`

internal/provider/ephemeral_public_key.go

@@ -122,6 +122,7 @@ func (p *publicKeyEphemeralResource) Open(ctx context.Context, req ephemeral.Ope
 	var prvKey crypto.PrivateKey
 	var algorithm Algorithm
 	var err error
+	var openSSHComment string
 
 	// Given the use of `ExactlyOneOf` in the Schema, we are guaranteed
 	// that either `private_key_pem` or `private_key_openssh` will be set.
@@ -131,7 +132,7 @@ func (p *publicKeyEphemeralResource) Open(ctx context.Context, req ephemeral.Ope
 		prvKey, algorithm, err = parsePrivateKeyPEM([]byte(prvKeyArg.ValueString()))
 	} else if req.Config.GetAttribute(ctx, path.Root("private_key_openssh"), &prvKeyArg); !prvKeyArg.IsNull() {
 		tflog.Debug(ctx, "Parsing private key from OpenSSH PEM")
-		prvKey, algorithm, err = parsePrivateKeyOpenSSHPEM([]byte(prvKeyArg.ValueString()))
+		prvKey, algorithm, openSSHComment, err = parsePrivateKeyOpenSSHPEM([]byte(prvKeyArg.ValueString()))
 	}
 	if err != nil {
 		resp.Diagnostics.AddError("Unable to parse private key", err.Error())
@@ -143,5 +144,5 @@ func (p *publicKeyEphemeralResource) Open(ctx context.Context, req ephemeral.Ope
 		return
 	}
 
-	resp.Diagnostics.Append(setPublicKeyAttributesEphemeral(ctx, &resp.Result, prvKey)...)
+	resp.Diagnostics.Append(setPublicKeyAttributesEphemeral(ctx, &resp.Result, prvKey, openSSHComment)...)
 }