finish off cert-v2 TODOs

cert/cert_v2.go

@@ -137,8 +137,10 @@ func (c *certificateV2) Fingerprint() (string, error) {
 }
 
 func (c *certificateV2) CheckSignature(key []byte) bool {
+	if len(c.rawDetails) == 0 {
+		return false
+	}
 	b := make([]byte, len(c.rawDetails)+1+len(c.publicKey))
-	//TODO: double check this, panic on empty raw details
 	copy(b, c.rawDetails)
 	b[len(c.rawDetails)] = byte(c.curve)
 	copy(b[len(c.rawDetails)+1:], c.publicKey)
@@ -147,7 +149,6 @@ func (c *certificateV2) CheckSignature(key []byte) bool {
 	case Curve_CURVE25519:
 		return ed25519.Verify(key, b, c.signature)
 	case Curve_P256:
-		//TODO: NewPublicKey
 		x, y := elliptic.Unmarshal(elliptic.P256(), key)
 		pubKey := &ecdsa.PublicKey{Curve: elliptic.P256(), X: x, Y: y}
 		hashed := sha256.Sum256(b)
@@ -229,12 +230,14 @@ func (c *certificateV2) String() string {
 }
 
 func (c *certificateV2) MarshalForHandshakes() ([]byte, error) {
+	if c.rawDetails == nil {
+		return nil, ErrEmptyRawDetails
+	}
 	var b cryptobyte.Builder
 	// Outermost certificate
 	b.AddASN1(asn1.SEQUENCE, func(b *cryptobyte.Builder) {
 
 		// Add the cert details which is already marshalled
-		//TODO: panic on nil rawDetails
 		b.AddBytes(c.rawDetails)
 
 		// Skipping the curve and public key since those come across in a different part of the handshake
@@ -249,6 +252,9 @@ func (c *certificateV2) MarshalForHandshakes() ([]byte, error) {
 }
 
 func (c *certificateV2) Marshal() ([]byte, error) {
+	if c.rawDetails == nil {
+		return nil, ErrEmptyRawDetails
+	}
 	var b cryptobyte.Builder
 	// Outermost certificate
 	b.AddASN1(asn1.SEQUENCE, func(b *cryptobyte.Builder) {
@@ -376,13 +382,11 @@ func (c *certificateV2) fromTBSCertificate(t *TBSCertificate) error {
 func (c *certificateV2) marshalForSigning() ([]byte, error) {
 	d, err := c.details.Marshal()
 	if err != nil {
-		//TODO: annotate?
-		return nil, err
+		return nil, fmt.Errorf("marshalling certificate details failed: %w", err)
 	}
 	c.rawDetails = d
 
 	b := make([]byte, len(c.rawDetails)+1+len(c.publicKey))
-	//TODO: double check this
 	copy(b, c.rawDetails)
 	b[len(c.rawDetails)] = byte(c.curve)
 	copy(b[len(c.rawDetails)+1:], c.publicKey)
@@ -516,7 +520,9 @@ func unmarshalCertificateV2(b []byte, publicKey []byte, curve Curve) (*certifica
 		return nil, ErrBadFormat
 	}
 
-	//TODO: Assert public key length
+	if len(rawPublicKey) == 0 {
+		return nil, ErrBadFormat
+	}
 
 	// Grab the signature
 	var rawSignature cryptobyte.String

cert/cert_v2_test.go

@@ -3,6 +3,7 @@ package cert
 import (
 	"crypto/ed25519"
 	"crypto/rand"
+	"encoding/hex"
 	"net/netip"
 	"slices"
 	"testing"
@@ -224,3 +225,43 @@ func TestUnmarshalCertificateV2(t *testing.T) {
 	_, err := unmarshalCertificateV2(data, nil, Curve_CURVE25519)
 	assert.EqualError(t, err, "bad wire format")
 }
+
+func TestCertificateV2_marshalForSigningStability(t *testing.T) {
+	before := time.Date(1996, time.May, 5, 0, 0, 0, 0, time.UTC)
+	after := before.Add(time.Second * 60).Round(time.Second)
+	pubKey := []byte("1234567890abcedfghij1234567890ab")
+
+	nc := certificateV2{
+		details: detailsV2{
+			name: "testing",
+			networks: []netip.Prefix{
+				mustParsePrefixUnmapped("10.1.1.2/16"),
+				mustParsePrefixUnmapped("10.1.1.1/24"),
+			},
+			unsafeNetworks: []netip.Prefix{
+				mustParsePrefixUnmapped("9.1.1.3/16"),
+				mustParsePrefixUnmapped("9.1.1.2/24"),
+			},
+			groups:    []string{"test-group1", "test-group2", "test-group3"},
+			notBefore: before,
+			notAfter:  after,
+			isCA:      false,
+			issuer:    "1234567890abcdef1234567890abcdef",
+		},
+		signature: []byte("1234567890abcdef1234567890abcdef"),
+		publicKey: pubKey,
+	}
+
+	const expectedRawDetailsStr = "a070800774657374696e67a10e04050a0101021004050a01010118a20e0405090101031004050901010218a3270c0b746573742d67726f7570310c0b746573742d67726f7570320c0b746573742d67726f7570338504318bef808604318befbc87101234567890abcdef1234567890abcdef"
+	expectedRawDetails, err := hex.DecodeString(expectedRawDetailsStr)
+	require.NoError(t, err)
+
+	db, err := nc.details.Marshal()
+	require.NoError(t, err)
+	assert.Equal(t, expectedRawDetails, db)
+
+	expectedForSigning, err := hex.DecodeString(expectedRawDetailsStr + "00313233343536373839306162636564666768696a313233343536373839306162")
+	b, err := nc.marshalForSigning()
+	require.NoError(t, err)
+	assert.Equal(t, expectedForSigning, b)
+}

cert/errors.go

@@ -30,6 +30,7 @@ var (
 	ErrNoPeerStaticKey = errors.New("no peer static key was present")
 	ErrNoPayload       = errors.New("provided payload was empty")
 
-	ErrMissingDetails = errors.New("certificate did not contain details")
-	ErrEmptySignature = errors.New("empty signature")
+	ErrMissingDetails  = errors.New("certificate did not contain details")
+	ErrEmptySignature  = errors.New("empty signature")
+	ErrEmptyRawDetails = errors.New("empty rawDetails not allowed")
 )

cert/sign_test.go

@@ -73,6 +73,7 @@ func TestCertificateV1_SignP256(t *testing.T) {
 	}
 
 	priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
+	assert.NoError(t, err)
 	pub := elliptic.Marshal(elliptic.P256(), priv.PublicKey.X, priv.PublicKey.Y)
 	rawPriv := priv.D.FillBytes(make([]byte, 32))