Reject v2 cert names and groups at signing the parser rejects (#1967)

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 <ip:port>` 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

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

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)
+}