Skip to content

Commit f573e8a

Browse files
authored
Merge commit from fork
Newly signed P256 based certificates will have their signature clamped to the low-s form. Update CHANGELOG.md
1 parent 42bee7c commit f573e8a

File tree

10 files changed

+317
-5
lines changed

10 files changed

+317
-5
lines changed

CHANGELOG.md

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
## [1.10.3] - 2026-02-06
11+
12+
### Security
13+
14+
- Fix an issue where blocklist bypass is possible when using curve P256 since the signature can have 2 valid representations.
15+
Both fingerprint representations will be tested against the blocklist.
16+
Any newly issued P256 based certificates will have their signature clamped to the low-s form.
17+
Nebula will assert the low-s signature form when validating certificates in a future version. [GHSA-69x3-g4r3-p962](https://github.com/slackhq/nebula/security/advisories/GHSA-69x3-g4r3-p962)
18+
19+
### Changed
20+
21+
- Improve error reporting if nebula fails to start due to a tun device naming issue. (#1588)
22+
1023
## [1.10.2] - 2026-01-21
1124

1225
### Fixed
@@ -775,7 +788,8 @@ created.)
775788

776789
- Initial public release.
777790

778-
[Unreleased]: https://github.com/slackhq/nebula/compare/v1.10.2...HEAD
791+
[Unreleased]: https://github.com/slackhq/nebula/compare/v1.10.3...HEAD
792+
[1.10.3]: https://github.com/slackhq/nebula/releases/tag/v1.10.3
779793
[1.10.2]: https://github.com/slackhq/nebula/releases/tag/v1.10.2
780794
[1.10.1]: https://github.com/slackhq/nebula/releases/tag/v1.10.1
781795
[1.10.0]: https://github.com/slackhq/nebula/releases/tag/v1.10.0

cert/ca_pool.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,23 @@ func (ncp *CAPool) VerifyCertificate(now time.Time, c Certificate) (*CachedCerti
141141
return nil, err
142142
}
143143

144+
// Pre nebula v1.10.3 could generate signatures in either high or low s form and validation
145+
// of signatures allowed for either. Nebula v1.10.3 and beyond clamps signature generation to low-s form
146+
// but validation still allows for either. Since a change in the signature bytes affects the fingerprint, we
147+
// need to test both forms until such a time comes that we enforce low-s form on signature validation.
148+
fp2, err := CalculateAlternateFingerprint(c)
149+
if err != nil {
150+
return nil, fmt.Errorf("could not calculate alternate fingerprint to verify: %w", err)
151+
}
152+
if fp2 != "" && ncp.IsBlocklisted(fp2) {
153+
return nil, ErrBlockListed
154+
}
155+
144156
cc := CachedCertificate{
145157
Certificate: c,
146158
InvertedGroups: make(map[string]struct{}),
147159
Fingerprint: fp,
160+
fingerprint2: fp2,
148161
signerFingerprint: signer.Fingerprint,
149162
}
150163

@@ -158,6 +171,11 @@ func (ncp *CAPool) VerifyCertificate(now time.Time, c Certificate) (*CachedCerti
158171
// VerifyCachedCertificate is the same as VerifyCertificate other than it operates on a pre-verified structure and
159172
// is a cheaper operation to perform as a result.
160173
func (ncp *CAPool) VerifyCachedCertificate(now time.Time, c *CachedCertificate) error {
174+
// Check any available alternate fingerprint forms for this certificate, re P256 high-s/low-s
175+
if c.fingerprint2 != "" && ncp.IsBlocklisted(c.fingerprint2) {
176+
return ErrBlockListed
177+
}
178+
161179
_, err := ncp.verify(c.Certificate, now, c.Fingerprint, c.signerFingerprint)
162180
return err
163181
}

cert/ca_pool_test.go

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66
"time"
77

8+
"github.com/slackhq/nebula/cert/p256"
89
"github.com/stretchr/testify/assert"
910
"github.com/stretchr/testify/require"
1011
)
@@ -170,6 +171,15 @@ func TestCertificateV1_VerifyP256(t *testing.T) {
170171
_, err = caPool.VerifyCertificate(time.Now(), c)
171172
require.EqualError(t, err, "certificate is in the block list")
172173

174+
// Create a copy of the cert and swap to the alternate form for the signature
175+
nc := c.Copy()
176+
b, err := p256.Swap(c.Signature())
177+
require.NoError(t, err)
178+
require.NoError(t, nc.(*certificateV1).setSignature(b))
179+
180+
_, err = caPool.VerifyCertificate(time.Now(), nc)
181+
require.EqualError(t, err, "certificate is in the block list")
182+
173183
caPool.ResetCertBlocklist()
174184
_, err = caPool.VerifyCertificate(time.Now(), c)
175185
require.NoError(t, err)
@@ -187,7 +197,7 @@ func TestCertificateV1_VerifyP256(t *testing.T) {
187197
require.NoError(t, err)
188198

189199
caPool = NewCAPool()
190-
b, err := caPool.AddCAFromPEM(caPem)
200+
b, err = caPool.AddCAFromPEM(caPem)
191201
require.NoError(t, err)
192202
assert.Empty(t, b)
193203

@@ -196,7 +206,17 @@ func TestCertificateV1_VerifyP256(t *testing.T) {
196206
})
197207

198208
c, _, _, _ = NewTestCert(Version1, Curve_P256, ca, caKey, "test", time.Now(), time.Now().Add(5*time.Minute), nil, nil, []string{"test1"})
199-
_, err = caPool.VerifyCertificate(time.Now(), c)
209+
cc, err := caPool.VerifyCertificate(time.Now(), c)
210+
require.NoError(t, err)
211+
212+
// Reset the blocklist and block the alternate form fingerprint
213+
caPool.ResetCertBlocklist()
214+
caPool.BlocklistFingerprint(cc.fingerprint2)
215+
err = caPool.VerifyCachedCertificate(time.Now(), cc)
216+
require.EqualError(t, err, "certificate is in the block list")
217+
218+
caPool.ResetCertBlocklist()
219+
err = caPool.VerifyCachedCertificate(time.Now(), cc)
200220
require.NoError(t, err)
201221
}
202222

@@ -394,6 +414,15 @@ func TestCertificateV2_VerifyP256(t *testing.T) {
394414
_, err = caPool.VerifyCertificate(time.Now(), c)
395415
require.EqualError(t, err, "certificate is in the block list")
396416

417+
// Create a copy of the cert and swap to the alternate form for the signature
418+
nc := c.Copy()
419+
b, err := p256.Swap(c.Signature())
420+
require.NoError(t, err)
421+
require.NoError(t, nc.(*certificateV2).setSignature(b))
422+
423+
_, err = caPool.VerifyCertificate(time.Now(), nc)
424+
require.EqualError(t, err, "certificate is in the block list")
425+
397426
caPool.ResetCertBlocklist()
398427
_, err = caPool.VerifyCertificate(time.Now(), c)
399428
require.NoError(t, err)
@@ -411,7 +440,7 @@ func TestCertificateV2_VerifyP256(t *testing.T) {
411440
require.NoError(t, err)
412441

413442
caPool = NewCAPool()
414-
b, err := caPool.AddCAFromPEM(caPem)
443+
b, err = caPool.AddCAFromPEM(caPem)
415444
require.NoError(t, err)
416445
assert.Empty(t, b)
417446

@@ -420,7 +449,17 @@ func TestCertificateV2_VerifyP256(t *testing.T) {
420449
})
421450

422451
c, _, _, _ = NewTestCert(Version2, Curve_P256, ca, caKey, "test", time.Now(), time.Now().Add(5*time.Minute), nil, nil, []string{"test1"})
423-
_, err = caPool.VerifyCertificate(time.Now(), c)
452+
cc, err := caPool.VerifyCertificate(time.Now(), c)
453+
require.NoError(t, err)
454+
455+
// Reset the blocklist and block the alternate form fingerprint
456+
caPool.ResetCertBlocklist()
457+
caPool.BlocklistFingerprint(cc.fingerprint2)
458+
err = caPool.VerifyCachedCertificate(time.Now(), cc)
459+
require.EqualError(t, err, "certificate is in the block list")
460+
461+
caPool.ResetCertBlocklist()
462+
err = caPool.VerifyCachedCertificate(time.Now(), cc)
424463
require.NoError(t, err)
425464
}
426465

cert/cert.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"fmt"
55
"net/netip"
66
"time"
7+
8+
"github.com/slackhq/nebula/cert/p256"
79
)
810

