Merge pull request ipfs/go-ipns#41 from ipfs/chore/require-v2-signatures

fix: require V2 signatures

This commit was moved from ipfs/go-ipns@1af8e5c

Author: lidel

ipns/README.md

@@ -1,14 +1,13 @@
 # go-ipns
 
-[![](https://img.shields.io/badge/made%20by-Protocol%20Labs-blue.svg?style=flat-square)](http://protocol.ai)
-[![](https://img.shields.io/badge/project-IPFS-blue.svg?style=flat-square)](http://ipfs.io/)
-[![](https://img.shields.io/badge/freenode-%23ipfs-blue.svg?style=flat-square)](http://webchat.freenode.net/?channels=%23ipfs)
+[![](https://img.shields.io/badge/made%20by-Protocol%20Labs-blue.svg?style=flat-square)](https://protocol.ai)
+[![](https://img.shields.io/badge/project-IPFS-blue.svg?style=flat-square)](https://ipfs.tech/)
 [![standard-readme compliant](https://img.shields.io/badge/standard--readme-OK-green.svg?style=flat-square)](https://github.com/RichardLitt/standard-readme)
 [![GoDoc](https://godoc.org/github.com/ipfs/go-datastore?status.svg)](https://godoc.org/github.com/ipfs/go-ipns)
 
 > ipns record definitions
 
-This package contains all of the components necessary to create, understand, and validate IPNS records. It does *not* publish or resolve those records. [`go-ipfs`](https://github.com/ipfs/go-ipfs) uses this package internally to manipulate records.
+This package contains all of the components necessary to create, understand, and validate IPNS records. It does *not* publish or resolve those records. [Kubo](https://github.com/ipfs/kubo) uses this package internally to manipulate records.
 
 ## Lead Maintainer
 

ipns/errors.go

@@ -35,3 +35,10 @@ var ErrPublicKeyMismatch = errors.New("public key in record did not match expect
 
 // ErrBadRecord should be returned when an ipns record cannot be unmarshalled
 var ErrBadRecord = errors.New("record could not be unmarshalled")
+
+// 10 KiB limit defined in https://github.com/ipfs/specs/pull/319
+const MaxRecordSize int = 10 << (10 * 1)
+
+// ErrRecordSize should be returned when an ipns record is
+// invalid due to being too big
+var ErrRecordSize = errors.New("record exceeds allowed size limit")

ipns/ipns.go

@@ -53,6 +53,10 @@ func Create(sk ic.PrivKey, val []byte, seq uint64, eol time.Time, ttl time.Durat
 	}
 	entry.Data = cborData
 
+	// For now we still create V1 signatures. These are deprecated, and not
+	// used during verification anymore (Validate func requires SignatureV2),
+	// but setting it here allows legacy nodes (e.g., go-ipfs < v0.9.0) to
+	// still resolve IPNS published by modern nodes.
 	sig1, err := sk.Sign(ipnsEntryDataForSigV1(entry))
 	if err != nil {
 		return nil, errors.Wrap(err, "could not compute signature data")
@@ -128,28 +132,31 @@ func createCborDataForIpnsEntry(e *pb.IpnsEntry) ([]byte, error) {
 
 // Validates validates the given IPNS entry against the given public key.
 func Validate(pk ic.PubKey, entry *pb.IpnsEntry) error {
+	// Make sure max size is respected
+	if entry.Size() > MaxRecordSize {
+		return ErrRecordSize
+	}
+
 	// Check the ipns record signature with the public key
+	if entry.GetSignatureV2() == nil {
+		// always error if no valid signature could be found
+		return ErrSignature
+	}
 
-	// Check v2 signature if it's available, otherwise use the v1 signature
-	if entry.GetSignatureV2() != nil {
-		sig2Data, err := ipnsEntryDataForSigV2(entry)
-		if err != nil {
-			return fmt.Errorf("could not compute signature data: %w", err)
-		}
-		if ok, err := pk.Verify(sig2Data, entry.GetSignatureV2()); err != nil || !ok {
-			return ErrSignature
-		}
+	sig2Data, err := ipnsEntryDataForSigV2(entry)
+	if err != nil {
+		return fmt.Errorf("could not compute signature data: %w", err)
+	}
+	if ok, err := pk.Verify(sig2Data, entry.GetSignatureV2()); err != nil || !ok {
+		return ErrSignature
+	}
 
-		// TODO: If we switch from pb.IpnsEntry to a more generic IpnsRecord type then perhaps we should only check
-		// this if there is no v1 signature. In the meanwhile this helps avoid some potential rough edges around people
-		// checking the entry fields instead of doing CBOR decoding everywhere.
-		if err := validateCborDataMatchesPbData(entry); err != nil {
-			return err
-		}
-	} else {
-		if ok, err := pk.Verify(ipnsEntryDataForSigV1(entry), entry.GetSignatureV1()); err != nil || !ok {
-			return ErrSignature
-		}
+	// TODO: If we switch from pb.IpnsEntry to a more generic IpnsRecord type then perhaps we should only check
+	// this if there is no v1 signature. In the meanwhile this helps avoid some potential rough edges around people
+	// checking the entry fields instead of doing CBOR decoding everywhere.
+	// See https://github.com/ipfs/go-ipns/pull/42 for next steps here
+	if err := validateCborDataMatchesPbData(entry); err != nil {
+		return err
 	}
 
 	eol, err := GetEOL(entry)

ipns/validate_test.go

@@ -178,7 +178,7 @@ func TestPeerIDPubKeyValidate(t *testing.T) {
 	testValidatorCase(t, sk, kbook, ipnsk, dataNoKey, goodeol, nil)
 }
 
-func TestBothSignatureVersionsValidate(t *testing.T) {
+func TestOnlySignatureV2Validate(t *testing.T) {
 	goodeol := time.Now().Add(time.Hour)
 
 	sk, pk, err := crypto.GenerateEd25519Key(rand.New(rand.NewSource(42)))
@@ -197,17 +197,12 @@ func TestBothSignatureVersionsValidate(t *testing.T) {
 	}
 
 	entry.SignatureV2 = nil
-	if err := Validate(pk, entry); err != nil {
-		t.Fatal(err)
-	}
-
-	entry.SignatureV1 = nil
 	if err := Validate(pk, entry); !errors.Is(err, ErrSignature) {
 		t.Fatal(err)
 	}
 }
 
-func TestNewSignatureVersionPreferred(t *testing.T) {
+func TestSignatureV1Ignored(t *testing.T) {
 	goodeol := time.Now().Add(time.Hour)
 
 	sk, pk, err := crypto.GenerateEd25519Key(rand.New(rand.NewSource(42)))
@@ -251,13 +246,13 @@ func TestNewSignatureVersionPreferred(t *testing.T) {
 		t.Fatal("entry2 should be better than entry1")
 	}
 
-	// Having only the v1 signature should be valid
+	// Having only the v1 signature should be invalid
 	entry2.SignatureV2 = nil
-	if err := Validate(pk, entry2); err != nil {
+	if err := Validate(pk, entry2); !errors.Is(err, ErrSignature) {
 		t.Fatal(err)
 	}
 
-	// However the v2 signature should be preferred
+	// Record with v2 signature should always be preferred
 	best, err = v.Select(ipnsk, [][]byte{mustMarshal(t, entry1), mustMarshal(t, entry2)})
 	if err != nil {
 		t.Fatal(err)
@@ -279,6 +274,26 @@ func TestNewSignatureVersionPreferred(t *testing.T) {
 	}
 }
 
+func TestMaxSizeValidate(t *testing.T) {
+	goodeol := time.Now().Add(time.Hour)
+
+	sk, pk, err := crypto.GenerateEd25519Key(rand.New(rand.NewSource(42)))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Create record over the max size (value+other fields)
+	value := make([]byte, MaxRecordSize)
+	entry, err := Create(sk, value, 1, goodeol, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Must fail with ErrRecordSize
+	if err := Validate(pk, entry); !errors.Is(err, ErrRecordSize) {
+		t.Fatal(err)
+	}
+}
+
 func TestCborDataCanonicalization(t *testing.T) {
 	goodeol := time.Now().Add(time.Hour)