mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-07 14:09:07 +02:00
fix handling of empty network map during decode and encode
This commit is contained in:
@@ -61,6 +61,8 @@ func EncodeNetworkMapEnvelope(in ComponentsEnvelopeInput) *proto.NetworkMapEnvel
|
|||||||
return &proto.NetworkMapEnvelope{
|
return &proto.NetworkMapEnvelope{
|
||||||
Payload: &proto.NetworkMapEnvelope_Full{
|
Payload: &proto.NetworkMapEnvelope_Full{
|
||||||
Full: &proto.NetworkMapComponentsFull{
|
Full: &proto.NetworkMapComponentsFull{
|
||||||
|
Serial: networkSerial(c.Network),
|
||||||
|
Network: toAccountNetwork(c.Network),
|
||||||
PeerConfig: in.PeerConfig,
|
PeerConfig: in.PeerConfig,
|
||||||
// components.Peers always contains the target peer
|
// components.Peers always contains the target peer
|
||||||
Peers: []*proto.PeerCompact{toPeerCompact(c.Peers[c.PeerID])},
|
Peers: []*proto.PeerCompact{toPeerCompact(c.Peers[c.PeerID])},
|
||||||
|
|||||||
@@ -758,6 +758,9 @@ func TestEncodeNetworkMapEnvelope_NilComponentsGracefulDegrade(t *testing.T) {
|
|||||||
assert.Equal(t, "netbird.cloud", full.DnsDomain)
|
assert.Equal(t, "netbird.cloud", full.DnsDomain)
|
||||||
assert.Len(t, full.Peers, 1)
|
assert.Len(t, full.Peers, 1)
|
||||||
assert.Empty(t, full.Policies)
|
assert.Empty(t, full.Policies)
|
||||||
|
require.NotNil(t, full.Network, "client runs Calculate() over the envelope and dereferences Network unconditionally; a nil here would crash the receiver")
|
||||||
|
assert.Equal(t, "net-empty", full.Network.Identifier)
|
||||||
|
assert.Equal(t, uint64(9), full.Serial)
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestEncodeNetworkMapEnvelope_AccountSettingsAlwaysEmitted(t *testing.T) {
|
func TestEncodeNetworkMapEnvelope_AccountSettingsAlwaysEmitted(t *testing.T) {
|
||||||
@@ -776,6 +779,12 @@ func TestEncodeNetworkMapEnvelope_AccountSettingsAlwaysEmitted(t *testing.T) {
|
|||||||
func emptyNetworkMapComponents() *types.NetworkMapComponents {
|
func emptyNetworkMapComponents() *types.NetworkMapComponents {
|
||||||
return types.EmptyNetworkMapComponents(
|
return types.EmptyNetworkMapComponents(
|
||||||
&types.NetworkMapComponents{
|
&types.NetworkMapComponents{
|
||||||
PeerID: "peer-id", Peers: map[string]*types.ComponentPeer{"peer-id": {}}},
|
PeerID: "peer-id", Peers: map[string]*types.ComponentPeer{"peer-id": {}},
|
||||||
|
Network: &types.Network{
|
||||||
|
Identifier: "net-empty",
|
||||||
|
Net: net.IPNet{IP: net.IP{100, 64, 0, 0}, Mask: net.CIDRMask(10, 32)},
|
||||||
|
Serial: 9,
|
||||||
|
},
|
||||||
|
},
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -228,15 +228,17 @@ func DecodeEnvelope(env *proto.NetworkMapEnvelope) (*types.NetworkMapComponents,
|
|||||||
return c, nil
|
return c, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// decodeAccountNetwork never returns nil — Calculate() dereferences
|
||||||
|
// c.Network unconditionally, and servers that predate the fix omit the field
|
||||||
|
// entirely from the empty-components envelope.
|
||||||
func decodeAccountNetwork(an *proto.AccountNetwork) *types.Network {
|
func decodeAccountNetwork(an *proto.AccountNetwork) *types.Network {
|
||||||
|
n := &types.Network{}
|
||||||
if an == nil {
|
if an == nil {
|
||||||
return nil
|
return n
|
||||||
}
|
|
||||||
n := &types.Network{
|
|
||||||
Identifier: an.Identifier,
|
|
||||||
Dns: an.Dns,
|
|
||||||
Serial: an.Serial,
|
|
||||||
}
|
}
|
||||||
|
n.Identifier = an.Identifier
|
||||||
|
n.Dns = an.Dns
|
||||||
|
n.Serial = an.Serial
|
||||||
if an.NetCidr != "" {
|
if an.NetCidr != "" {
|
||||||
if _, ipnet, err := net.ParseCIDR(an.NetCidr); err == nil && ipnet != nil {
|
if _, ipnet, err := net.ParseCIDR(an.NetCidr); err == nil && ipnet != nil {
|
||||||
n.Net = *ipnet
|
n.Net = *ipnet
|
||||||
|
|||||||
@@ -221,6 +221,66 @@ func TestEnvelopeRoundTrip_AllGroupShortCircuitParity(t *testing.T) {
|
|||||||
"client-side Calculate must connect the same remote peers as the server")
|
"client-side Calculate must connect the same remote peers as the server")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestEnvelopeToNetworkMap_EmptyComponents covers the graceful-degrade path
|
||||||
|
// the server takes for a peer that is missing from the account or absent from
|
||||||
|
// the validated-peers map. The legacy server short-circuited before
|
||||||
|
// Calculate() and shipped a NetworkMap carrying only the account Network; the
|
||||||
|
// components path runs Calculate() on the client instead, so the envelope must
|
||||||
|
// carry Network or the client panics dereferencing a nil *types.Network.
|
||||||
|
func TestEnvelopeToNetworkMap_EmptyComponents(t *testing.T) {
|
||||||
|
localPeerKey := randomWgKey(t)
|
||||||
|
c := types.EmptyNetworkMapComponents(&types.NetworkMapComponents{
|
||||||
|
PeerID: "peer-A",
|
||||||
|
Network: &types.Network{
|
||||||
|
Identifier: "net-empty",
|
||||||
|
Net: net.IPNet{IP: net.IP{100, 64, 0, 0}, Mask: net.CIDRMask(10, 32)},
|
||||||
|
Serial: 7,
|
||||||
|
},
|
||||||
|
Peers: map[string]*types.ComponentPeer{
|
||||||
|
"peer-A": {ID: "peer-A", Key: localPeerKey, IP: netip.AddrFrom4([4]byte{100, 64, 0, 1})},
|
||||||
|
},
|
||||||
|
})
|
||||||
|
|
||||||
|
envelope := mgmtgrpc.EncodeNetworkMapEnvelope(mgmtgrpc.ComponentsEnvelopeInput{
|
||||||
|
Components: c,
|
||||||
|
DNSDomain: "netbird.cloud",
|
||||||
|
})
|
||||||
|
require.NotNil(t, envelope.GetFull().Network, "empty envelope must carry the account Network")
|
||||||
|
|
||||||
|
wire, err := goproto.Marshal(envelope)
|
||||||
|
require.NoError(t, err, "marshal envelope")
|
||||||
|
var decoded proto.NetworkMapEnvelope
|
||||||
|
require.NoError(t, goproto.Unmarshal(wire, &decoded), "unmarshal envelope")
|
||||||
|
|
||||||
|
result, err := nbnetworkmap.EnvelopeToNetworkMap(context.Background(), &decoded, localPeerKey, "netbird.cloud")
|
||||||
|
require.NoError(t, err, "EnvelopeToNetworkMap must degrade gracefully on empty components")
|
||||||
|
require.Equal(t, uint64(7), result.NetworkMap.Serial)
|
||||||
|
require.Empty(t, result.NetworkMap.RemotePeers, "unvalidated peer connects to nobody")
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestEnvelopeToNetworkMap_MissingNetwork simulates a server that omits
|
||||||
|
// AccountNetwork from the envelope. Clients must degrade rather than panic, so
|
||||||
|
// they survive talking to a management server that predates the encoder fix.
|
||||||
|
func TestEnvelopeToNetworkMap_MissingNetwork(t *testing.T) {
|
||||||
|
c, localPeerKey := buildSmokeComponents(t)
|
||||||
|
|
||||||
|
envelope := mgmtgrpc.EncodeNetworkMapEnvelope(mgmtgrpc.ComponentsEnvelopeInput{
|
||||||
|
Components: c,
|
||||||
|
DNSDomain: "netbird.cloud",
|
||||||
|
})
|
||||||
|
envelope.GetFull().Network = nil
|
||||||
|
|
||||||
|
wire, err := goproto.Marshal(envelope)
|
||||||
|
require.NoError(t, err, "marshal envelope")
|
||||||
|
var decoded proto.NetworkMapEnvelope
|
||||||
|
require.NoError(t, goproto.Unmarshal(wire, &decoded), "unmarshal envelope")
|
||||||
|
|
||||||
|
result, err := nbnetworkmap.EnvelopeToNetworkMap(context.Background(), &decoded, localPeerKey, "netbird.cloud")
|
||||||
|
require.NoError(t, err, "a missing AccountNetwork must not panic the client")
|
||||||
|
require.NotNil(t, result.Components.Network)
|
||||||
|
require.NotEmpty(t, result.NetworkMap.RemotePeers, "the rest of the snapshot stays usable")
|
||||||
|
}
|
||||||
|
|
||||||
// buildSmokeComponents returns a minimal NetworkMapComponents (2 peers, 1
|
// buildSmokeComponents returns a minimal NetworkMapComponents (2 peers, 1
|
||||||
// group, 1 allow policy) plus the receiving peer's WG public key. Sufficient
|
// group, 1 allow policy) plus the receiving peer's WG public key. Sufficient
|
||||||
// to validate the encode → marshal → decode → Calculate pipeline produces
|
// to validate the encode → marshal → decode → Calculate pipeline produces
|
||||||
|
|||||||
Reference in New Issue
Block a user