911
type Version uint8
@@ -110,6 +112,9 @@ type CachedCertificate struct {
110112
InvertedGroups map[string]struct{}
111113
Fingerprint string
112114
signerFingerprint string
115+
116+
// A place to store a 2nd fingerprint if the certificate could have one, such as with P256
117+
fingerprint2 string
113118
}
114119

115120
func (cc *CachedCertificate) String() string {
@@ -152,3 +157,31 @@ func Recombine(v Version, rawCertBytes, publicKey []byte, curve Curve) (Certific
152157

153158
return c, nil
154159
}
160+
161+
// CalculateAlternateFingerprint calculates a 2nd fingerprint representation for P256 certificates
162+
// CAPool blocklist testing through `VerifyCertificate` and `VerifyCachedCertificate` automatically performs this step.
163+
func CalculateAlternateFingerprint(c Certificate) (string, error) {
164+
if c.Curve() != Curve_P256 {
165+
return "", nil
166+
}
167+
168+
nc := c.Copy()
169+
b, err := p256.Swap(nc.Signature())
170+
if err != nil {
171+
return "", err
172+
}
173+
174+
switch v := nc.(type) {
175+
case *certificateV1:
176+
err = v.setSignature(b)
177+
case *certificateV2:
178+
err = v.setSignature(b)
179+
default:
180+
return "", ErrUnknownVersion
181+
}
182+
183+
if err != nil {
184+
return "", err
185+
}
186+
return nc.Fingerprint()
187+
}

cert/p256/p256.go

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
package p256
2+
3+
import (
4+
"crypto/elliptic"
5+
"errors"
6+
"math/big"
7+
8+
"filippo.io/bigmod"
9+
10+
"golang.org/x/crypto/cryptobyte"
11+
"golang.org/x/crypto/cryptobyte/asn1"
12+
)
13+
14+
var halfN = new(big.Int).Rsh(elliptic.P256().Params().N, 1)
15+
var nMod *bigmod.Modulus
16+
17+
func init() {
18+
n, err := bigmod.NewModulus(elliptic.P256().Params().N.Bytes())
19+
if err != nil {
20+
panic(err)
21+
}
22+
nMod = n
23+
}
24+
25+
func IsNormalized(sig []byte) (bool, error) {
26+
r, s, err := parseSignature(sig)
27+
if err != nil {
28+
return false, err
29+
}
30+
return checkLowS(r, s), nil
31+
}
32+
33+
func checkLowS(_, s []byte) bool {
34+
bigS := new(big.Int).SetBytes(s)
35+
// Check if S <= (N/2), because we want to include the midpoint in the set of low-s
36+
return bigS.Cmp(halfN) <= 0
37+
}
38+
39+
func swap(r, s []byte) ([]byte, []byte, error) {
40+
var err error
41+
bigS, err := bigmod.NewNat().SetBytes(s, nMod)
42+
if err != nil {
43+
return nil, nil, err
44+
}
45+
sNormalized := nMod.Nat().Sub(bigS, nMod)
46+
47+
return r, sNormalized.Bytes(nMod), nil
48+
}
49+
50+
func Normalize(sig []byte) ([]byte, error) {
51+
r, s, err := parseSignature(sig)
52+
if err != nil {
53+
return nil, err
54+
}
55+
56+
if checkLowS(r, s) {
57+
return sig, nil
58+
}
59+
60+
newR, newS, err := swap(r, s)
61+
if err != nil {
62+
return nil, err
63+
}
64+
65+
return encodeSignature(newR, newS)
66+
}
67+
68+
// Swap will change sig between its current form to the opposite high or low form.
69+
func Swap(sig []byte) ([]byte, error) {
70+
r, s, err := parseSignature(sig)
71+
if err != nil {
72+
return nil, err
73+
}
74+
75+
newR, newS, err := swap(r, s)
76+
if err != nil {
77+
return nil, err
78+
}
79+
80+
return encodeSignature(newR, newS)
81+
}
82+
83+
// parseSignature taken exactly from crypto/ecdsa/ecdsa.go
84+
func parseSignature(sig []byte) (r, s []byte, err error) {
85+
var inner cryptobyte.String
86+
input := cryptobyte.String(sig)
87+
if !input.ReadASN1(&inner, asn1.SEQUENCE) ||
88+
!input.Empty() ||
89+
!inner.ReadASN1Integer(&r) ||
90+
!inner.ReadASN1Integer(&s) ||
91+
!inner.Empty() {
92+
return nil, nil, errors.New("invalid ASN.1")
93+
}
94+
return r, s, nil
95+
}
96+
97+
func encodeSignature(r, s []byte) ([]byte, error) {
98+
var b cryptobyte.Builder
99+
b.AddASN1(asn1.SEQUENCE, func(b *cryptobyte.Builder) {
100+
addASN1IntBytes(b, r)
101+
addASN1IntBytes(b, s)
102+
})
103+
return b.Bytes()
104+
}
105+
106+
// addASN1IntBytes encodes in ASN.1 a positive integer represented as
107+
// a big-endian byte slice with zero or more leading zeroes.
108+
func addASN1IntBytes(b *cryptobyte.Builder, bytes []byte) {
109+
for len(bytes) > 0 && bytes[0] == 0 {
110+
bytes = bytes[1:]
111+
}
112+
if len(bytes) == 0 {
113+
b.SetError(errors.New("invalid integer"))
114+
return
115+
}
116+
b.AddASN1(asn1.INTEGER, func(c *cryptobyte.Builder) {
117+
if bytes[0]&0x80 != 0 {
118+
c.AddUint8(0)
119+
}
120+
c.AddBytes(bytes)
121+
})
122+
}

cert/p256/p256_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package p256
2+
3+
import (
4+
"crypto/ecdsa"
5+
"crypto/elliptic"
6+
"crypto/rand"
7+
"testing"
8+
9+
"github.com/stretchr/testify/require"
10+
)
11+
12+
func TestFlipping(t *testing.T) {
13+
priv, err1 := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
14+
require.NoError(t, err1)
15+
16+
out, err := ecdsa.SignASN1(rand.Reader, priv, []byte("big chungus"))
17+
require.NoError(t, err)
18+
19+
r, s, err := parseSignature(out)
20+
require.NoError(t, err)
21+
22+
r, s1, err := swap(r, s)
23+
require.NoError(t, err)
24+
r, s2, err := swap(r, s1)
25+
require.NoError(t, err)
26+
require.Equal(t, s, s2)
27+
require.NotEqual(t, s, s1)
28+
}

cert/sign.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ import (
99
"fmt"
1010
"net/netip"
1111
"time"
12+
13+
"github.com/slackhq/nebula/cert/p256"
1214
)
1315

1416
// TBSCertificate represents a certificate intended to be signed.
@@ -126,6 +128,13 @@ func (t *TBSCertificate) SignWith(signer Certificate, curve Curve, sp SignerLamb
126128
return nil, err
127129
}
128130

131+
if curve == Curve_P256 {
132+
sig, err = p256.Normalize(sig)
133+
if err != nil {
134+
return nil, err
135+
}
136+
}
137+
129138
err = c.setSignature(sig)
130139
if err != nil {
131140
return nil, err

0 commit comments

Comments
 (0)