chore: apply @lidel feedback, tests

core/commands/keystore.go

@@ -697,8 +697,14 @@ type KeySignOutput struct {
 }
 
 var keySignCmd = &cmds.Command{
+	Status: cmds.Experimental,
 	Helptext: cmds.HelpText{
-		Tagline: "Generates a signature for the given data with a specified key.",
+		Tagline: "Generates a signature for the given data with a specified key. Useful for proving the key ownership.",
+		LongDescription: `
+Sign arbitrary bytes, such as to prove ownership of a Peer ID or an IPNS Name.
+To avoid signature reuse, the signed payload is always prefixed with
+"libp2p-key signed message:".
+`,
 	},
 	Options: []cmds.Option{
 		cmds.StringOption("key", "k", "The name of the key to use for signing."),
@@ -757,8 +763,13 @@ type KeyVerifyOutput struct {
 }
 
 var keyVerifyCmd = &cmds.Command{
+	Status: cmds.Experimental,
 	Helptext: cmds.HelpText{
 		Tagline: "Verify that the given data and signature match.",
+		LongDescription: `
+Verify if the given data and signatures match. To avoid the signature reuse,
+the signed payload is always prefixed with "libp2p-key signed message:".
+`,
 	},
 	Options: []cmds.Option{
 		cmds.StringOption("key", "k", "The name of the key to use for signing."),

core/coreapi/key.go

@@ -263,6 +263,8 @@ func (api *KeyAPI) Self(ctx context.Context) (coreiface.Key, error) {
 	return newKey("self", api.identity)
 }
 
+const signedMessagePrefix = "libp2p-key signed message:"
+
 func (api *KeyAPI) Sign(ctx context.Context, name string, data []byte) (coreiface.Key, []byte, error) {
 	var (
 		sk  crypto.PrivKey
@@ -288,7 +290,7 @@ func (api *KeyAPI) Sign(ctx context.Context, name string, data []byte) (coreifac
 		return nil, nil, err
 	}
 
-	data = append([]byte("libp2p-key signed message:"), data...)
+	data = append([]byte(signedMessagePrefix), data...)
 
 	sig, err := sk.Sign(data)
 	if err != nil {
@@ -310,14 +312,15 @@ func (api *KeyAPI) Verify(ctx context.Context, keyOrName string, signature, data
 	} else if sk, err := api.repo.Keystore().Get(keyOrName); err == nil {
 		name = keyOrName
 		pk = sk.GetPublic()
-	} else if pid, err := peer.Decode(keyOrName); err == nil {
+	} else if ipnsName, err := ipns.NameFromString(keyOrName); err == nil {
+		// This works for both IPNS names and Peer IDs.
 		name = ""
-		pk, err = pid.ExtractPublicKey()
+		pk, err = ipnsName.Peer().ExtractPublicKey()
 		if err != nil {
 			return nil, false, err
 		}
 	} else {
-		return nil, false, fmt.Errorf("'%q' is not a known key, or a valid peer id", keyOrName)
+		return nil, false, fmt.Errorf("'%q' is not a known key, an IPNS Name, or a valid PeerID", keyOrName)
 	}
 
 	pid, err := peer.IDFromPublicKey(pk)
@@ -330,7 +333,7 @@ func (api *KeyAPI) Verify(ctx context.Context, keyOrName string, signature, data
 		return nil, false, err
 	}
 
-	data = append([]byte("libp2p-key signed message:"), data...)
+	data = append([]byte(signedMessagePrefix), data...)
 
 	valid, err := pk.Verify(data, signature)
 	if err != nil {

core/coreiface/tests/key.go

@@ -5,6 +5,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/ipfs/boxo/ipns"
 	"github.com/ipfs/go-cid"
 	iface "github.com/ipfs/kubo/core/coreiface"
 	opt "github.com/ipfs/kubo/core/coreiface/options"
@@ -342,7 +343,11 @@ func (tp *TestSuite) TestSign(t *testing.T) {
 }
 
 func (tp *TestSuite) TestVerify(t *testing.T) {
+	t.Parallel()
+
 	t.Run("Verify Own Key", func(t *testing.T) {
+		t.Parallel()
+
 		ctx, cancel := context.WithCancel(context.Background())
 		defer cancel()
 
@@ -363,6 +368,8 @@ func (tp *TestSuite) TestVerify(t *testing.T) {
 	})
 
 	t.Run("Verify Self", func(t *testing.T) {
+		t.Parallel()
+
 		ctx, cancel := context.WithCancel(context.Background())
 		defer cancel()
 
@@ -379,7 +386,10 @@ func (tp *TestSuite) TestVerify(t *testing.T) {
 		require.True(t, valid)
 	})
 
-	t.Run("Verify With Key CID", func(t *testing.T) {
+	t.Run("Verify With Key In Different Formats", func(t *testing.T) {
+		t.Parallel()
+
+		// Spin some node and get signature out.
 		ctx, cancel := context.WithCancel(context.Background())
 		defer cancel()
 
@@ -394,11 +404,24 @@ func (tp *TestSuite) TestVerify(t *testing.T) {
 		_, signature, err := api.Key().Sign(ctx, "foo", data)
 		require.NoError(t, err)
 
-		_, err = api.Key().Remove(ctx, "foo")
-		require.NoError(t, err)
-
-		_, valid, err := api.Key().Verify(ctx, peer.ToCid(key.ID()).String(), signature, data)
-		require.NoError(t, err)
-		require.True(t, valid)
+		for _, testCase := range [][]string{
+			{"Base58 Encoded Peer ID", key.ID().String()},
+			{"CIDv1 Encoded Peer ID", peer.ToCid(key.ID()).String()},
+			{"CIDv1 Encoded IPNS Name", ipns.NameFromPeer(key.ID()).String()},
+			{"Prefixed IPNS Path", ipns.NameFromPeer(key.ID()).AsPath().String()},
+		} {
+			t.Run(testCase[0], func(t *testing.T) {
+				ctx, cancel := context.WithCancel(context.Background())
+				defer cancel()
+
+				// Spin new node.
+				api, err := tp.makeAPI(t, ctx)
+				require.NoError(t, err)
+
+				_, valid, err := api.Key().Verify(ctx, testCase[1], signature, data)
+				require.NoError(t, err)
+				require.True(t, valid)
+			})
+		}
 	})
 }

docs/changelogs/v0.25.md

@@ -58,12 +58,9 @@ For more information see https://github.com/ipfs/kubo/pull/9747.
 
 ##### Commands `ipfs key sign` and `ipfs key verify`
 
-The Kubo CLI `ipfs key` now includes two new subcommands: `sign` and `verify`.
-This allows the Kubo node to sign arbitrary cryptographic bytes. For security,
-these will always be prefixed with `libp2p-key signed message:`.
+This allows the Kubo node to sign arbitrary bytes to prove ownership of a PeerID or an IPNS Name. To avoid signature reuse, the signed payload is always prefixed with `libp2p-key signed message:`.
 
-These commands are also both available through the RPC client and implemented
-in `client/rpc`.
+These commands are also both available through the RPC client and implemented in `client/rpc`.
 
 For more information see https://github.com/ipfs/kubo/issues/10230.