tests: finish parsing and X25519 tests, distinguish HMAC errors

If the implementation re-encodes the header before checking the HMAC,
that would mask malleability issues: the HMAC check would fail because
the tests HMAC'd the original header, but an attacker could also produce
the right HMAC. Instead of duplicating every parsing tests (with the
original and re-encoded HMAC), we make the test framework distinguish
HMAC errors, which ensures bad encodings are recognized as such and not
bypassable HMAC errors.

internal/format/format.go

@@ -258,7 +258,7 @@ func Parse(input io.Reader) (*Header, io.Reader, error) {
 				return nil, nil, errorf("malformed closing line: %q", line)
 			}
 			h.MAC, err = DecodeString(args[0])
-			if err != nil {
+			if err != nil || len(h.MAC) != 32 {
 				return nil, nil, errorf("malformed closing line %q: %v", line, err)
 			}
 			break

internal/testkit/testkit.go

@@ -106,16 +106,22 @@ func (f *TestFile) Body(body []byte) {
 	}
 }
 
-func (f *TestFile) Stanza(args []string, body []byte) {
-	f.ArgsLine(args...)
-	f.Body(body)
-}
-
 func (f *TestFile) AEADBody(key, body []byte) {
 	aead, _ := chacha20poly1305.New(key)
 	f.Body(aead.Seal(nil, make([]byte, chacha20poly1305.NonceSize), body, nil))
 }
 
