From 8adba3960b72884faf28d7675a0df2c4744fce59 Mon Sep 17 00:00:00 2001 From: Jack Doan Date: Tue, 5 Nov 2024 11:00:10 -0500 Subject: [PATCH] finish off cert-v2 TODOs --- cert/cert_v2.go | 20 +++++++++++++------- cert/cert_v2_test.go | 41 +++++++++++++++++++++++++++++++++++++++++ cert/errors.go | 5 +++-- cert/sign_test.go | 1 + 4 files changed, 58 insertions(+), 9 deletions(-) diff --git a/cert/cert_v2.go b/cert/cert_v2.go index 618373c..dce9296 100644 --- a/cert/cert_v2.go +++ b/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 diff --git a/cert/cert_v2_test.go b/cert/cert_v2_test.go index 7bdf550..3afbcab 100644 --- a/cert/cert_v2_test.go +++ b/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) +} diff --git a/cert/errors.go b/cert/errors.go index 3fcedbf..ab18cf2 100644 --- a/cert/errors.go +++ 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") ) diff --git a/cert/sign_test.go b/cert/sign_test.go index f87ee61..2b8dbe8 100644 --- a/cert/sign_test.go +++ b/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))