improve docs and implementation of Signature, PrivateKey and `Pub…

…licKey`

This commit improves the documentation for the core minisign types and
improves the implementation for various functions. It also adds an
additional test vector for (un)marshaling private keys.

Signed-off-by: Andreas Auernhammer <github@aead.dev>

.golangci.yml

@@ -1,7 +1,4 @@
 linters-settings:
-  golint:
-    min-confidence: 0
-
   misspell:
     locale: US
 
@@ -11,24 +8,27 @@ linters-settings:
 linters:
   disable-all: true
   enable:
-    - typecheck
+    - durationcheck
+    - gocritic
+    - gofmt
     - goimports
-    - misspell
-    - staticcheck
+    - gomodguard
     - govet
-    - revive
     - ineffassign
-    - gosimple
-    - unused
-    - prealloc
+    - misspell
+    - revive
+    - staticcheck
+    - tenv
+    - typecheck
     - unconvert
-    - gofumpt
+    - unused
 
 issues:
   exclude-use-default: false
   exclude:
-      - "var-naming: don't use ALL_CAPS in Go names; use CamelCase"
       - "package-comments: should have a package comment"
+      - "exitAfterDefer:"
+      - "captLocal:"
 
 service:
-  golangci-lint-version: 1.48.0 # use the fixed version to not introduce new linters unexpectedly
+  golangci-lint-version: 1.57.2 # use the fixed version to not introduce new linters unexpectedly

internal/testdata/unencrypted.key

@@ -0,0 +1,5 @@
+untrusted comment: minisign encrypted secret key
+RWQAAEIyAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAbuUYgQpHKDcmmMQj9cgqohWX321PrXUDFfCVWOXDZp8kLw2/qju66KnI28LcOaA7ZywNP5vDVtlHeyzit3lxeqirS5+2UImrAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=
+
+untrusted comment: minisign encrypted secret key
+RWQAAEIyAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAb/yydu4x5dcvbgaLZRtY5v8wFvgzMkvKyALUXUWcT+bvaqFvuvkUyUfMd7ozqYIs8zOaPqWf6EjnWSqkOpOQiD1UJpOgCFm0AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=

minisign.pub

@@ -0,0 +1,2 @@
+untrusted comment: minisign public key D7E531EE76B2FC6F
+RWRv/LJ27jHl10fMd7ozqYIs8zOaPqWf6EjnWSqkOpOQiD1UJpOgCFm0

private.go

