diff --git a/CHANGELOG.md b/CHANGELOG.md index 454da670..f941ab89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Signing a v2 certificate with an empty name, a name longer than 253 bytes, or an empty group now fails. + These certificates were signed successfully but could not be loaded. - `create-tunnel -address ` panicked on a nil RemoteList and, over the ssh debug server, took nebula down with it. A hostinfo returned by StartHandshake had no RemoteList until the handshake loop's first pass attached one, and `create-tunnel -address` set the remote before that. The list is now attached when diff --git a/cert/cert_v2.go b/cert/cert_v2.go index c2b43a69..37393096 100644 --- a/cert/cert_v2.go +++ b/cert/cert_v2.go @@ -389,7 +389,16 @@ func (c *certificateV2) fromTBSCertificate(t *TBSCertificate) error { } func (c *certificateV2) validate() error { - // Empty names are allowed + // Mirror the name and group rules in unmarshalDetails so signing fails instead of producing an unparseable certificate + if c.details.name == "" || len(c.details.name) > MaxNameLength { + return NewErrInvalidCertificateProperties("name must be between 1 and %d bytes", MaxNameLength) + } + + for _, group := range c.details.groups { + if group == "" { + return NewErrInvalidCertificateProperties("groups may not be empty") + } + } if len(c.publicKey) == 0 { return ErrInvalidPublicKey diff --git a/cert/sign_test.go b/cert/sign_test.go index 46d149e3..ed568ee4 100644 --- a/cert/sign_test.go +++ b/cert/sign_test.go @@ -6,6 +6,7 @@ import ( "crypto/elliptic" "crypto/rand" "net/netip" + "strings" "testing" "time" @@ -135,3 +136,47 @@ func TestCertificate_SignP256_AlwaysNormalized(t *testing.T) { assert.True(t, normie) } } + +func TestCertificateV2_SignRejectsUnparseable(t *testing.T) { + _, priv, err := ed25519.GenerateKey(rand.Reader) + require.NoError(t, err) + var invalid *ErrInvalidCertificateProperties + + newTBS := func() TBSCertificate { + return TBSCertificate{ + Version: Version2, + Name: "testing", + Networks: []netip.Prefix{mustParsePrefixUnmapped("10.1.1.1/24")}, + Groups: []string{"test-group1"}, + NotBefore: time.Now().Add(-time.Minute), + NotAfter: time.Now().Add(time.Minute), + PublicKey: []byte("1234567890abcedfghij1234567890ab"), + Curve: Curve_CURVE25519, + IsCA: true, + } + } + + tbs := newTBS() + tbs.Name = strings.Repeat("a", MaxNameLength) + c, err := tbs.Sign(nil, Curve_CURVE25519, priv) + require.NoError(t, err) + b, err := c.Marshal() + require.NoError(t, err) + _, err = unmarshalCertificateV2(b, nil, Curve_CURVE25519) + require.NoError(t, err) + + tbs = newTBS() + tbs.Name = strings.Repeat("a", MaxNameLength+1) + _, err = tbs.Sign(nil, Curve_CURVE25519, priv) + require.ErrorAs(t, err, &invalid) + + tbs = newTBS() + tbs.Name = "" + _, err = tbs.Sign(nil, Curve_CURVE25519, priv) + require.ErrorAs(t, err, &invalid) + + tbs = newTBS() + tbs.Groups = []string{"test-group1", ""} + _, err = tbs.Sign(nil, Curve_CURVE25519, priv) + require.ErrorAs(t, err, &invalid) +}