Stop formatting signers as raw private keys

identity/identity.go

@@ -20,23 +20,24 @@ type Identity struct {
 // New creates a new identity. If privateKeyBase64 is empty, generates a new
 // key. If serviceDID is empty, uses the key DID derived from the key.
 func New(privateKeyBase64 string, serviceDID string) (Identity, error) {
-	var signer multikey.Signer
+	var keySigner multikey.Signer
 	var issuer multikey.Issuer
 	var err error
 
 	if privateKeyBase64 == "" {
 		// Generate ephemeral identity
-		signer, err = ed25519.Generate()
+		keySigner, err = ed25519.Generate()
 		if err != nil {
 			return Identity{}, fmt.Errorf("failed to generate signer: %w", err)
 		}
 	} else {
 		// Decode provided key
-		signer, err = ed25519.Parse(privateKeyBase64)
+		keySigner, err = ed25519.Parse(privateKeyBase64)
 		if err != nil {
 			return Identity{}, fmt.Errorf("failed to create signer from key: %w", err)
 		}
 	}
+	signer := NewSigner(keySigner)
 
 	if serviceDID == "" {
 		issuer = multikey.KeyIssuer(signer)

identity/pem.go

@@ -13,7 +13,11 @@ import (
 
 // EncodeSignerToPEM encodes a signer to a PKCS#8 PEM format. The signer's key
 // should be of a type supported by ["crypto/x509".MarshalPKCS8PrivateKey].
-func EncodeSignerToPEM(signer multikey.Signer) ([]byte, error) {
+func EncodeSignerToPEM(keySigner multikey.Signer) ([]byte, error) {
+	// Wrap the signer so the error messages below name it by its DID instead of
+	// printing its private key bytes.
+	signer := NewSigner(keySigner)
+
 	privateKeyBytes, err := x509.MarshalPKCS8PrivateKey(signer.PrivateKey())
 	if err != nil {
 		return nil, fmt.Errorf("marshaling private key of signer %s: %w", signer, err)
@@ -34,7 +38,7 @@ func EncodeSignerToPEM(signer multikey.Signer) ([]byte, error) {
 
 // DecodeSignerFromPEM loads a private key from a PKCS#8 PEM as a signer.
 // Currently, only Ed25519 keys are supported.
-func DecodeSignerFromPEM(pemData []byte) (multikey.Signer, error) {
+func DecodeSignerFromPEM(pemData []byte) (Signer, error) {
 	var privateKey *crypto_ed25519.PrivateKey
 	rest := pemData
 	for {
@@ -47,21 +51,26 @@ func DecodeSignerFromPEM(pemData []byte) (multikey.Signer, error) {
 		if block.Type == "PRIVATE KEY" {
 			parsedKey, err := x509.ParsePKCS8PrivateKey(block.Bytes)
 			if err != nil {
-				return nil, fmt.Errorf("parsing PKCS#8 private key: %w", err)
+				return Signer{}, fmt.Errorf("parsing PKCS#8 private key: %w", err)
 			}
 
 			key, ok := parsedKey.(crypto_ed25519.PrivateKey)
 			if !ok {
-				return nil, fmt.Errorf("key is not an Ed25519 private key")
+				return Signer{}, fmt.Errorf("key is not an Ed25519 private key")
 			}
 			privateKey = &key
 			break
 		}
 	}
 
 	if privateKey == nil {
-		return nil, fmt.Errorf("no PRIVATE KEY block found in PEM file")
+		return Signer{}, fmt.Errorf("no PRIVATE KEY block found in PEM file")
+	}
+
+	signer, err := ed25519.FromRaw(privateKey.Seed())
+	if err != nil {
+		return Signer{}, fmt.Errorf("creating signer from private key: %w", err)
 	}
 
-	return ed25519.FromRaw(privateKey.Seed())
+	return NewSigner(signer), nil
 }

identity/pem_test.go

@@ -4,6 +4,7 @@ import (
 	"testing"
 
 	"github.com/fil-forge/libforge/identity"
+	"github.com/fil-forge/ucantone/multikey"
 	"github.com/fil-forge/ucantone/multikey/ed25519"
 	"github.com/stretchr/testify/require"
 )
@@ -24,6 +25,35 @@ func TestEd25519SignerPEMRoundTrip(t *testing.T) {
 	require.Equal(t, original.KeyDID(), decoded.KeyDID())
 }
 
+// unmarshalableSigner reports a private key that
+// ["crypto/x509".MarshalPKCS8PrivateKey] cannot encode, to exercise the error
+// path of EncodeSignerToPEM.
+type unmarshalableSigner struct {
+	multikey.Signer
+}
+
+func (s unmarshalableSigner) PrivateKey() any {
+	return struct{}{}
+}
+
+func TestEncodeSignerToPEM_MarshalErrorNamesSignerByDID(t *testing.T) {
+	keySigner, err := ed25519.Generate()
+	require.NoError(t, err)
+
+	_, err = identity.EncodeSignerToPEM(unmarshalableSigner{keySigner})
+
+	require.ErrorContains(t, err, "marshaling private key of signer "+keySigner.KeyDID().String())
+}
+
+func TestEncodeSignerToPEM_MarshalErrorHidesPrivateKey(t *testing.T) {
+	keySigner, err := ed25519.Generate()
+	require.NoError(t, err)
+
+	_, err = identity.EncodeSignerToPEM(unmarshalableSigner{keySigner})
+
+	require.NotContains(t, err.Error(), string(keySigner.Raw()))
+}
+
 func TestDecodeEd25519SignerFromPEM_NoPrivateKeyBlock(t *testing.T) {
 	pemData := []byte("-----BEGIN CERTIFICATE-----\nMIIB\n-----END CERTIFICATE-----\n")
 	_, err := identity.DecodeSignerFromPEM(pemData)

identity/signer.go

@@ -0,0 +1,27 @@
+package identity
+
+import (
+	"github.com/fil-forge/ucantone/multikey"
+)
+
+// Signer is a [multikey.Signer] that is safe to pass to formatting and logging
+// functions. Multikey signers are byte slices holding the private key, so
+// formatting one directly writes the key material into the output.
+//
+// Prefer this type over [multikey.Signer] wherever libforge holds a signer.
+type Signer struct {
+	multikey.Signer
+}
+
+var _ multikey.Signer = Signer{}
+
+// NewSigner wraps a multikey signer so that formatting it is safe.
+func NewSigner(signer multikey.Signer) Signer {
+	return Signer{Signer: signer}
+}
+
+// String returns the DID of the signer's key. It never returns private key
+// material.
+func (s Signer) String() string {
+	return s.KeyDID().String()
+}

identity/signer_test.go

@@ -0,0 +1,41 @@
+package identity_test
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/fil-forge/libforge/identity"
+	"github.com/fil-forge/ucantone/multikey/ed25519"
+	"github.com/stretchr/testify/require"
+)
+
+func TestSignerStringReturnsKeyDID(t *testing.T) {
+	keySigner, err := ed25519.Generate()
+	require.NoError(t, err)
+
+	signer := identity.NewSigner(keySigner)
+
+	require.Equal(t, keySigner.KeyDID().String(), signer.String())
+}
+
+// Formatting verbs a signer may plausibly reach in an error message or a log
+// line. None of them may print the private key.
+var signerFormatVerbs = map[string]string{
+	"%s": "%s",
+	"%v": "%s",
+	"%q": "%q",
+}
+
+func TestSignerFormattingPrintsKeyDID(t *testing.T) {
+	for verb, expectedVerb := range signerFormatVerbs {
+		t.Run(verb, func(t *testing.T) {
+			keySigner, err := ed25519.Generate()
+			require.NoError(t, err)
+
+			formatted := fmt.Sprintf(verb, identity.NewSigner(keySigner))
+
+			expected := fmt.Sprintf(expectedVerb, keySigner.KeyDID().String())
+			require.Equal(t, expected, formatted)
+		})
+	}
+}