@@ -106,6 +106,8 @@ func (p PrivateKey) Equal(x crypto.PrivateKey) bool {
 //
 // For password-protected private keys refer to [EncryptKey].
 func (p PrivateKey) MarshalText() ([]byte, error) {
+	// A non-encrypted private key has the same format as an encrypted one.
+	// However, the salt, and auth. tag are set to all zero.
 	var b [privateKeySize]byte
 
 	binary.LittleEndian.PutUint16(b[:], EdDSA)
@@ -115,6 +117,8 @@ func (p PrivateKey) MarshalText() ([]byte, error) {
 	binary.LittleEndian.PutUint64(b[54:], p.id)
 	copy(b[62:], p.bytes[:])
 
+	// It seems odd that the comment says: "encrypted secret key".
+	// However, the original C implementation behaves like this.
 	const comment = "untrusted comment: minisign encrypted secret key\n"
 	encodedBytes := make([]byte, len(comment)+base64.StdEncoding.EncodedLen(len(b)))
 	copy(encodedBytes, []byte(comment))
@@ -140,7 +144,7 @@ func (p *PrivateKey) UnmarshalText(text []byte) error {
 	}
 
 	var (
-		empty [32]byte
+		empty [32]byte // For checking that the salt/tag are empty
 
 		kType     = binary.LittleEndian.Uint16(b)
 		kdf       = binary.LittleEndian.Uint16(b[2:])
@@ -149,7 +153,7 @@ func (p *PrivateKey) UnmarshalText(text []byte) error {
 		scryptOps = binary.LittleEndian.Uint64(b[38:])
 		scryptMem = binary.LittleEndian.Uint64(b[46:])
 		key       = b[54:126]
-		checksum  = b[126:privateKeySize]
+		tag       = b[126:privateKeySize]
 	)
 	if kType != EdDSA {
 		return fmt.Errorf("minisign: invalid private key: invalid key type '%d'", kType)
@@ -163,7 +167,7 @@ func (p *PrivateKey) UnmarshalText(text []byte) error {
 	if hType != algorithmBlake2b {
 		return fmt.Errorf("minisign: invalid private key: invalid hash type '%d'", hType)
 	}
-	if !bytes.Equal(salt[:], empty[:]) {
+	if !bytes.Equal(salt, empty[:]) {
 		return errors.New("minisign: invalid private key: salt is not empty")
 	}
 	if scryptOps != 0 {
@@ -172,11 +176,11 @@ func (p *PrivateKey) UnmarshalText(text []byte) error {
 	if scryptMem != 0 {
 		return errors.New("minisign: invalid private key: scrypt mem parameter is not zero")
 	}
-	if !bytes.Equal(checksum, empty[:]) {
+	if !bytes.Equal(tag, empty[:]) {
 		return errors.New("minisign: invalid private key: salt is not empty")
 	}
 
-	p.id = binary.LittleEndian.Uint64(key[:8])
+	p.id = binary.LittleEndian.Uint64(key)
 	copy(p.bytes[:], key[8:])
 	return nil
 }
@@ -235,10 +239,7 @@ func IsEncrypted(privateKey []byte) bool {
 	}
 	bytes = bytes[:n]
 
-	if len(bytes) != privateKeySize {
-		return false
-	}
-	return binary.LittleEndian.Uint16(bytes[2:4]) == algorithmScrypt
+	return len(bytes) >= 4 && binary.LittleEndian.Uint16(bytes[2:]) == algorithmScrypt
 }
 
 var errDecrypt = errors.New("minisign: decryption failed")
@@ -247,47 +248,50 @@ var errDecrypt = errors.New("minisign: decryption failed")
 // the given password.
 func DecryptKey(password string, privateKey []byte) (PrivateKey, error) {
 	privateKey = trimUntrustedComment(privateKey)
-	bytes := make([]byte, base64.StdEncoding.DecodedLen(len(privateKey)))
-	n, err := base64.StdEncoding.Decode(bytes, privateKey)
+	b := make([]byte, base64.StdEncoding.DecodedLen(len(privateKey)))
+	n, err := base64.StdEncoding.Decode(b, privateKey)
 	if err != nil {
 		return PrivateKey{}, err
 	}
-	bytes = bytes[:n]
+	b = b[:n]
 
-	if len(bytes) != privateKeySize {
+	if len(b) != privateKeySize {
 		return PrivateKey{}, errDecrypt
 	}
-	if a := binary.LittleEndian.Uint16(bytes[:2]); a != EdDSA {
+	var (
+		kType      = binary.LittleEndian.Uint16(b)
+		kdf        = binary.LittleEndian.Uint16(b[2:])
+		hType      = binary.LittleEndian.Uint16(b[4:])
+		salt       = b[6:38]
+		scryptOps  = binary.LittleEndian.Uint64(b[38:])
+		scryptMem  = binary.LittleEndian.Uint64(b[46:])
+		ciphertext = b[54:]
+	)
+	if kType != EdDSA {
 		return PrivateKey{}, errDecrypt
 	}
-	if a := binary.LittleEndian.Uint16(bytes[2:4]); a != algorithmScrypt {
+	if kdf != algorithmScrypt {
 		return PrivateKey{}, errDecrypt
 	}
-	if a := binary.LittleEndian.Uint16(bytes[4:6]); a != algorithmBlake2b {
+	if hType != algorithmBlake2b {
 		return PrivateKey{}, errDecrypt
 	}
-
-	var (
-		scryptOps = binary.LittleEndian.Uint64(bytes[38:46])
-		scryptMem = binary.LittleEndian.Uint64(bytes[46:54])
-	)
 	if scryptOps > scryptOpsLimit {
 		return PrivateKey{}, errDecrypt
 	}
 	if scryptMem > scryptMemLimit {
 		return PrivateKey{}, errDecrypt
 	}
-	var salt [32]byte
-	copy(salt[:], bytes[6:38])
-	privateKeyBytes, err := decryptKey(password, salt[:], scryptOps, scryptMem, bytes[54:])
+
+	plaintext, err := decryptKey(password, salt, scryptOps, scryptMem, ciphertext)
 	if err != nil {
 		return PrivateKey{}, err
 	}
 
 	key := PrivateKey{
-		id: binary.LittleEndian.Uint64(privateKeyBytes[:8]),
+		id: binary.LittleEndian.Uint64(plaintext),
 	}
-	copy(key.bytes[:], privateKeyBytes[8:])
+	copy(key.bytes[:], plaintext[8:])
 	return key, nil
 }
 
@@ -368,7 +372,7 @@ func decryptKey(password string, salt []byte, ops, mem uint64, ciphertext []byte
 	binary.LittleEndian.PutUint16(message[:2], EdDSA)
 	copy(message[2:], privateKeyBytes)
 
-	if sum := blake2b.Sum256(message[:]); subtle.ConstantTimeCompare(sum[:], checksum[:]) != 1 {
+	if sum := blake2b.Sum256(message[:]); subtle.ConstantTimeCompare(sum[:], checksum) != 1 {
 		return nil, errDecrypt
 	}
 	return privateKeyBytes, nil

private_test.go

@@ -6,21 +6,98 @@ package minisign
 
 import (
 	"bytes"
+	"encoding/base64"
 	"os"
+	"runtime"
+	"strconv"
 	"testing"
 )
 
+var marshalPrivateKeyTests = []struct {
+	ID    uint64
+	Bytes []byte
+}{
+	{
+		ID:    htoi("3728470A8118E56E"),
+		Bytes: b64("JpjEI/XIKqIVl99tT611AxXwlVjlw2afJC8Nv6o7uuipyNvC3DmgO2csDT+bw1bZR3ss4rd5cXqoq0uftlCJqw=="),
+	},
+	{
+		ID:    htoi("D7E531EE76B2FC6F"),
+		Bytes: b64("L24Gi2UbWOb/MBb4MzJLysgC1F1FnE/m72qhb7r5FMlHzHe6M6mCLPMzmj6ln+hI51kqpDqTkIg9VCaToAhZtA=="),
+	},
+}
+
+func TestPrivateKey_Marshal(t *testing.T) {
+	raw, err := os.ReadFile("./internal/testdata/unencrypted.key")
+	if err != nil {
+		t.Fatalf("Failed to read private key: %v", err)
+	}
+
+	var newline string
+	if runtime.GOOS == "windows" {
+		newline = "\r\n"
+	} else {
+		newline = "\n"
+	}
+
+	keys := bytes.Split(raw, []byte(newline+newline)) // Private keys are separated by a newline
+	if len(keys) != len(marshalPrivateKeyTests) {
+		t.Fatalf("Test vectors don't match: got %d - want %d", len(marshalPrivateKeyTests), len(keys))
+	}
+	for i, test := range marshalPrivateKeyTests {
+		keys[i] = bytes.TrimSuffix(keys[i], []byte(newline))
+
+		key := PrivateKey{
+			id: test.ID,
+		}
+		copy(key.bytes[:], test.Bytes)
+
+		text, err := key.MarshalText()
+		if err != nil {
+			t.Fatalf("Test %d: failed to marshal private key: %v", i, err)
+		}
+		if !bytes.Equal(text, keys[i]) {
+			t.Fatalf("Test %d: failed to marshal private key:\nGot: %s\nWant: %s\n", i, string(text), string(keys[i]))
+		}
+	}
+}
+
 func TestPrivateKey_Unmarshal(t *testing.T) {
-	raw, err := os.ReadFile("./internal/testdata/minisign_unencrypted.key")
+	raw, err := os.ReadFile("./internal/testdata/unencrypted.key")
 	if err != nil {
 		t.Fatalf("Failed to read private key: %v", err)
 	}
 
-	keys := bytes.Split(raw, []byte("\n\n")) // Private keys are separated by a newline
+	var keys [][]byte // Private keys are separated by a newline
+	if runtime.GOOS == "windows" {
+		keys = bytes.Split(raw, []byte("\r\n\r\n"))
+	} else {
+		keys = bytes.Split(raw, []byte("\n\n"))
+	}
+
 	for _, k := range keys {
 		var key PrivateKey
 		if err := key.UnmarshalText(k); err != nil {
 			t.Fatalf("Failed to unmarshal private key: %v\nPrivate key:\n%s", err, string(k))
 		}
+
+		// Print test vector for marshaling:
+		// t.Logf("\n{\n\tID: htoi(\"%X\"),\n\tBytes: b64(\"%s\"),\n}", key.id, base64.StdEncoding.EncodeToString(key.bytes[:]))
+	}
+}
+
+func htoi(s string) uint64 {
+	i, err := strconv.ParseUint(s, 16, 64)
+	if err != nil {
+		panic(err)
+	}
+	return i
+}
+
+func b64(s string) []byte {
+	b, err := base64.StdEncoding.DecodeString(s)
+	if err != nil {
+		panic(err)
 	}
+	return b
 }

public.go

@@ -16,10 +16,11 @@ import (
 	"strings"
 )
 
-// PublicKeyFromFile reads a new PublicKey from the
-// given file.
-func PublicKeyFromFile(path string) (PublicKey, error) {
-	bytes, err := os.ReadFile(path)
+const publicKeySize = 2 + 8 + ed25519.PublicKeySize
+
+// PublicKeyFromFile reads a PublicKey from the given file.
+func PublicKeyFromFile(filename string) (PublicKey, error) {
+	bytes, err := os.ReadFile(filename)
 	if err != nil {
 		return PublicKey{}, err
 	}
@@ -57,7 +58,7 @@ func (p PublicKey) Equal(x crypto.PublicKey) bool {
 
 // String returns a base64 string representation of the PublicKey p.
 func (p PublicKey) String() string {
-	var bytes [2 + 8 + ed25519.PublicKeySize]byte
+	var bytes [publicKeySize]byte
 	binary.LittleEndian.PutUint16(bytes[:2], EdDSA)
 	binary.LittleEndian.PutUint64(bytes[2:10], p.ID())
 	copy(bytes[10:], p.bytes[:])
@@ -69,22 +70,27 @@ func (p PublicKey) String() string {
 //
 // It never returns an error.
 func (p PublicKey) MarshalText() ([]byte, error) {
-	comment := "untrusted comment: minisign public key: " + strings.ToUpper(strconv.FormatUint(p.ID(), 16)) + "\n"
-	return []byte(comment + p.String()), nil
+	s := make([]byte, 0, 113) // Size of a public key in text format
+	s = append(s, "untrusted comment: minisign public key: "...)
+	s = append(s, strings.ToUpper(strconv.FormatUint(p.ID(), 16))...)
+	s = append(s, '\n')
+	s = append(s, p.String()...)
+	return s, nil
 }
 
-// UnmarshalText parses text as textual-encoded public key.
-// It returns an error if text is not a well-formed public key.
+// UnmarshalText decodes a textual representation of a public key into p.
+//
+// It returns an error in case of a malformed key.
 func (p *PublicKey) UnmarshalText(text []byte) error {
 	text = trimUntrustedComment(text)
 	bytes := make([]byte, base64.StdEncoding.DecodedLen(len(text)))
 	n, err := base64.StdEncoding.Decode(bytes, text)
 	if err != nil {
 		return fmt.Errorf("minisign: invalid public key: %v", err)
 	}
-	bytes = bytes[:n] // Adjust b/c text may contain '\r' or '\n' which would have been ignored during decoding.
+	bytes = bytes[:n] // Adjust since text may contain '\r' or '\n' which would have been ignored during decoding.
 
-	if n = len(bytes); n != 2+8+ed25519.PublicKeySize {
+	if n = len(bytes); n != publicKeySize {
 		return errors.New("minisign: invalid public key length " + strconv.Itoa(n))
 	}
 	if a := binary.LittleEndian.Uint16(bytes[:2]); a != EdDSA {