diff --git a/e2e/pq_handshakes_test.go b/e2e/pq_handshakes_test.go index f3e72a28..79f0871a 100644 --- a/e2e/pq_handshakes_test.go +++ b/e2e/pq_handshakes_test.go @@ -10,7 +10,6 @@ import ( "github.com/slackhq/nebula" "github.com/slackhq/nebula/cert" - "github.com/slackhq/nebula/cert_test" "github.com/slackhq/nebula/e2e/router" "github.com/slackhq/nebula/handshake/fragment" "github.com/slackhq/nebula/header" @@ -79,7 +78,7 @@ func (ht *handshakeTally) observe(p *udp.Packet) { } ht.Lock() defer ht.Unlock() - if h.Subtype == header.HandshakeIXPSK2MLKEM1024 { + if h.Subtype == header.HandshakeIXHybridMLKEM1024 { ht.pq++ ht.maxPQLen = max(ht.maxPQLen, len(p.Data)) } else { @@ -102,11 +101,14 @@ func isTunnelData(p *udp.Packet, receiver, to *nebula.Control) bool { return h.Type == header.Message && h.Subtype == header.MessageNone } +// newPQPair builds two hosts with v3 certificates that also trust the CA's v2 +// form, so a classical handshake can complete when the hybrid one does not. func newPQPair(t *testing.T, mine, theirs m) (myControl, theirControl *nebula.Control, myVpn, theirVpn []netip.Prefix, myUdp, theirUdp netip.AddrPort, r *router.R) { t.Helper() - ca, _, caKey, _ := cert_test.NewTestCaCert(cert.Version2, cert.Curve_CURVE25519, time.Now(), time.Now().Add(10*time.Minute), nil, nil, []string{}) - myControl, myVpn, myUdp, _ = newSimpleServer(cert.Version2, ca, caKey, "me", "10.128.0.1/24", mine) - theirControl, theirVpn, theirUdp, _ = newSimpleServer(cert.Version2, ca, caKey, "them", "10.128.0.2/24", theirs) + ca := newHybridCA(t) + ca2, _ := ca.v2() + myControl, myVpn, myUdp, _ = newSimpleServer(cert.Version3, ca.ca, ca.key, "me", "10.128.0.1/24", withCA(mine, ca.ca, ca2)) + theirControl, theirVpn, theirUdp, _ = newSimpleServer(cert.Version3, ca.ca, ca.key, "them", "10.128.0.2/24", withCA(theirs, ca.ca, ca2)) myControl.InjectLightHouseAddr(theirVpn[0].Addr(), theirUdp) r = router.NewR(t, myControl, theirControl) myControl.Start() @@ -114,36 +116,6 @@ func newPQPair(t *testing.T, mine, theirs m) (myControl, theirControl *nebula.Co return } -func TestPQHandshake(t *testing.T) { - t.Parallel() - // They only accept, which is all a PQ initiator needs. - myControl, theirControl, myVpn, theirVpn, myUdp, theirUdp, r := newPQPair(t, pqConfig(true, true, false), pqConfig(false, true, false)) - defer r.RenderFlow() - - var tally handshakeTally - myControl.InjectTunPacket(BuildTunUDPPacket(theirVpn[0].Addr(), 80, myVpn[0].Addr(), 80, []byte("Hi from me"))) - require.True(t, r.RouteForAllExitFuncOrTimeout(5*time.Second, func(p *udp.Packet, receiver *nebula.Control) router.ExitType { - tally.observe(p) - if isTunnelData(p, receiver, theirControl) { - return router.RouteAndExit - } - return router.KeepRouting - }), "tunnel never came up") - - assertUdpPacket(t, []byte("Hi from me"), theirControl.GetFromTun(true), myVpn[0].Addr(), theirVpn[0].Addr(), 80, 80) - assert.Zero(t, tally.classical, "no classical handshake should be needed") - assert.GreaterOrEqual(t, tally.pq, 4, "both messages travel as several fragments") - assert.LessOrEqual(t, tally.maxPQLen, fragment.MaxPacketLen) - - assertHostInfoPair(t, myUdp, theirUdp, myVpn, theirVpn, myControl, theirControl) - assertTunnel(t, myVpn[0].Addr(), theirVpn[0].Addr(), myControl, theirControl, r) - assert.Empty(t, myControl.ListHostmapIndexes(true), "no pending index may outlive the handshake") - assert.Empty(t, theirControl.ListHostmapIndexes(true)) - - myControl.Stop() - theirControl.Stop() -} - // TestPQHandshakeLoss drops data fragments of both messages; the repair // fragments must carry the handshake without a retry. func TestPQHandshakeLoss(t *testing.T) { @@ -223,12 +195,12 @@ func TestPQResendFreshSymbols(t *testing.T) { case h != nil && fh == nil: // Keep the classical fallback out of the way. return router.Drop - case fh != nil && h.MessageCounter == 2 && fh.Index < 4: + case fh != nil && h.MessageCounter == 2 && fh.Index < 12: firstMsg2++ return router.Drop case fh != nil && h.MessageCounter == 2: freshMsg2++ - case fh != nil && h.MessageCounter == 1 && fh.Index >= 4: + case fh != nil && h.MessageCounter == 1 && fh.Index >= 10: freshMsg1++ } if isTunnelData(p, receiver, theirControl) { @@ -238,7 +210,7 @@ func TestPQResendFreshSymbols(t *testing.T) { }), "tunnel never came up") assertUdpPacket(t, []byte("Hi from me"), theirControl.GetFromTun(true), myVpn[0].Addr(), theirVpn[0].Addr(), 80, 80) - assert.Equal(t, 4, firstMsg2, "the first msg2 is 2 data and 2 repair fragments") + assert.Equal(t, 12, firstMsg2, "the first msg2 is 9 data and 3 repair fragments") assert.Positive(t, freshMsg1, "the retry offers fresh msg1 symbols") assert.Positive(t, freshMsg2, "the resend offers fresh msg2 symbols") assertTunnel(t, myVpn[0].Addr(), theirVpn[0].Addr(), myControl, theirControl, r) @@ -268,7 +240,7 @@ func TestPQSticky(t *testing.T) { myControl.ReHandshake(theirVpn[0].Addr()) r.RouteForAllExitFuncOrTimeout(time.Second, func(p *udp.Packet, receiver *nebula.Control) router.ExitType { tally.observe(p) - if h, _ := handshakeOf(p); h != nil && h.Subtype == header.HandshakeIXPSK2MLKEM1024 { + if h, _ := handshakeOf(p); h != nil && h.Subtype == header.HandshakeIXHybridMLKEM1024 { return router.Drop } return router.KeepRouting @@ -280,58 +252,6 @@ func TestPQSticky(t *testing.T) { theirControl.Stop() } -func TestPQRelays(t *testing.T) { - t.Parallel() - ca, _, caKey, _ := cert_test.NewTestCaCert(cert.Version1, cert.Curve_CURVE25519, time.Now(), time.Now().Add(10*time.Minute), nil, nil, []string{}) - useRelays := pqConfig(true, true, false) - useRelays["relay"] = m{"use_relays": true} - amRelay := pqConfig(true, true, false) - amRelay["relay"] = m{"am_relay": true} - myControl, myVpnIpNet, _, _ := newSimpleServer(cert.Version1, ca, caKey, "me ", "10.128.0.1/24", useRelays) - relayControl, relayVpnIpNet, relayUdpAddr, _ := newSimpleServer(cert.Version1, ca, caKey, "relay ", "10.128.0.128/24", amRelay) - theirControl, theirVpnIpNet, theirUdpAddr, _ := newSimpleServer(cert.Version1, ca, caKey, "them ", "10.128.0.2/24", useRelays) - - myControl.InjectLightHouseAddr(relayVpnIpNet[0].Addr(), relayUdpAddr) - myControl.InjectRelays(theirVpnIpNet[0].Addr(), []netip.Addr{relayVpnIpNet[0].Addr()}) - relayControl.InjectLightHouseAddr(theirVpnIpNet[0].Addr(), theirUdpAddr) - - r := router.NewR(t, myControl, relayControl, theirControl) - defer r.RenderFlow() - - myControl.Start() - relayControl.Start() - theirControl.Start() - - var tally handshakeTally - var relayedPQ, relayedClassical int - myControl.InjectTunPacket(BuildTunUDPPacket(theirVpnIpNet[0].Addr(), 80, myVpnIpNet[0].Addr(), 80, []byte("Hi from me"))) - require.True(t, r.RouteForAllExitFuncOrTimeout(10*time.Second, func(p *udp.Packet, receiver *nebula.Control) router.ExitType { - tally.observe(p) - outer := &header.H{} - if outer.Parse(p.Data) == nil && outer.Type == header.Message && outer.Subtype == header.MessageRelay { - if h, fh := handshakeOf(p); h != nil && fh != nil { - relayedPQ++ - assert.LessOrEqual(t, len(p.Data), fragment.MaxPacketLen+32, "a relayed fragment fits the IPv6 minimum MTU") - } else if h != nil { - relayedClassical++ - } - } - if isTunnelData(p, receiver, theirControl) { - return router.RouteAndExit - } - return router.KeepRouting - }), "tunnel never came up") - - assertUdpPacket(t, []byte("Hi from me"), theirControl.GetFromTun(true), myVpnIpNet[0].Addr(), theirVpnIpNet[0].Addr(), 80, 80) - assert.Positive(t, relayedPQ, "the end to end handshake is PQ through the relay") - assert.Zero(t, relayedClassical, "the relay coming up carries the PQ attempt, not the classical retry") - r.RenderHostmaps("Final hostmaps", myControl, relayControl, theirControl) - - myControl.Stop() - relayControl.Stop() - theirControl.Stop() -} - // TestPQTimeoutReleasesIndexes lets a handshake with both a PQ and a classical // attempt pending time out, and checks neither index is left behind. func TestPQTimeoutReleasesIndexes(t *testing.T) { @@ -370,7 +290,7 @@ func TestPQCookieUnderLoad(t *testing.T) { t.Log("Flood them with the first fragment of msg1s nobody will finish") for i := range 512 { from := netip.AddrPortFrom(netip.AddrFrom4([4]byte{198, 18, byte(i >> 8), byte(i)}), 4242) - s, err := fragment.NewSender(make([]byte, 2000), header.HandshakeIXPSK2MLKEM1024, 0, 1, uint32(i+1)) + s, err := fragment.NewSender(make([]byte, 2000), header.HandshakeIXHybridMLKEM1024, 0, 1, uint32(i+1)) require.NoError(t, err) pkts, _, err := s.Attempt(0) require.NoError(t, err) @@ -382,7 +302,7 @@ func TestPQCookieUnderLoad(t *testing.T) { myControl.InjectTunPacket(BuildTunUDPPacket(theirVpn[0].Addr(), 80, myVpn[0].Addr(), 80, []byte("Hi from me"))) require.True(t, r.RouteForAllExitFuncOrTimeout(5*time.Second, func(p *udp.Packet, receiver *nebula.Control) router.ExitType { tally.observe(p) - if h, fh := handshakeOf(p); h != nil && h.Subtype == header.HandshakeIXPSK2MLKEM1024 { + if h, fh := handshakeOf(p); h != nil && h.Subtype == header.HandshakeIXHybridMLKEM1024 { switch { case h.MessageCounter == 0: cookieReplies++ diff --git a/e2e/pq_hybrid_test.go b/e2e/pq_hybrid_test.go index 94c0f67a..f3a1949e 100644 --- a/e2e/pq_hybrid_test.go +++ b/e2e/pq_hybrid_test.go @@ -23,9 +23,9 @@ import ( // hybridTally counts handshake packets by subtype and message. type hybridTally struct { sync.Mutex - hybrid, phase1, classical int - msg3 int - maxLen int + hybrid, classical int + msg3 int + maxLen int } func (ht *hybridTally) observe(p *udp.Packet) { @@ -42,8 +42,6 @@ func (ht *hybridTally) observe(p *udp.Packet) { if h.MessageCounter == 3 { ht.msg3++ } - case header.HandshakeIXPSK2MLKEM1024: - ht.phase1++ default: ht.classical++ } @@ -139,7 +137,7 @@ func TestHybridHandshake(t *testing.T) { }), "tunnel never came up") assertUdpPacket(t, []byte("Hi from me"), theirControl.GetFromTun(true), myVpn[0].Addr(), theirVpn[0].Addr(), 80, 80) - assert.Zero(t, tally.classical+tally.phase1, "a node with a v3 certificate speaks the hybrid handshake") + assert.Zero(t, tally.classical, "a node with a v3 certificate speaks the hybrid handshake") assert.GreaterOrEqual(t, tally.hybrid, 7+9+2, "all three messages travel as several fragments") assert.Positive(t, tally.msg3) assert.LessOrEqual(t, tally.maxLen, fragment.MaxPacketLen) @@ -233,9 +231,9 @@ func TestHybridNeverConfirmed(t *testing.T) { theirControl.Stop() } -// TestHybridToV2Peer has a v3 initiator meet a peer that speaks phase 1 post-quantum -// handshakes but holds only a v2 certificate. Holding v3, the initiator never -// offers phase 1, so the tunnel comes up classical on the v2 form. +// TestHybridToV2Peer has a v3 initiator meet a peer that holds only a v2 +// certificate. Its post-quantum settings do nothing without a v3 certificate, so +// the tunnel comes up classical on the v2 form. func TestHybridToV2Peer(t *testing.T) { t.Parallel() ca := newHybridCA(t) @@ -257,7 +255,6 @@ func TestHybridToV2Peer(t *testing.T) { assertUdpPacket(t, []byte("Hi from me"), theirControl.GetFromTun(true), myVpn[0].Addr(), theirVpn[0].Addr(), 80, 80) assert.Positive(t, tally.hybrid, "the hybrid attempt comes first") - assert.Zero(t, tally.phase1, "a v3 holder never initiates phase 1") assert.Positive(t, tally.classical) assertHostInfoPair(t, myUdp, theirUdp, myVpn, theirVpn, myControl, theirControl) assert.Equal(t, cert.Version2, theirControl.GetHostInfoByVpnAddr(myVpn[0].Addr(), false).Cert.Version()) @@ -313,41 +310,12 @@ func TestHybridRequire(t *testing.T) { return router.KeepRouting }), "tunnel never came up") assertUdpPacket(t, []byte("Hi from me"), theirControl.GetFromTun(true), myVpn[0].Addr(), theirVpn[0].Addr(), 80, 80) - assert.Zero(t, tally.classical+tally.phase1) + assert.Zero(t, tally.classical) assertTunnel(t, myVpn[0].Addr(), theirVpn[0].Addr(), myControl, theirControl, r) myControl.Stop() theirControl.Stop() }) - - t.Run("responder refuses phase 1", func(t *testing.T) { - ca := newHybridCA(t) - ca2, key2 := ca.v2() - myControl, myVpn, _, _ := newSimpleServer(cert.Version2, ca2, key2, "me", "10.128.0.1/24", pqRequire()) - theirControl, theirVpn, theirUdp, _ := newSimpleServer(cert.Version3, ca.ca, ca.key, "them", "10.128.0.2/24", withCA(pqRequire(), ca.ca, ca2)) - myControl.InjectLightHouseAddr(theirVpn[0].Addr(), theirUdp) - r := router.NewR(t, myControl, theirControl) - defer r.RenderFlow() - myControl.Start() - theirControl.Start() - - var tally hybridTally - var answers int - myControl.InjectTunPacket(BuildTunUDPPacket(theirVpn[0].Addr(), 80, myVpn[0].Addr(), 80, []byte("Hi from me"))) - r.RouteForAllExitFuncOrTimeout(time.Second, func(p *udp.Packet, receiver *nebula.Control) router.ExitType { - tally.observe(p) - if h, _ := handshakeOf(p); h != nil && h.MessageCounter == 2 { - answers++ - } - return router.KeepRouting - }) - assert.Positive(t, tally.phase1) - assert.Zero(t, answers, "a phase 1 msg1 gets no answer from a v3 node requiring post-quantum") - assert.Nil(t, theirControl.GetHostInfoByVpnAddr(myVpn[0].Addr(), false)) - - myControl.Stop() - theirControl.Stop() - }) } func TestHybridRelays(t *testing.T) { @@ -432,75 +400,113 @@ func TestHybridBackToBack(t *testing.T) { theirControl.Stop() } -// TestHybridRaceWithClassical has the initiator's classical attempt reach the -// responder, which completes it at once, while the hybrid handshake is still -// waiting for msg3. The initiator gets the hybrid msg2 first and commits to it, -// so the responder must take the hybrid tunnel when msg3 arrives even though the -// classical one is newer. +// TestHybridRaceWithClassical has the initiator's classical attempt cross its +// hybrid one: the hybrid msg2 is held back until the classical msg1 has reached +// the responder, and the classical msg2 is lost, so the initiator commits to the +// hybrid handshake. +// +// Arriving within the crossing window, the classical msg1 goes unanswered and +// the responder ends up with the hybrid tunnel alone. Held past the window, as on +// a very slow path, the responder completes it, and must still take the hybrid +// tunnel when msg3 arrives even though the classical one is newer. func TestHybridRaceWithClassical(t *testing.T) { t.Parallel() - // Both trust the v2 form of the CA too, or the classical attempt could not complete. - ca := newHybridCA(t) - ca2, _ := ca.v2() - myControl, theirControl, myVpn, theirVpn, _, _, r := newHybridPairWithCA(t, ca, - withCA(pqConfig(true, true, false), ca.ca, ca2), - withCA(pqConfig(false, true, false), ca.ca, ca2), - ) - defer r.RenderFlow() + for _, tc := range []struct { + name string + delay time.Duration + answered bool + }{ + {"within the window", 0, false}, + {"past the window", 500 * time.Millisecond, true}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + // Both trust the v2 form of the CA too, or the classical attempt could not complete. + ca := newHybridCA(t) + ca2, _ := ca.v2() + myControl, theirControl, myVpn, theirVpn, _, _, r := newHybridPairWithCA(t, ca, + withCA(pqConfig(true, true, false), ca.ca, ca2), + withCA(pqConfig(false, true, false), ca.ca, ca2), + ) + defer r.RenderFlow() - var heldMsg2 []*udp.Packet - var sawClassical, recvErrors int - myControl.InjectTunPacket(BuildTunUDPPacket(theirVpn[0].Addr(), 80, myVpn[0].Addr(), 80, []byte("Hi from me"))) - require.True(t, r.RouteForAllExitFuncOrTimeout(3*time.Second, func(p *udp.Packet, receiver *nebula.Control) router.ExitType { - h, fh := handshakeOf(p) - switch { - case h != nil && fh != nil && h.MessageCounter == 2: - // Hold the hybrid msg2 back until the classical attempt has gone through. - heldMsg2 = append(heldMsg2, p.Copy()) - return router.Drop - case h != nil && fh == nil && h.MessageCounter == 1: - sawClassical++ - return router.RouteAndExit - } - return router.KeepRouting - }), "no classical attempt") - require.Positive(t, sawClassical) + var heldMsg2 []*udp.Packet + var classicalMsg1 *udp.Packet + holdMsg2 := func(p *udp.Packet) bool { + if h, fh := handshakeOf(p); h != nil && fh != nil && h.MessageCounter == 2 { + heldMsg2 = append(heldMsg2, p.Copy()) + return true + } + return false + } + myControl.InjectTunPacket(BuildTunUDPPacket(theirVpn[0].Addr(), 80, myVpn[0].Addr(), 80, []byte("Hi from me"))) + require.True(t, r.RouteForAllExitFuncOrTimeout(3*time.Second, func(p *udp.Packet, receiver *nebula.Control) router.ExitType { + if holdMsg2(p) { + return router.Drop + } + if h, fh := handshakeOf(p); h != nil && fh == nil && h.MessageCounter == 1 { + classicalMsg1 = p.Copy() + return router.ExitNow + } + return router.KeepRouting + }), "no classical attempt") - // Lose the classical msg2, so the initiator only ever completes the hybrid handshake. - r.RouteForAllExitFuncOrTimeout(50*time.Millisecond, func(p *udp.Packet, receiver *nebula.Control) router.ExitType { - if h, fh := handshakeOf(p); h != nil && fh == nil && h.MessageCounter == 2 { - return router.Drop - } - if h, fh := handshakeOf(p); h != nil && fh != nil && h.MessageCounter == 2 { - heldMsg2 = append(heldMsg2, p.Copy()) - return router.Drop - } - return router.KeepRouting - }) - require.NotNil(t, theirControl.GetHostInfoByVpnAddr(myVpn[0].Addr(), false), "the responder completed the classical handshake") + time.Sleep(tc.delay) + r.InjectUDPPacket(myControl, theirControl, classicalMsg1) - for _, p := range heldMsg2 { - r.InjectUDPPacket(theirControl, myControl, p) + // Lose any classical msg2, so the initiator only ever completes the hybrid + // handshake. With msg2 held the initiator keeps retrying, and a later + // classical retry may land past the window; only the crossing one counts. + var classicalMsg2 int + r.RouteForAllExitFuncOrTimeout(50*time.Millisecond, func(p *udp.Packet, receiver *nebula.Control) router.ExitType { + if h, fh := handshakeOf(p); h != nil && fh == nil && h.MessageCounter == 1 { + return router.Drop + } + if h, fh := handshakeOf(p); h != nil && fh == nil && h.MessageCounter == 2 { + classicalMsg2++ + return router.Drop + } + if holdMsg2(p) { + return router.Drop + } + return router.KeepRouting + }) + if tc.answered { + require.Positive(t, classicalMsg2, "past the window the classical msg1 is answered") + require.NotNil(t, theirControl.GetHostInfoByVpnAddr(myVpn[0].Addr(), false)) + } else { + require.Zero(t, classicalMsg2, "within the window the classical msg1 goes unanswered") + require.Nil(t, theirControl.GetHostInfoByVpnAddr(myVpn[0].Addr(), false)) + } + + var recvErrors int + for _, p := range heldMsg2 { + r.InjectUDPPacket(theirControl, myControl, p) + } + require.True(t, routeUntilData(t, r, theirControl, 3*time.Second, func(p *udp.Packet, _ *nebula.Control) router.ExitType { + if isType(p, header.RecvError) { + recvErrors++ + } + return router.KeepRouting + }), "data never arrived") + assertUdpPacket(t, []byte("Hi from me"), theirControl.GetFromTun(true), myVpn[0].Addr(), theirVpn[0].Addr(), 80, 80) + + assert.Zero(t, recvErrors) + mine := myControl.GetHostInfoByVpnAddr(theirVpn[0].Addr(), false) + theirs := theirControl.GetHostInfoByVpnAddr(myVpn[0].Addr(), false) + require.NotNil(t, mine) + require.NotNil(t, theirs) + assert.Equal(t, cert.Version3, mine.Cert.Version()) + assert.Equal(t, cert.Version3, theirs.Cert.Version(), "the responder's primary tunnel is the hybrid one") + assert.Equal(t, mine.LocalIndex, theirs.RemoteIndex) + if !tc.answered { + assert.Len(t, theirControl.ListHostmapIndexes(false), 1, "and the only one") + } + + myControl.Stop() + theirControl.Stop() + }) } - require.True(t, routeUntilData(t, r, theirControl, 3*time.Second, func(p *udp.Packet, _ *nebula.Control) router.ExitType { - if isType(p, header.RecvError) { - recvErrors++ - } - return router.KeepRouting - }), "data never arrived") - assertUdpPacket(t, []byte("Hi from me"), theirControl.GetFromTun(true), myVpn[0].Addr(), theirVpn[0].Addr(), 80, 80) - - assert.Zero(t, recvErrors) - mine := myControl.GetHostInfoByVpnAddr(theirVpn[0].Addr(), false) - theirs := theirControl.GetHostInfoByVpnAddr(myVpn[0].Addr(), false) - require.NotNil(t, mine) - require.NotNil(t, theirs) - assert.Equal(t, cert.Version3, mine.Cert.Version()) - assert.Equal(t, cert.Version3, theirs.Cert.Version(), "the responder's primary tunnel is the hybrid one") - assert.Equal(t, mine.LocalIndex, theirs.RemoteIndex) - - myControl.Stop() - theirControl.Stop() } // TestHybridWrongResponder gives the initiator a stale address for its peer diff --git a/handshake/credential.go b/handshake/credential.go index ffd285b0..4892dbc7 100644 --- a/handshake/credential.go +++ b/handshake/credential.go @@ -47,19 +47,15 @@ func NewCredential( return hc } -// buildHandshakeState creates a noise.HandshakeState from this credential. -// pskPlacement selects the noise psk modifier; 0 means none. A psk for -// placements 2 and up is supplied later with SetPresharedKey. A hybrid -// pattern also gets its KEM and the static KEM key. +// buildHandshakeState creates a noise.HandshakeState from this credential. A +// hybrid pattern also gets its KEM and the static KEM key. func (hc *Credential) buildHandshakeState(initiator bool, info subtypeInfo) (*noise.HandshakeState, error) { cfg := noise.Config{ - CipherSuite: hc.cipherSuite, - Random: rand.Reader, - Pattern: info.pattern, - Initiator: initiator, - StaticKeypair: noise.DHKey{Private: hc.privateKey, Public: hc.publicKey}, - PresharedKey: []byte{}, - PresharedKeyPlacement: info.pskPlacement, + CipherSuite: hc.cipherSuite, + Random: rand.Reader, + Pattern: info.pattern, + Initiator: initiator, + StaticKeypair: noise.DHKey{Private: hc.privateKey, Public: hc.publicKey}, } if info.kem != nil { cfg.KEMFunc = info.kem diff --git a/handshake/handshake.proto b/handshake/handshake.proto index 591aa797..0fe94271 100644 --- a/handshake/handshake.proto +++ b/handshake/handshake.proto @@ -24,9 +24,9 @@ message NebulaHandshakeDetails { uint64 Cookie = 4 [deprecated = true]; uint64 Time = 5; uint32 CertVersion = 8; - // KEMKey is the initiator's ephemeral ML-KEM-1024 encapsulation key. It is - // only present in msg1 of the ix_psk2_mlkem1024 subtype. - bytes KEMKey = 9; // reserved for WIP multiport reserved 6, 7; + // 9 carried an ephemeral ML-KEM key in ix_psk2_mlkem1024, a handshake that + // was never released + reserved 9; } diff --git a/handshake/hybrid_test.go b/handshake/hybrid_test.go index a398e185..5d857189 100644 --- a/handshake/hybrid_test.go +++ b/handshake/hybrid_test.go @@ -149,8 +149,6 @@ func TestHybridIX_CertVersions(t *testing.T) { require.ErrorIs(t, err, ErrCertVersionMismatch) _, err = NewMachine(cert.Version3, p.initCS.getCredential, v, func() (uint32, error) { return 1, nil }, true, header.HandshakeIXPSK0) require.ErrorIs(t, err, ErrCertVersionMismatch) - _, err = NewMachine(cert.Version3, p.initCS.getCredential, v, func() (uint32, error) { return 1, nil }, true, header.HandshakeIXPSK2MLKEM1024) - require.ErrorIs(t, err, ErrCertVersionMismatch) // The v2 form still does a classical handshake, here with a peer that only has v2. ca2 := cert.V2Form(p.ca) diff --git a/handshake/kem.go b/handshake/kem.go index a301a1f6..ca06caf1 100644 --- a/handshake/kem.go +++ b/handshake/kem.go @@ -1,9 +1,7 @@ package handshake import ( - "crypto/hkdf" "crypto/mlkem" - "crypto/sha256" "errors" "fmt" "io" @@ -11,65 +9,8 @@ import ( "github.com/DefinedNet/noise" ) -// kemPSKSalt separates the psk derivation from any other use of the secret. -const kemPSKSalt = "nebula ix_psk2_mlkem1024 v1" - var ErrInvalidKEMKey = errors.New("peer sent an invalid ML-KEM encapsulation key") -// kemPSK derives the noise psk from an ML-KEM shared secret. The ciphertext is -// bound in because, unlike the encapsulation key, it travels outside the noise -// transcript. -func kemPSK(sharedKey, ciphertext []byte) ([]byte, error) { - secret := make([]byte, 0, len(sharedKey)+len(ciphertext)) - secret = append(secret, sharedKey...) - secret = append(secret, ciphertext...) - return hkdf.Extract(sha256.New, secret, []byte(kemPSKSalt)) -} - -// kemEncapsulate is the responder half: encapsulate to the initiator's key, -// install the psk, and remember the ciphertext to prefix the response with. -func (m *Machine) kemEncapsulate(encapsulationKey []byte) error { - ek, err := mlkem.NewEncapsulationKey1024(encapsulationKey) - if err != nil { - return fmt.Errorf("%w: %w", ErrInvalidKEMKey, err) - } - sharedKey, ct := ek.Encapsulate() - psk, err := kemPSK(sharedKey, ct) - if err != nil { - return err - } - if err := m.hs.SetPresharedKey(psk); err != nil { - return err - } - m.kemCiphertext = ct - return nil -} - -// kemDecapsulate is the initiator half: recover the shared secret from the -// ciphertext prefixing the response and install the psk. It returns the rest of -// the message, which is the noise message. -func (m *Machine) kemDecapsulate(msg []byte) ([]byte, error) { - if len(msg) < mlkem.CiphertextSize1024 { - return nil, ErrPacketTooShort - } - if m.kemKey == nil { - return nil, errors.New("no ML-KEM decapsulation key for this handshake") - } - ct := msg[:mlkem.CiphertextSize1024] - sharedKey, err := m.kemKey.Decapsulate(ct) - if err != nil { - return nil, err - } - psk, err := kemPSK(sharedKey, ct) - if err != nil { - return nil, err - } - if err := m.hs.SetPresharedKey(psk); err != nil { - return nil, err - } - return msg[mlkem.CiphertextSize1024:], nil -} - // KEMMLKEM1024 is ML-KEM-1024 for the noise library's hybrid patterns. Private // keys are the 64 byte seed, the same form v3 private keys store. var KEMMLKEM1024 noise.KEMFunc = kemMLKEM1024{} diff --git a/handshake/kem_test.go b/handshake/kem_test.go deleted file mode 100644 index c4f40b05..00000000 --- a/handshake/kem_test.go +++ /dev/null @@ -1,208 +0,0 @@ -package handshake - -import ( - "bytes" - "crypto/mlkem" - "net/netip" - "testing" - "time" - - "github.com/DefinedNet/noise" - "github.com/slackhq/nebula/cert" - ct "github.com/slackhq/nebula/cert_test" - "github.com/slackhq/nebula/header" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func newKEMMachine(t *testing.T, cs *testCertState, v CertVerifier, initiator bool, index uint32) *Machine { - t.Helper() - m, err := NewMachine( - cs.version, cs.getCredential, v, - func() (uint32, error) { return index, nil }, - initiator, header.HandshakeIXPSK2MLKEM1024, - ) - require.NoError(t, err) - return m -} - -type kemFixture struct { - initCS, respCS *testCertState - v CertVerifier -} - -func newKEMFixture(t *testing.T, cipher noise.CipherFunc) kemFixture { - ca, _, caKey, _ := ct.NewTestCaCert( - cert.Version2, cert.Curve_CURVE25519, time.Time{}, time.Time{}, nil, nil, nil, - ) - return kemFixture{ - initCS: newTestCertStateWithCipher(t, ca, caKey, "initiator", []netip.Prefix{netip.MustParsePrefix("10.0.0.1/24")}, cipher), - respCS: newTestCertStateWithCipher(t, ca, caKey, "responder", []netip.Prefix{netip.MustParsePrefix("10.0.0.2/24")}, cipher), - v: testVerifier(ct.NewTestCAPool(ca)), - } -} - -func requireKeysMatch(t *testing.T, a, b *Result) { - t.Helper() - c, err := a.EKey.Encrypt(nil, nil, []byte("hello")) - require.NoError(t, err) - p, err := b.DKey.Decrypt(nil, nil, c) - require.NoError(t, err) - assert.Equal(t, []byte("hello"), p) - - c, err = b.EKey.Encrypt(nil, nil, []byte("world")) - require.NoError(t, err) - p, err = a.DKey.Decrypt(nil, nil, c) - require.NoError(t, err) - assert.Equal(t, []byte("world"), p) -} - -func TestKEMHandshake(t *testing.T) { - for _, cipher := range []noise.CipherFunc{noise.CipherChaChaPoly, noise.CipherAESGCM} { - t.Run(cipher.CipherName(), func(t *testing.T) { - f := newKEMFixture(t, cipher) - initM := newKEMMachine(t, f.initCS, f.v, true, 1000) - respM := newKEMMachine(t, f.respCS, f.v, false, 2000) - - msg1, err := initM.Initiate(nil) - require.NoError(t, err) - assert.Equal(t, header.HandshakeIXPSK2MLKEM1024, header.MessageSubType(msg1[1])) - assert.Greater(t, len(msg1), header.Len+mlkem.EncapsulationKeySize1024) - - msg2, respR, err := respM.ProcessPacket(nil, msg1) - require.NoError(t, err) - require.NotNil(t, respR) - assert.Greater(t, len(msg2), header.Len+mlkem.CiphertextSize1024) - - _, initR, err := initM.ProcessPacket(nil, msg2) - require.NoError(t, err) - require.NotNil(t, initR) - - assert.Equal(t, "responder", initR.RemoteCert.Certificate.Name()) - assert.Equal(t, "initiator", respR.RemoteCert.Certificate.Name()) - assert.Equal(t, uint32(2000), initR.RemoteIndex) - assert.Equal(t, uint32(1000), respR.RemoteIndex) - requireKeysMatch(t, initR, respR) - }) - } -} - -// TestKEMSecretMatters checks the ML-KEM secret really reaches the transport -// keys: an initiator holding a different decapsulation key cannot complete even -// though every DH input is genuine. -func TestKEMSecretMatters(t *testing.T) { - f := newKEMFixture(t, noise.CipherChaChaPoly) - initM := newKEMMachine(t, f.initCS, f.v, true, 1000) - respM := newKEMMachine(t, f.respCS, f.v, false, 2000) - - msg1, err := initM.Initiate(nil) - require.NoError(t, err) - msg2, _, err := respM.ProcessPacket(nil, msg1) - require.NoError(t, err) - - other, err := mlkem.GenerateKey1024() - require.NoError(t, err) - initM.kemKey = other - - _, res, err := initM.ProcessPacket(nil, msg2) - require.Error(t, err) - assert.Nil(t, res) -} - -func TestKEMTamperedCiphertext(t *testing.T) { - f := newKEMFixture(t, noise.CipherChaChaPoly) - initM := newKEMMachine(t, f.initCS, f.v, true, 1000) - respM := newKEMMachine(t, f.respCS, f.v, false, 2000) - - msg1, err := initM.Initiate(nil) - require.NoError(t, err) - msg2, respR, err := respM.ProcessPacket(nil, msg1) - require.NoError(t, err) - - t.Run("truncated", func(t *testing.T) { - _, res, err := initM.ProcessPacket(nil, msg2[:header.Len+mlkem.CiphertextSize1024-1]) - require.ErrorIs(t, err, ErrPacketTooShort) - assert.Nil(t, res) - assert.False(t, initM.Failed()) - }) - - t.Run("flipped ciphertext bit", func(t *testing.T) { - bad := bytes.Clone(msg2) - bad[header.Len+10] ^= 1 - _, res, err := initM.ProcessPacket(nil, bad) - require.Error(t, err) - assert.Nil(t, res) - assert.False(t, initM.Failed(), "a forged ciphertext must not kill the handshake") - }) - - t.Run("genuine response still completes", func(t *testing.T) { - _, initR, err := initM.ProcessPacket(nil, msg2) - require.NoError(t, err) - require.NotNil(t, initR) - requireKeysMatch(t, initR, respR) - }) -} - -// TestKEMBadMsg1 drives processPayload by hand. With the psk modifier msg1's -// payload is encrypted under a key derived from the public ephemeral, so a -// forged payload would need re-encrypting; the parsing rules are what matter. -func TestKEMBadMsg1(t *testing.T) { - f := newKEMFixture(t, noise.CipherChaChaPoly) - msg1Flags := subtypeInfos[header.HandshakeIXPSK2MLKEM1024].msgs[0] - cert := f.initCS.creds[cert.Version2] - payload := func(kemKey []byte) []byte { - return MarshalPayload(nil, Payload{ - Cert: cert.Bytes, - CertVersion: uint32(cert.Cert.Version()), - InitiatorIndex: 1000, - Time: 1, - KEMKey: kemKey, - }) - } - dk, err := mlkem.GenerateKey1024() - require.NoError(t, err) - - t.Run("missing KEM key", func(t *testing.T) { - m := newKEMMachine(t, f.respCS, f.v, false, 2000) - err := m.processPayload(payload(nil), msg1Flags) - require.ErrorIs(t, err, ErrUnexpectedContent) - assert.True(t, m.Failed()) - }) - - t.Run("invalid KEM key", func(t *testing.T) { - m := newKEMMachine(t, f.respCS, f.v, false, 2000) - // Coefficients of 0xfff exceed the modulus, which ML-KEM must reject. - bad := bytes.Repeat([]byte{0xff}, mlkem.EncapsulationKeySize1024) - err := m.kemEncapsulate(bad) - require.ErrorIs(t, err, ErrInvalidKEMKey) - err = m.kemEncapsulate(bad[:100]) - require.ErrorIs(t, err, ErrInvalidKEMKey) - }) - - t.Run("valid KEM key", func(t *testing.T) { - m := newKEMMachine(t, f.respCS, f.v, false, 2000) - require.NoError(t, m.kemEncapsulate(dk.EncapsulationKey().Bytes())) - assert.Len(t, m.kemCiphertext, mlkem.CiphertextSize1024) - }) - - t.Run("KEM key on the classical subtype", func(t *testing.T) { - m := newTestMachine(t, f.respCS, f.v, false, 2000) - classical := subtypeInfos[header.HandshakeIXPSK0].msgs[0] - err := m.processPayload(payload(dk.EncapsulationKey().Bytes()), classical) - require.ErrorIs(t, err, ErrUnexpectedContent) - assert.True(t, m.Failed()) - }) -} - -// TestKEMDistinctFromClassical makes sure the two subtypes cannot be mixed: a -// classical responder cannot answer a KEM initiator. -func TestKEMDistinctFromClassical(t *testing.T) { - f := newKEMFixture(t, noise.CipherChaChaPoly) - initM := newKEMMachine(t, f.initCS, f.v, true, 1000) - respM := newTestMachine(t, f.respCS, f.v, false, 2000) - - msg1, err := initM.Initiate(nil) - require.NoError(t, err) - _, _, err = respM.ProcessPacket(nil, msg1) - require.ErrorIs(t, err, ErrSubtypeMismatch) -} diff --git a/handshake/machine.go b/handshake/machine.go index f879bc5e..5337b5ad 100644 --- a/handshake/machine.go +++ b/handshake/machine.go @@ -2,8 +2,6 @@ package handshake import ( "bytes" - "crypto/mlkem" - "errors" "fmt" "slices" "time" @@ -70,9 +68,6 @@ type Machine struct { remoteCertSet bool payloadSet bool failed bool - - kemKey *mlkem.DecapsulationKey1024 // initiator's ephemeral ML-KEM key, until completion - kemCiphertext []byte // responder's ciphertext to prefix its response with } // NewMachine creates a handshake state machine. The subtype determines both @@ -93,7 +88,7 @@ func NewMachine( } // Version 3 certificates go with hybrid patterns and nothing else, the - // classical and phase 1 subtypes use the v2 form instead. + // classical subtype uses the v2 form instead. hybrid := info.kem != nil if hybrid != (version == cert.Version3) { return nil, fmt.Errorf("%w: %v", ErrCertVersionMismatch, version) @@ -246,18 +241,7 @@ func (m *Machine) ProcessPacket(out, packet []byte) ([]byte, *Result, error) { return nil, nil, ErrInitiateNotCalled } - noiseMsg := packet[header.Len:] - if m.myMsgFlags().kemCiphertext { - var err error - // Installing a psk derived from a forged ciphertext is harmless: ReadMessage - // then fails without advancing, and the next packet installs its own. - noiseMsg, err = m.kemDecapsulate(noiseMsg) - if err != nil { - return nil, nil, err - } - } - - msg, cs1, cs2, err := m.hs.ReadMessage(nil, noiseMsg) + msg, cs1, cs2, err := m.hs.ReadMessage(nil, packet[header.Len:]) eKey, dKey := m.cipherStates(cs1, cs2) if err != nil { // Noise ReadMessage failed. The noise library checkpoints and rolls back @@ -318,8 +302,6 @@ func (m *Machine) cipherStates(cs1, cs2 *noise.CipherState) (*noise.CipherState, } func (m *Machine) completed(eKey, dKey *noise.CipherState) *Result { - m.kemKey = nil - m.kemCiphertext = nil m.result.EKey = eKey m.result.DKey = dKey m.result.MessageIndex = uint64(m.hs.MessageIndex()) @@ -328,7 +310,7 @@ func (m *Machine) completed(eKey, dKey *noise.CipherState) *Result { func (m *Machine) processPayload(msg []byte, flags msgFlags) error { if len(msg) == 0 { - if flags.expectsPayload || flags.expectsCert || flags.kemKey { + if flags.expectsPayload || flags.expectsCert { m.failed = true return ErrMissingContent } @@ -354,12 +336,6 @@ func (m *Machine) processPayload(msg []byte, flags msgFlags) error { return ErrUnexpectedContent } - hasKEMKey := len(payload.KEMKey) > 0 - if hasKEMKey != flags.kemKey { - m.failed = true - return ErrUnexpectedContent - } - // Process payload if flags.expectsPayload { var remoteIndex uint32 @@ -386,13 +362,6 @@ func (m *Machine) processPayload(msg []byte, flags msgFlags) error { } } - if flags.kemKey { - if err := m.kemEncapsulate(payload.KEMKey); err != nil { - m.failed = true - return err - } - } - return nil } @@ -462,21 +431,11 @@ func (m *Machine) validateCert(payload Payload) error { } func (m *Machine) marshalOutgoing(flags msgFlags) ([]byte, error) { - if !flags.expectsPayload && !flags.expectsCert && !flags.kemKey { + if !flags.expectsPayload && !flags.expectsCert { return nil, nil } var p Payload - if flags.kemKey { - if m.kemKey == nil { - dk, err := mlkem.GenerateKey1024() - if err != nil { - return nil, fmt.Errorf("generate ML-KEM key: %w", err) - } - m.kemKey = dk - } - p.KEMKey = m.kemKey.EncapsulationKey().Bytes() - } if flags.expectsPayload { if !m.indexAllocated { index, err := m.allocIndex() @@ -528,13 +487,6 @@ func (m *Machine) buildResponse(out []byte) ([]byte, *noise.CipherState, *noise. uint64(m.hs.MessageIndex()+1), ) - if flags.kemCiphertext { - if m.kemCiphertext == nil { - return nil, nil, nil, errors.New("no ML-KEM ciphertext to send") - } - out = append(out, m.kemCiphertext...) - } - // noise.WriteMessage appends the encrypted handshake message to out, // reusing capacity when present. out, cs1, cs2, err := m.hs.WriteMessage(out, hsBytes) diff --git a/handshake/patterns.go b/handshake/patterns.go index d27390f1..ebd8e61a 100644 --- a/handshake/patterns.go +++ b/handshake/patterns.go @@ -12,16 +12,13 @@ import ( type msgFlags struct { expectsPayload bool // message carries indexes and time expectsCert bool // message carries the certificate - kemKey bool // payload carries an ML-KEM encapsulation key - kemCiphertext bool // message is prefixed with the ML-KEM ciphertext for the peer's key } // subtypeInfo bundles the noise pattern with the per-message flags for a // given handshake subtype. type subtypeInfo struct { - pattern noise.HandshakePattern - pskPlacement int // noise psk modifier position, 0 means no psk token - msgs []msgFlags + pattern noise.HandshakePattern + msgs []msgFlags // kem is the KEM of a hybrid pattern, nil for any other. Hybrid patterns // carry composite static keys and need version 3 certificates on both @@ -41,22 +38,6 @@ var subtypeInfos = map[header.MessageSubType]subtypeInfo{ }, }, - // IX with an ephemeral ML-KEM-1024 exchange: - // msg1 (I->R): payload + cert + encapsulation key - // msg2 (R->I): ciphertext || noise(payload + cert) - // The responder encapsulates after reading msg1 and mixes the shared secret in - // through the psk2 token at the end of msg2. The ciphertext travels outside the - // noise message because the initiator needs the secret before it can process - // that token. - header.HandshakeIXPSK2MLKEM1024: { - pattern: noise.HandshakeIX, - pskPlacement: 2, - msgs: []msgFlags{ - {expectsPayload: true, expectsCert: true, kemKey: true}, - {expectsPayload: true, expectsCert: true, kemCiphertext: true}, - }, - }, - // Hybrid IX (github.com/DefinedNet/noise, see its PQ_RESEARCH.md): // msg1 (I->R): e, s payload + cert // msg2 (R->I): ekem, skem, e, ee, se, s, es payload + cert diff --git a/handshake/payload.go b/handshake/payload.go index 93ebf061..4567fc0d 100644 --- a/handshake/payload.go +++ b/handshake/payload.go @@ -20,7 +20,6 @@ type Payload struct { ResponderIndex uint32 Time uint64 CertVersion uint32 - KEMKey []byte // ML-KEM encapsulation key, only in subtypes that carry one } // Proto field numbers for NebulaHandshakeDetails @@ -30,7 +29,6 @@ const ( fieldResponderIndex = 3 // uint32 fieldTime = 5 // uint64 fieldCertVersion = 8 // uint32 - fieldKEMKey = 9 // bytes ) // MarshalPayload encodes a handshake payload in protobuf wire format compatible @@ -59,10 +57,6 @@ func MarshalPayload(out []byte, p Payload) []byte { details = protowire.AppendTag(details, fieldCertVersion, protowire.VarintType) details = protowire.AppendVarint(details, uint64(p.CertVersion)) } - if len(p.KEMKey) > 0 { - details = protowire.AppendTag(details, fieldKEMKey, protowire.BytesType) - details = protowire.AppendBytes(details, p.KEMKey) - } out = protowire.AppendTag(out, 1, protowire.BytesType) out = protowire.AppendBytes(out, details) @@ -167,16 +161,6 @@ func unmarshalPayloadDetails(p *Payload, b []byte) error { } p.CertVersion = uint32(v) b = b[n:] - case fieldKEMKey: - if typ != protowire.BytesType { - return errInvalidHandshakeDetails - } - v, n := protowire.ConsumeBytes(b) - if n < 0 { - return errInvalidHandshakeDetails - } - p.KEMKey = append([]byte(nil), v...) - b = b[n:] default: n := protowire.ConsumeFieldValue(num, typ, b) if n < 0 { diff --git a/handshake_manager.go b/handshake_manager.go index 0c76ddf3..96704ad7 100644 --- a/handshake_manager.go +++ b/handshake_manager.go @@ -191,7 +191,7 @@ func (hm *HandshakeManager) HandleIncoming(via ViaSender, packet []byte, h *head // don't yet support) are dropped here rather than silently routed through // the IX path. Add a case when introducing a new pattern. switch { - case h.Subtype == header.HandshakeIXPSK0, h.Subtype == header.HandshakeIXPSK2MLKEM1024, handshake.IsHybrid(h.Subtype): + case h.Subtype == header.HandshakeIXPSK0, handshake.IsHybrid(h.Subtype): // supported default: hm.l.Debug("dropping handshake with unsupported subtype", @@ -870,6 +870,13 @@ func (hm *HandshakeManager) beginHandshake(via ViaSender, packet []byte, h *head return } + // A classical attempt crossing the peer's hybrid one goes unanswered. + if !handshake.IsHybrid(machine.Subtype()) && hm.crossesHybrid(vpnAddrs, time.Now()) { + f.l.Debug("Ignoring classical handshake that crossed a hybrid one", + "vpnAddrs", vpnAddrs, "from", via, "window", hm.hybridWindow()) + return + } + connState, err := newConnectionStateFromResult(result) if err != nil { f.l.Error("Discarding handshake with an invalid message index", "error", err, "vpnAddrs", vpnAddrs) diff --git a/handshake_manager_hybrid.go b/handshake_manager_hybrid.go index 76ee3c44..84ee5da1 100644 --- a/handshake_manager_hybrid.go +++ b/handshake_manager_hybrid.go @@ -4,6 +4,7 @@ import ( "context" "errors" "log/slog" + "net/netip" "sync" "time" @@ -60,15 +61,63 @@ type hybridState struct { confirmsMu sync.Mutex confirms map[*HostInfo]*hybridConfirm + + // answered holds when this responder answered each peer's latest new + // hybrid msg1, for the crossing window. + answeredMu sync.Mutex + answered map[netip.Addr]time.Time } func newHybridState() hybridState { return hybridState{ responders: map[uint32]*HandshakeHostInfo{}, confirms: map[*HostInfo]*hybridConfirm{}, + answered: map[netip.Addr]time.Time{}, } } +// A peer that sent a hybrid msg1 alternates, and its first classical attempt +// follows anywhere from right away to three retry intervals later: the retry +// timer is set one interval out, lands two slots ahead on the timer wheel, and +// the wheel only moves on whole ticks of the manager's clock. Answering that +// classical msg1 races the hybrid handshake and leaves a classical tunnel behind +// on the responder whichever wins. So for four retry intervals after answering a +// peer's hybrid msg1, its classical msg1s go unanswered; the crossing one arrives +// within that wherever the round trip, since both travel the same path. The +// window is short on purpose: an initiator whose hybrid attempt fails on its own +// side, say one that does not trust our v3 CA yet, still gets a classical answer +// to a later attempt. Blocking for as long as any hybrid state remains would +// refuse it until its handshake timed out, and its next handshake would start +// with a hybrid msg1 again. + +// hybridWindow is how long classical msg1s go unanswered after a hybrid msg1. +func (hm *HandshakeManager) hybridWindow() time.Duration { + return 4 * hm.config.tryInterval +} + +// noteHybridMsg1 opens the crossing window for a peer whose new hybrid msg1 this +// responder answered. +func (hm *HandshakeManager) noteHybridMsg1(vpnAddrs []netip.Addr, now time.Time) { + hm.hybrid.answeredMu.Lock() + defer hm.hybrid.answeredMu.Unlock() + for _, a := range vpnAddrs { + hm.hybrid.answered[a] = now + } +} + +// crossesHybrid reports whether a classical msg1 from a peer arrived within the +// crossing window of its hybrid msg1. +func (hm *HandshakeManager) crossesHybrid(vpnAddrs []netip.Addr, now time.Time) bool { + hm.hybrid.answeredMu.Lock() + defer hm.hybrid.answeredMu.Unlock() + for _, a := range vpnAddrs { + if at, ok := hm.hybrid.answered[a]; ok && now.Sub(at) < hm.hybridWindow() { + return true + } + } + return false +} + // awaitMsg3 keeps a responder's hybrid handshake pending after msg1, and sends // msg2. hh is already in the indexes under the local index the Machine // allocated. @@ -128,6 +177,7 @@ func (hm *HandshakeManager) awaitMsg3(via ViaSender, hh *HandshakeHostInfo, mach hm.sendHandshakeResponse(via, response, hostinfo, false, received) hm.pqAnswered.add(packet[header.Len:], hostinfo, now) + hm.noteHybridMsg1(vpnAddrs, now) } // continueHybridResponder collects msg3 fragments for a pending responder and @@ -294,6 +344,14 @@ func (hm *HandshakeManager) isHybridResponder(index uint32) bool { // expireHybridResponders drops responders that never got their msg3. func (hm *HandshakeManager) expireHybridResponders(now time.Time) { + hm.hybrid.answeredMu.Lock() + for a, at := range hm.hybrid.answered { + if now.Sub(at) >= hm.hybridWindow() { + delete(hm.hybrid.answered, a) + } + } + hm.hybrid.answeredMu.Unlock() + hm.Lock() defer hm.Unlock() for idx, hh := range hm.hybrid.responders { diff --git a/handshake_manager_pq.go b/handshake_manager_pq.go index d6d9c5a1..a4370d29 100644 --- a/handshake_manager_pq.go +++ b/handshake_manager_pq.go @@ -72,6 +72,17 @@ func (hm *HandshakeManager) pqConfig() *pqHandshakeConfig { return hm.pq.Load() } +// warnPQWithoutV3 says so when post-quantum settings are on but the node has no +// version 3 certificate: post-quantum handshakes are hybrid only, so they do +// nothing until it has one. require is refused outright when loading pki. +func warnPQWithoutV3(l *slog.Logger, cfg pqHandshakeConfig, cs *CertState) { + if _, ok := cs.hybridSubtype(); ok || !(cfg.initiate || cfg.accept || cfg.sticky) { + return + } + l.Warn("handshakes.pq has no effect without a version 3 certificate in pki.cert", + "initiate", cfg.initiate, "accept", cfg.accept, "sticky", cfg.sticky) +} + // reloadPQ applies changed post-quantum settings. Handshakes already in flight // keep going; the new settings apply to the next attempt and packet. func (hm *HandshakeManager) reloadPQ(c *config.C) { @@ -80,6 +91,12 @@ func (hm *HandshakeManager) reloadPQ(c *config.C) { } old := hm.pqConfig() cfg := pqHandshakeConfigFrom(c) + cs := hm.f.pki.getCertState() + if _, ok := cs.hybridSubtype(); cfg.require && !ok { + hm.l.Error("Refusing handshakes.pq.require without a version 3 certificate in pki.cert, keeping the previous settings") + return + } + warnPQWithoutV3(hm.l, cfg, cs) hm.pq.Store(&cfg) hm.l.Info("Post-quantum handshake settings changed", "initiate", cfg.initiate, "oldInitiate", old.initiate, @@ -220,6 +237,9 @@ func reassembled(h *header.H, msg []byte) []byte { // it, so classical stops getting turns. func (hm *HandshakeManager) usePQ(hh *HandshakeHostInfo, vpnIp netip.Addr, triggered bool) bool { cfg := hm.pqConfig() + if _, ok := hm.f.pki.getCertState().hybridSubtype(); !ok { + return false + } switch { case !cfg.initiate, hh.noPQ: return false @@ -281,11 +301,11 @@ func (hm *HandshakeManager) relayEstablished(target netip.Addr, relayHI *HostInf func (hm *HandshakeManager) buildPQAttempt(hh *HandshakeHostInfo) bool { cs := hm.f.pki.getCertState() v := hm.initiatingVersion(hh, cs) - subtype := header.HandshakeIXPSK2MLKEM1024 - if st, ok := cs.hybridSubtype(); ok { - v = cert.Version3 - subtype = st + subtype, ok := cs.hybridSubtype() + if !ok { + return false } + v = cert.Version3 if cs.GetCredential(v) == nil { hm.f.l.Error("Unable to handshake with host because no certificate is available", "vpnAddrs", hh.hostinfo.vpnAddrs, "certVersion", v) @@ -410,23 +430,17 @@ func (hm *HandshakeManager) handleFragment(via ViaSender, packet []byte, h *head hm.l.Debug("dropping post-quantum handshake, not accepting them", "from", via) return } - hybrid := handshake.IsHybrid(h.Subtype) - if hybrid || cfg.require { - mine, hasV3 := hm.f.pki.getCertState().hybridSubtype() - switch { - case hybrid && !hasV3: - hm.l.Debug("dropping hybrid handshake, no v3 certificate to answer it with", "from", via) - return - case hybrid && h.Subtype != mine: - // The peer's suite uses another KEM. Like a peer without PQ, it - // falls back to classical unless either side requires PQ. - hm.l.Debug("dropping hybrid handshake, our v3 certificate's suite uses another KEM", "from", via, - "style", header.SubTypeName(header.Handshake, h.Subtype), "ours", header.SubTypeName(header.Handshake, mine)) - return - case !hybrid && hasV3: - hm.l.Debug("dropping post-quantum handshake, hybrid handshakes are required", "from", via) - return - } + mine, hasV3 := hm.f.pki.getCertState().hybridSubtype() + switch { + case !hasV3: + hm.l.Debug("dropping hybrid handshake, no v3 certificate to answer it with", "from", via) + return + case h.Subtype != mine: + // The peer's suite uses another KEM. Like a peer without PQ, it + // falls back to classical unless either side requires PQ. + hm.l.Debug("dropping hybrid handshake, our v3 certificate's suite uses another KEM", "from", via, + "style", header.SubTypeName(header.Handshake, h.Subtype), "ours", header.SubTypeName(header.Handshake, mine)) + return } if h.RemoteIndex != 0 { hm.l.Debug("dropping stage-1 handshake with non-zero RemoteIndex", diff --git a/handshake_manager_pq_test.go b/handshake_manager_pq_test.go index 673c599f..8da03755 100644 --- a/handshake_manager_pq_test.go +++ b/handshake_manager_pq_test.go @@ -8,6 +8,8 @@ import ( "testing" "time" + "github.com/slackhq/nebula/cert" + "github.com/slackhq/nebula/cert_test" "github.com/slackhq/nebula/config" "github.com/slackhq/nebula/handshake/fragment" "github.com/slackhq/nebula/header" @@ -49,11 +51,26 @@ func newPQTestManager(t *testing.T) (*HandshakeManager, *recordingConn) { handshakeManager: hm, outside: conn, messageMetrics: newMessageMetricsOnlyRecvError(), + pki: newV3TestPKI(t), l: l, } return hm, conn } +// newV3TestPKI holds a version 3 certificate, which post-quantum handshakes need. +func newV3TestPKI(t *testing.T) *PKI { + t.Helper() + ca, _, caKey, _ := cert_test.NewTestCaCert(cert.Version3, cert.Curve_CURVE25519, time.Time{}, time.Time{}, nil, nil, nil) + c, _, keyPEM, _ := cert_test.NewTestCert(cert.Version3, cert.Curve_CURVE25519, ca, caKey, "me", time.Time{}, time.Time{}, []netip.Prefix{netip.MustParsePrefix("10.0.0.1/24")}, nil, nil) + key, _, curve, err := cert.UnmarshalPrivateKeyFromPEM(keyPEM) + require.NoError(t, err) + cs, err := newCertState(cert.Version2, nil, cert.V2Form(c), c, false, curve, key, "aes") + require.NoError(t, err) + p := &PKI{} + p.cs.Store(cs) + return p +} + // fragmentIndexes checks every packet is a fragment of the expected response and // returns their symbol indexes. func fragmentIndexes(t *testing.T, pkts [][]byte, fragID, remoteIndex uint32) []int { @@ -63,7 +80,7 @@ func fragmentIndexes(t *testing.T, pkts [][]byte, fragID, remoteIndex uint32) [] require.LessOrEqual(t, len(p), fragment.MaxPacketLen) var h header.H require.NoError(t, h.Parse(p)) - assert.Equal(t, header.HandshakeIXPSK2MLKEM1024, h.Subtype) + assert.Equal(t, header.HandshakeIXHybridMLKEM1024, h.Subtype) assert.Equal(t, remoteIndex, h.RemoteIndex) assert.Equal(t, uint64(2), h.MessageCounter) fh, _, err := fragment.Parse(p[header.Len:]) @@ -81,7 +98,7 @@ func TestSendFragmentedResponse(t *testing.T) { // A 2000 byte message is 2 symbols of 1000, so packets of 1024 bytes. msg := make([]byte, header.Len, header.Len+2000) - header.Encode(msg, header.Version, header.Handshake, header.HandshakeIXPSK2MLKEM1024, 5, 2) + header.Encode(msg, header.Version, header.Handshake, header.HandshakeIXHybridMLKEM1024, 5, 2) r := rand.New(rand.NewPCG(1, 1)) for range 2000 { msg = append(msg, byte(r.Uint32())) @@ -184,7 +201,7 @@ func TestPQCookies(t *testing.T) { // pqMsg1Fragment builds the first fragment of a msg1, optionally with a cookie. func pqMsg1Fragment(t *testing.T, fragID uint32, cookie *fragment.Cookie) ([]byte, *header.H) { t.Helper() - s, err := fragment.NewSender(make([]byte, 2000), header.HandshakeIXPSK2MLKEM1024, 0, 1, fragID) + s, err := fragment.NewSender(make([]byte, 2000), header.HandshakeIXHybridMLKEM1024, 0, 1, fragID) require.NoError(t, err) if cookie != nil { s.SetCookie(*cookie) @@ -239,7 +256,7 @@ func TestPQCookieUnderLoad(t *testing.T) { var rh header.H require.NoError(t, rh.Parse(reply)) assert.Equal(t, header.Handshake, rh.Type) - assert.Equal(t, header.HandshakeIXPSK2MLKEM1024, rh.Subtype) + assert.Equal(t, header.HandshakeIXHybridMLKEM1024, rh.Subtype) assert.Equal(t, uint32(7), rh.RemoteIndex, "the reply names the initiator's FragID") assert.Equal(t, uint64(pqCookieCounter), rh.MessageCounter) copy(cookie[:], reply[header.Len:]) @@ -271,7 +288,7 @@ func TestPQCookieUnderLoad(t *testing.T) { t.Run("a reply never outgrows what prompted it", func(t *testing.T) { p, h := pqMsg1Fragment(t, 8, nil) - hm.sendCookie(via, header.HandshakeIXPSK2MLKEM1024, fragment.Header{FragID: 8}, pqFragKey{}, pqCookieReplyLen-1, time.Now()) + hm.sendCookie(via, header.HandshakeIXHybridMLKEM1024, fragment.Header{FragID: 8}, pqFragKey{}, pqCookieReplyLen-1, time.Now()) assert.Empty(t, conn.take()) hm.handleFragment(via, p, h) assert.Len(t, conn.take(), 1) @@ -282,7 +299,7 @@ func TestPQHandleCookie(t *testing.T) { hm, conn := newPQTestManager(t) via := ViaSender{UdpAddr: netip.MustParseAddrPort("192.0.2.1:4242")} - s, err := fragment.NewSender(make([]byte, 2000), header.HandshakeIXPSK2MLKEM1024, 0, 1, 99) + s, err := fragment.NewSender(make([]byte, 2000), header.HandshakeIXHybridMLKEM1024, 0, 1, 99) require.NoError(t, err) _, _, err = s.Attempt(2) require.NoError(t, err) @@ -290,7 +307,7 @@ func TestPQHandleCookie(t *testing.T) { hm.indexes[99] = hh reply := make([]byte, header.Len, pqCookieReplyLen) - header.Encode(reply, header.Version, header.Handshake, header.HandshakeIXPSK2MLKEM1024, 99, pqCookieCounter) + header.Encode(reply, header.Version, header.Handshake, header.HandshakeIXHybridMLKEM1024, 99, pqCookieCounter) reply = append(reply, bytes16(0xab)...) var h header.H require.NoError(t, h.Parse(reply)) @@ -312,7 +329,7 @@ func TestPQHandleCookie(t *testing.T) { t.Run("replies for other indexes are ignored", func(t *testing.T) { other := append([]byte(nil), reply...) - header.Encode(other, header.Version, header.Handshake, header.HandshakeIXPSK2MLKEM1024, 98, pqCookieCounter) + header.Encode(other, header.Version, header.Handshake, header.HandshakeIXHybridMLKEM1024, 98, pqCookieCounter) var oh header.H require.NoError(t, oh.Parse(other)) hh.pq.cookieResendAt = time.Time{} @@ -336,6 +353,7 @@ func TestReloadPQ(t *testing.T) { cfg := defaultHandshakeConfig cfg.pq = pqHandshakeConfigFrom(c) hm := NewHandshakeManager(l, newHostMap(l), newTestLighthouse(), &udp.NoopConn{}, cfg) + hm.f = &Interface{pki: newV3TestPKI(t)} c.RegisterReloadCallback(hm.reloadPQ) assert.Equal(t, pqHandshakeConfig{repairDelay: DefaultPQRepairDelay}, *hm.pqConfig()) @@ -394,7 +412,7 @@ func TestUsePQ(t *testing.T) { set(pqHandshakeConfig{initiate: true, sticky: true}) assert.Equal(t, []bool{true, false}, attempts(false, false), "sticky alternates until the peer has done PQ") - hm.notePQPeer(header.HandshakeIXPSK2MLKEM1024, []netip.Addr{peer}) + hm.notePQPeer(header.HandshakeIXHybridMLKEM1024, []netip.Addr{peer}) assert.Equal(t, []bool{true, true, true}, attempts(false, false, false), "then only PQ") } @@ -528,3 +546,59 @@ func TestPQMsg2Slots(t *testing.T) { assert.False(t, hh.pq.heard) }) } + +// Post-quantum handshakes are hybrid only, so without a version 3 certificate +// there is nothing to initiate and require is refused. +func TestPQNeedsV3(t *testing.T) { + l := test.NewLogger() + ca, _, caKey, _ := cert_test.NewTestCaCert(cert.Version2, cert.Curve_CURVE25519, time.Time{}, time.Time{}, nil, nil, nil) + c2, _, keyPEM, _ := cert_test.NewTestCert(cert.Version2, cert.Curve_CURVE25519, ca, caKey, "me", time.Time{}, time.Time{}, []netip.Prefix{netip.MustParsePrefix("10.0.0.1/24")}, nil, nil) + key, _, curve, err := cert.UnmarshalPrivateKeyFromPEM(keyPEM) + require.NoError(t, err) + cs, err := newCertState(cert.Version2, nil, c2, nil, false, curve, key, "aes") + require.NoError(t, err) + v2PKI := &PKI{} + v2PKI.cs.Store(cs) + + hm := NewHandshakeManager(l, newHostMap(l), newTestLighthouse(), &udp.NoopConn{}, defaultHandshakeConfig) + hm.f = &Interface{pki: v2PKI} + hm.pq.Store(&pqHandshakeConfig{initiate: true, accept: true}) + assert.False(t, hm.usePQ(&HandshakeHostInfo{counter: 1}, netip.MustParseAddr("10.0.0.2"), false), "no v3 certificate, no hybrid attempt") + + conf := config.NewC(l) + require.NoError(t, conf.LoadString(`handshakes: {pq: {initiate: true}}`)) + conf.RegisterReloadCallback(hm.reloadPQ) + require.NoError(t, conf.ReloadConfigString(`handshakes: {pq: {require: true}}`)) + assert.False(t, hm.pqConfig().require, "require without a v3 certificate is refused on reload") + + // And when loading the pki. + certPEM, err := c2.MarshalPEM() + require.NoError(t, err) + conf = config.NewC(l) + conf.Settings["pki"] = map[string]any{"cert": string(certPEM), "key": string(keyPEM)} + conf.Settings["handshakes"] = map[string]any{"pq": map[string]any{"require": true}} + _, err = newCertStateFromConfig(conf, "aes") + require.ErrorContains(t, err, "handshakes.pq.require needs a version 3 certificate") +} + +// After answering a peer's hybrid msg1, its classical msg1s go unanswered for +// four retry intervals, and only for that peer. +func TestHybridCrossingWindow(t *testing.T) { + hm, _ := newPQTestManager(t) + peer := []netip.Addr{netip.MustParseAddr("10.1.0.1")} + other := []netip.Addr{netip.MustParseAddr("10.1.0.2")} + now := time.Unix(1000, 0) + w := 4 * hm.config.tryInterval + assert.Equal(t, w, hm.hybridWindow()) + + assert.False(t, hm.crossesHybrid(peer, now)) + hm.noteHybridMsg1(peer, now) + assert.True(t, hm.crossesHybrid(peer, now.Add(w-time.Millisecond))) + assert.False(t, hm.crossesHybrid(peer, now.Add(w))) + assert.False(t, hm.crossesHybrid(other, now)) + + hm.expireHybridResponders(now.Add(w)) + hm.hybrid.answeredMu.Lock() + assert.Empty(t, hm.hybrid.answered, "expired windows are forgotten") + hm.hybrid.answeredMu.Unlock() +} diff --git a/header/header.go b/header/header.go index e8e5829d..d6f5cef4 100644 --- a/header/header.go +++ b/header/header.go @@ -68,10 +68,8 @@ const ( HandshakeIXPSK0 MessageSubType = 0 HandshakeXXPSK0 MessageSubType = 1 - // HandshakeIXPSK2MLKEM1024 is IX with an ephemeral ML-KEM-1024 exchange mixed - // in through the psk2 slot. Its messages exceed the MTU and travel as - // fragments (see handshake/fragment). - HandshakeIXPSK2MLKEM1024 MessageSubType = 2 + // 2 was ix_psk2_mlkem1024, an ephemeral ML-KEM exchange over IX on version 2 + // certificates that was never released. Do not reuse it. // HandshakeIXHybridMLKEM1024 is the hybrid IX of github.com/DefinedNet/noise: // IX with X25519 or P256 and ML-KEM-1024 side by side, authenticated by @@ -82,7 +80,7 @@ const ( // IsFragmented reports whether handshake messages of this subtype travel as // fragments. func IsFragmented(st MessageSubType) bool { - return st == HandshakeIXPSK2MLKEM1024 || st == HandshakeIXHybridMLKEM1024 + return st == HandshakeIXHybridMLKEM1024 } var ErrHeaderTooShort = errors.New("header is too short") @@ -105,7 +103,6 @@ var subTypeMap = map[MessageType]*map[MessageSubType]string{ CloseTunnel: &subTypeNoneMap, Handshake: { HandshakeIXPSK0: "ix_psk0", - HandshakeIXPSK2MLKEM1024: "ix_psk2_mlkem1024", HandshakeIXHybridMLKEM1024: "ix_hybrid_mlkem1024", }, Control: &subTypeNoneMap, @@ -217,7 +214,7 @@ func IsValidSubType(t MessageType, s MessageSubType) bool { case Message: return s == MessageNone || s == MessageRelay case Handshake: - return s == HandshakeIXPSK0 || s == HandshakeIXPSK2MLKEM1024 || s == HandshakeIXHybridMLKEM1024 + return s == HandshakeIXPSK0 || s == HandshakeIXHybridMLKEM1024 case Test: return s == TestReply || s == TestRequest case Control, CloseTunnel, RecvError, LightHouse: diff --git a/header/header_test.go b/header/header_test.go index 505040d3..cb259c57 100644 --- a/header/header_test.go +++ b/header/header_test.go @@ -97,7 +97,6 @@ func TestTypeMap(t *testing.T) { CloseTunnel: &subTypeNoneMap, Handshake: { HandshakeIXPSK0: "ix_psk0", - HandshakeIXPSK2MLKEM1024: "ix_psk2_mlkem1024", HandshakeIXHybridMLKEM1024: "ix_hybrid_mlkem1024", }, Control: &subTypeNoneMap, @@ -124,7 +123,9 @@ func TestIsValidSubType(t *testing.T) { assert.False(t, IsValidSubType(Message, 2)) assert.True(t, IsValidSubType(Handshake, HandshakeIXPSK0)) - assert.True(t, IsValidSubType(Handshake, HandshakeIXPSK2MLKEM1024)) + assert.True(t, IsValidSubType(Handshake, HandshakeIXHybridMLKEM1024)) + // 2 was the unreleased ix_psk2_mlkem1024 and stays reserved. + assert.False(t, IsValidSubType(Handshake, 2)) // HandshakeXXPSK0 is defined but not a wire-valid subtype. assert.False(t, IsValidSubType(Handshake, HandshakeXXPSK0)) diff --git a/main.go b/main.go index 21168481..1f5d2b41 100644 --- a/main.go +++ b/main.go @@ -235,6 +235,7 @@ func Main(c *config.C, configTest bool, buildVersion string, l *slog.Logger, dev pq: pqHandshakeConfigFrom(c), } + warnPQWithoutV3(l, handshakeConfig.pq, pki.getCertState()) handshakeManager := NewHandshakeManager(l, hostMap, lightHouse, udpConns[0], handshakeConfig) c.RegisterReloadCallback(handshakeManager.reloadPQ) lightHouse.handshakeTrigger = handshakeManager.trigger diff --git a/pki.go b/pki.go index 893c258c..3a88b431 100644 --- a/pki.go +++ b/pki.go @@ -222,7 +222,6 @@ func (cs *CertState) GetDefaultCertificate() cert.Certificate { // DefaultVersion returns the preferred cert version for initiating handshakes. func (cs *CertState) DefaultVersion() cert.Version { return cs.initiatingVersion } -// GetCredential returns the pre-computed handshake credential for the given version, or nil. // hybridSubtype returns the hybrid handshake subtype for the KEM of the v3 // certificate's suite, false without a v3 certificate. func (cs *CertState) hybridSubtype() (header.MessageSubType, bool) { @@ -232,6 +231,7 @@ func (cs *CertState) hybridSubtype() (header.MessageSubType, bool) { return handshake.HybridSubtype(cert.PQSuiteOf(cs.v3Cert)) } +// GetCredential returns the pre-computed handshake credential for the given version, or nil. func (cs *CertState) GetCredential(v cert.Version) *handshake.Credential { switch v { case cert.Version1: @@ -446,6 +446,12 @@ func newCertStateFromConfig(c *config.C, cipher string) (*CertState, error) { return nil, fmt.Errorf("unknown pki.initiating_version: %v", rawInitiatingVersion) } + // Post-quantum handshakes are hybrid only, so requiring them without a v3 + // certificate would leave this node unable to reach anyone. + if v3 == nil && c.GetBool("handshakes.pq.require", false) { + return nil, errors.New("handshakes.pq.require needs a version 3 certificate in pki.cert") + } + return newCertState(initiatingVersion, v1, v2, v3, isPkcs11, curve, rawKey, cipher) }