+func x25519(scalar, point []byte) []byte {
+	secret, err := curve25519.X25519(scalar, point)
+	if err != nil {
+		if err.Error() == "bad input point: low order point" {
+			return make([]byte, 32)
+		}
+		panic(err)
+	}
+	return secret
+}
+
 func (f *TestFile) X25519(identity []byte) {
 	f.X25519RecordIdentity(identity)
 	f.X25519NoRecordIdentity(identity)
@@ -127,11 +133,16 @@ func (f *TestFile) X25519RecordIdentity(identity []byte) {
 }
 
 func (f *TestFile) X25519NoRecordIdentity(identity []byte) {
-	recipient, _ := curve25519.X25519(identity, curve25519.Basepoint)
-	ephemeral := f.Rand(32)
-	share, _ := curve25519.X25519(ephemeral, curve25519.Basepoint)
+	share := x25519(f.Rand(32), curve25519.Basepoint)
+	f.X25519Stanza(share, identity)
+}
+
+func (f *TestFile) X25519Stanza(share, identity []byte) {
+	recipient := x25519(identity, curve25519.Basepoint)
 	f.ArgsLine("X25519", b64(share))
-	secret, _ := curve25519.X25519(ephemeral, recipient)
+	// This would be ordinarily done as [ephemeral]recipient rather than
+	// [identity]share, but for some tests we don't have the dlog of share.
+	secret := x25519(identity, share)
 	key := make([]byte, 32)
 	hkdf.New(sha256.New, secret, append(share, recipient...),
 		[]byte("age-encryption.org/v1/X25519")).Read(key)
@@ -150,8 +161,11 @@ func (f *TestFile) ScryptRecordPassphrase(passphrase string) {
 func (f *TestFile) ScryptNoRecordPassphrase(passphrase string, workFactor int) {
 	salt := f.Rand(16)
 	f.ArgsLine("scrypt", b64(salt), strconv.Itoa(workFactor))
-	key, _ := scrypt.Key([]byte(passphrase), append([]byte("age-encryption.org/v1/scrypt"), salt...),
+	key, err := scrypt.Key([]byte(passphrase), append([]byte("age-encryption.org/v1/scrypt"), salt...),
 		1<<workFactor, 8, 1, 32)
+	if err != nil {
+		panic(err)
+	}
 	f.AEADBody(key, f.fileKey)
 }
 
@@ -201,6 +215,10 @@ func (f *TestFile) ExpectPayloadFailure() {
 	f.expect = "payload failure"
 }
 
+func (f *TestFile) ExpectHMACFailure() {
+	f.expect = "HMAC failure"
+}
+
 func (f *TestFile) Comment(c string) {
 	f.comment = c
 }

testkit_test.go

@@ -75,10 +75,9 @@ func TestVectors(t *testing.T) {
 
 func testVector(t *testing.T, test []byte) {
 	var (
-		expectHeaderFailure  bool
-		expectPayloadFailure bool
-		payloadHash          *[32]byte
-		identities           []age.Identity
+		expect      string
+		payloadHash *[32]byte
+		identities  []age.Identity
 	)
 
 	for {
@@ -95,13 +94,13 @@ func testVector(t *testing.T, test []byte) {
 		case "expect":
 			switch value {
 			case "success":
+			case "HMAC failure":
 			case "header failure":
-				expectHeaderFailure = true
 			case "payload failure":
-				expectPayloadFailure = true
 			default:
 				t.Fatal("invalid test file: unknown expect value:", value)
 			}
+			expect = value
 		case "payload":
 			h, err := hex.DecodeString(value)
 			if err != nil {
@@ -130,24 +129,30 @@ func testVector(t *testing.T, test []byte) {
 	}
 
 	r, err := age.Decrypt(bytes.NewReader(test), identities...)
-	if err != nil {
-		if expectHeaderFailure {
+	if err != nil && strings.HasSuffix(err.Error(), "bad header MAC") {
+		if expect == "HMAC failure" {
 			t.Log(err)
 			return
 		}
-		t.Fatal("unexpected header error:", err)
-	} else if expectHeaderFailure {
-		t.Fatal("expected header error")
+		t.Fatalf("expected %s, got HMAC error", expect)
+	} else if err != nil {
+		if expect == "header failure" {
+			t.Log(err)
+			return
+		}
+		t.Fatalf("expected %s, got: %v", expect, err)
+	} else if expect != "success" && expect != "payload failure" {
+		t.Fatalf("expected %s, got success", expect)
 	}
 	out, err := io.ReadAll(r)
 	if err != nil {
-		if expectPayloadFailure {
+		if expect == "payload failure" {
 			t.Log(err)
 			return
 		}
-		t.Fatal("unexpected payload error:", err)
-	} else if expectPayloadFailure {
-		t.Fatal("expected payload error")
+		t.Fatalf("expected %s, got: %v", expect, err)
+	} else if expect != "success" {
+		t.Fatalf("expected %s, got success", expect)
 	}
 	if sha256.Sum256(out) != *payloadHash {
 		t.Error("payload hash mismatch")

tests/hmac_bad.go

@@ -16,6 +16,6 @@ func main() {
 	f.HMAC()
 	f.FileKey(testkit.TestFileKey)
 	f.Payload("age")
-	f.ExpectHeaderFailure()
+	f.ExpectHMACFailure()
 	f.Generate()
 }

tests/stanza_bad_start.go

@@ -0,0 +1,21 @@
+// Copyright 2022 The age Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build ignore
+
+package main
+
+import "filippo.io/age/internal/testkit"
+
+func main() {
+	f := testkit.NewTestFile()
+	f.VersionLine("v1")
+	f.X25519(testkit.TestX25519Recipient)
+	f.TextLine("-- stanza")
+	f.Body([]byte(""))
+	f.HMAC()
+	f.Payload("age")
+	f.ExpectHeaderFailure()
+	f.Generate()
+}

tests/stanza_base64_padding.go

@@ -0,0 +1,26 @@
+// Copyright 2022 The age Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build ignore
+
+package main
+
+import (
+	"bytes"
+
+	"filippo.io/age/internal/testkit"
+)
+
+func main() {
+	f := testkit.NewTestFile()
+	f.VersionLine("v1")
+	f.X25519(testkit.TestX25519Recipient)
+	f.ArgsLine("stanza")
+	f.Body(bytes.Repeat([]byte("A"), 50))
+	f.TextLine(f.UnreadLine() + "=")
+	f.HMAC()
+	f.Payload("age")
+	f.ExpectHeaderFailure()
+	f.Generate()
+}

tests/stanza_empty_argument.go

@@ -0,0 +1,21 @@
+// Copyright 2022 The age Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build ignore
+
+package main
+
+import "filippo.io/age/internal/testkit"
+
+func main() {
+	f := testkit.NewTestFile()
+	f.VersionLine("v1")
+	f.X25519(testkit.TestX25519Recipient)
+	f.ArgsLine("stanza", "", "argument")
+	f.Body([]byte(""))
+	f.HMAC()
+	f.Payload("age")
+	f.ExpectHeaderFailure()
+	f.Generate()
+}

tests/stanza_empty_last_line.go

@@ -17,7 +17,7 @@ func main() {
 	f.VersionLine("v1")
 	f.X25519(testkit.TestX25519Recipient)
 	f.ArgsLine("stanza")
-	f.Body(bytes.Repeat([]byte("A"), 48))
+	f.Body(bytes.Repeat([]byte("A"), 48*2))
 	f.HMAC()
 	f.Payload("age")
 	f.Generate()

tests/stanza_invalid_character.go

@@ -0,0 +1,21 @@
+// Copyright 2022 The age Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build ignore
+
+package main
+
+import "filippo.io/age/internal/testkit"
+
+func main() {
+	f := testkit.NewTestFile()
+	f.VersionLine("v1")
+	f.X25519(testkit.TestX25519Recipient)
+	f.ArgsLine("stanza", "è")
+	f.Body([]byte(""))
+	f.HMAC()
+	f.Payload("age")
+	f.ExpectHeaderFailure()
+	f.Generate()
+}

tests/stanza_long_line.go

@@ -0,0 +1,27 @@
+// Copyright 2022 The age Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build ignore
+
+package main
+
+import (
+	"strings"
+
+	"filippo.io/age/internal/testkit"
+)
+
+func main() {
+	f := testkit.NewTestFile()
+	f.VersionLine("v1")
+	f.X25519(testkit.TestX25519Recipient)
+	f.ArgsLine("stanza")
+	f.TextLine(strings.Repeat("A", 68))
+	f.TextLine("")
+	f.HMAC()
+	f.Payload("age")
+	f.ExpectHeaderFailure()
+	f.Comment("a body line is longer than 64 columns")
+	f.Generate()
+}

tests/stanza_no_arguments.go

@@ -0,0 +1,21 @@
+// Copyright 2022 The age Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build ignore
+
+package main
+
+import "filippo.io/age/internal/testkit"
+
+func main() {
+	f := testkit.NewTestFile()
+	f.VersionLine("v1")
+	f.X25519(testkit.TestX25519Recipient)
+	f.ArgsLine()
+	f.Body([]byte(""))
+	f.HMAC()
+	f.Payload("age")
+	f.ExpectHeaderFailure()
+	f.Generate()
+}

tests/stanza_not_canonical.go

@@ -0,0 +1,26 @@
+// Copyright 2022 The age Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build ignore
+
+package main
+
+import (
+	"bytes"
+
+	"filippo.io/age/internal/testkit"
+)
+
+func main() {
+	f := testkit.NewTestFile()
+	f.VersionLine("v1")
+	f.X25519(testkit.TestX25519Recipient)
+	f.ArgsLine("stanza")
+	f.Body(bytes.Repeat([]byte("A"), 50))
+	f.TextLine(testkit.NotCanonicalBase64(f.UnreadLine()))
+	f.HMAC()
+	f.Payload("age")
+	f.ExpectHeaderFailure()
+	f.Generate()
+}

tests/stanza_spurious_cr.go

@@ -0,0 +1,25 @@
+// Copyright 2022 The age Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build ignore
+
+package main
+
+import (
+	"strings"
+
+	"filippo.io/age/internal/testkit"
+)
+
+func main() {
+	f := testkit.NewTestFile()
+	f.VersionLine("v1")
+	f.X25519(testkit.TestX25519Recipient)
+	f.ArgsLine("stanza")
+	f.TextLine(strings.Repeat("A", 32) + "\r" + strings.Repeat("A", 31))
+	f.HMAC()
+	f.Payload("age")
+	f.ExpectHeaderFailure()
+	f.Generate()
+}

tests/version_unsupported.go

@@ -0,0 +1,19 @@
+// Copyright 2022 The age Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build ignore
+
+package main
+
+import "filippo.io/age/internal/testkit"
+
+func main() {
+	f := testkit.NewTestFile()
+	f.VersionLine("v1234")
+	f.X25519(testkit.TestX25519Recipient)
+	f.HMAC()
+	f.Payload("age")
+	f.ExpectHeaderFailure()
+	f.Generate()
+}

tests/x25519_bad_tag.go

@@ -0,0 +1,27 @@
+// Copyright 2022 The age Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build ignore
+
+package main
+
+import (
+	"encoding/base64"
+
+	"filippo.io/age/internal/testkit"
+)
+
+func main() {
+	f := testkit.NewTestFile()
+	f.VersionLine("v1")
+	f.X25519(testkit.TestX25519Recipient)
+	body, _ := base64.RawStdEncoding.DecodeString(f.UnreadLine())
+	body[len(body)-1] ^= 0xff
+	f.TextLine(base64.RawStdEncoding.EncodeToString(body))
+	f.HMAC()
+	f.Payload("age")
+	f.ExpectHeaderFailure()
+	f.Comment("the ChaCha20Poly1305 authentication tag on the body of the X25519 stanza is wrong")
+	f.Generate()
+}

tests/x25519_grease.go

@@ -0,0 +1,22 @@
+// Copyright 2022 The age Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build ignore
+
+package main
+
+import "filippo.io/age/internal/testkit"
+
+func main() {
+	f := testkit.NewTestFile()
+	f.VersionLine("v1")
+	f.ArgsLine("grease")
+	f.Body(nil)
+	f.X25519(testkit.TestX25519Recipient)
+	f.ArgsLine("grease")
+	f.Body(nil)
+	f.HMAC()
+	f.Payload("age")
+	f.Generate()
+}

tests/x25519_identity.go

@@ -6,26 +6,14 @@
 
 package main
 
-import (
-	"crypto/sha256"
-	"encoding/base64"
-
-	"filippo.io/age/internal/testkit"
-	"golang.org/x/crypto/curve25519"
-	"golang.org/x/crypto/hkdf"
-)
+import "filippo.io/age/internal/testkit"
 
 func main() {
 	f := testkit.NewTestFile()
 	f.VersionLine("v1")
 	f.X25519RecordIdentity(testkit.TestX25519Identity)
-	share := make([]byte, curve25519.PointSize)
-	f.ArgsLine("X25519", base64.RawStdEncoding.EncodeToString(share))
-	secret := make([]byte, curve25519.PointSize)
-	key := make([]byte, 32)
-	hkdf.New(sha256.New, secret, append(share, testkit.TestX25519Recipient...),
-		[]byte("age-encryption.org/v1/X25519")).Read(key)
-	f.AEADBody(key, testkit.TestFileKey)
+	share := make([]byte, 32)
+	f.X25519Stanza(share, testkit.TestX25519Identity)
 	f.HMAC()
 	f.Payload("age")
 	f.ExpectHeaderFailure()

tests/x25519_long_share.go

@@ -0,0 +1,30 @@
+// Copyright 2022 The age Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build ignore
+
+package main
+
+import (
+	"encoding/base64"
+
+	"filippo.io/age/internal/testkit"
+	"golang.org/x/crypto/curve25519"
+)
+
+func main() {
+	f := testkit.NewTestFile()
+	f.VersionLine("v1")
+	share, _ := curve25519.X25519(f.Rand(32), curve25519.Basepoint)
+	f.X25519RecordIdentity(testkit.TestX25519Identity)
+	f.X25519Stanza(share, testkit.TestX25519Identity)
+	body, _ := f.UnreadLine(), f.UnreadLine()
+	f.TextLine("-> X25519 " + base64.RawStdEncoding.EncodeToString(append(share, 0x00)))
+	f.TextLine(body)
+	f.HMAC()
+	f.Payload("age")
+	f.ExpectHeaderFailure()
+	f.Comment("a trailing zero is missing from the X25519 share")
+	f.Generate()
+}

tests/x25519_low_order.go

@@ -6,14 +6,7 @@
 
 package main
 
-import (
-	"crypto/sha256"
-	"encoding/base64"
-
-	"filippo.io/age/internal/testkit"
-	"golang.org/x/crypto/curve25519"
-	"golang.org/x/crypto/hkdf"
-)
+import "filippo.io/age/internal/testkit"
 
 func main() {
 	f := testkit.NewTestFile()
@@ -24,12 +17,7 @@ func main() {
 	share := []byte{0x5f, 0x9c, 0x95, 0xbc, 0xa3, 0x50, 0x8c, 0x24, 0xb1, 0xd0,
 		0xb1, 0x55, 0x9c, 0x83, 0xef, 0x5b, 0x04, 0x44, 0x5c, 0xc4, 0x58, 0x1c,
 		0x8e, 0x86, 0xd8, 0x22, 0x4e, 0xdd, 0xd0, 0x9f, 0x11, 0xd7}
-	f.ArgsLine("X25519", base64.RawStdEncoding.EncodeToString(share))
-	secret := make([]byte, curve25519.PointSize)
-	key := make([]byte, 32)
-	hkdf.New(sha256.New, secret, append(share, testkit.TestX25519Recipient...),
-		[]byte("age-encryption.org/v1/X25519")).Read(key)
-	f.AEADBody(key, testkit.TestFileKey)
+	f.X25519Stanza(share, testkit.TestX25519Identity)
 	f.HMAC()
 	f.Payload("age")
 	f.ExpectHeaderFailure()

tests/x25519_lowercase.go

@@ -0,0 +1,27 @@
+// Copyright 2022 The age Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build ignore
+
+package main
+
+import (
+	"strings"
+
+	"filippo.io/age/internal/testkit"
+)
+
+func main() {
+	f := testkit.NewTestFile()
+	f.VersionLine("v1")
+	f.X25519(testkit.TestX25519Recipient)
+	body, args := f.UnreadLine(), f.UnreadLine()
+	f.TextLine(strings.Replace(args, "X25519", "x25519", -1))
+	f.TextLine(body)
+	f.HMAC()
+	f.Payload("age")
+	f.ExpectHeaderFailure()
+	f.Comment("the first argument in the X25519 stanza is lowercase")
+	f.Generate()
+}

tests/x25519_short_share.go

@@ -0,0 +1,30 @@
+// Copyright 2022 The age Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build ignore
+
+package main
+
+import (
+	"encoding/base64"
+	"encoding/hex"
+
+	"filippo.io/age/internal/testkit"
+)
+
+func main() {
+	f := testkit.NewTestFile()
+	f.VersionLine("v1")
+	share, _ := hex.DecodeString("97ba38a135fd5f9137fca3836bfec24340ab03d7ca316b26f482636334a52600")
+	f.X25519RecordIdentity(testkit.TestX25519Identity)
+	f.X25519Stanza(share, testkit.TestX25519Identity)
+	body, _ := f.UnreadLine(), f.UnreadLine()
+	f.TextLine("-> X25519 " + base64.RawStdEncoding.EncodeToString(share[:31]))
+	f.TextLine(body)
+	f.HMAC()
+	f.Payload("age")
+	f.ExpectHeaderFailure()
+	f.Comment("a trailing zero is missing from the X25519 share")
+	f.Generate()
+}