Compare commits

..

1 Commits

Author SHA1 Message Date
pascal
fdc13d9fe7 fix handling of empty network map during decode and encode 2026-07-30 19:57:00 +02:00
9 changed files with 118 additions and 48 deletions

View File

@@ -113,14 +113,11 @@ func (c *ConnectClient) RunOnAndroid(
stateFilePath string,
cacheDir string,
) error {
notifier := tunnelnotifier.New(networkChangeListener, nil)
defer notifier.Close()
// in case of non Android os these variables will be nil
mobileDependency := MobileDependency{
TunAdapter: tunAdapter,
IFaceDiscover: iFaceDiscover,
NetworkChangeListener: notifier,
NetworkChangeListener: networkChangeListener,
HostDNSAddresses: dnsAddresses,
DnsReadyListener: dnsReadyListener,
StateFilePath: stateFilePath,

View File

@@ -51,5 +51,7 @@ func (n *notifier) notify() {
return
}
n.listener.OnNetworkChanged("")
go func(l listener.NetworkChangeListener) {
l.OnNetworkChanged("")
}(n.listener)
}

View File

@@ -479,7 +479,7 @@ func (d *DnsInterceptor) removeDNATMappings(realPrefixes []netip.Prefix, logger
// internalDnatFw checks if the firewall supports internal DNAT
func (d *DnsInterceptor) internalDnatFw() (internalDNATer, bool) {
if d.firewall == nil || d.fakeIPManager == nil || runtime.GOOS != "android" {
if d.firewall == nil || runtime.GOOS != "android" {
return nil, false
}
fw, ok := d.firewall.(internalDNATer)

View File

@@ -165,36 +165,31 @@ func (m *DefaultManager) setupAndroidRoutes(config ManagerConfig) {
routesForComparison := slices.Clone(cr)
if config.DNSFeatureFlag {
cr = append(cr, m.enableFakeIPRoutes()...)
m.fakeIPManager = fakeip.NewManager()
v4ID := uuid.NewString()
fakeIPRoute := &route.Route{
ID: route.ID(v4ID),
Network: m.fakeIPManager.GetFakeIPBlock(),
NetID: route.NetID(v4ID),
Peer: m.pubKey,
NetworkType: route.IPv4Network,
}
v6ID := uuid.NewString()
fakeIPv6Route := &route.Route{
ID: route.ID(v6ID),
Network: m.fakeIPManager.GetFakeIPv6Block(),
NetID: route.NetID(v6ID),
Peer: m.pubKey,
NetworkType: route.IPv6Network,
}
cr = append(cr, fakeIPRoute, fakeIPv6Route)
m.notifier.SetFakeIPRoutes([]*route.Route{fakeIPRoute, fakeIPv6Route})
}
m.notifier.SetInitialClientRoutes(cr, routesForComparison)
}
func (m *DefaultManager) enableFakeIPRoutes() []*route.Route {
m.fakeIPManager = fakeip.NewManager()
v4ID := uuid.NewString()
fakeIPRoute := &route.Route{
ID: route.ID(v4ID),
Network: m.fakeIPManager.GetFakeIPBlock(),
NetID: route.NetID(v4ID),
Peer: m.pubKey,
NetworkType: route.IPv4Network,
}
v6ID := uuid.NewString()
fakeIPv6Route := &route.Route{
ID: route.ID(v6ID),
Network: m.fakeIPManager.GetFakeIPv6Block(),
NetID: route.NetID(v6ID),
Peer: m.pubKey,
NetworkType: route.IPv6Network,
}
fakeRoutes := []*route.Route{fakeIPRoute, fakeIPv6Route}
m.notifier.SetFakeIPRoutes(fakeRoutes)
return fakeRoutes
}
func (m *DefaultManager) setupRefCounters(useNoop bool) {
var once sync.Once
var wgIface *net.Interface
@@ -469,9 +464,6 @@ func (m *DefaultManager) UpdateRoutes(
var merr *multierror.Error
if !m.disableClientRoutes {
if runtime.GOOS == "android" && useNewDNSRoute && m.fakeIPManager == nil {
m.enableFakeIPRoutes()
}
// Update route selector based on management server's isSelected status
m.updateRouteSelectorFromManagement(clientRoutes)

View File

@@ -41,7 +41,6 @@ func (n *Notifier) SetInitialClientRoutes(initialRoutes []*route.Route, routesFo
// SetFakeIPRoutes stores the fake IP routes to be included in every TUN rebuild.
func (n *Notifier) SetFakeIPRoutes(routes []*route.Route) {
n.fakeIPRoutes = routes
n.notify()
}
func (n *Notifier) OnNewRoutes(idMap route.HAMap) {
@@ -79,7 +78,9 @@ func (n *Notifier) notify() {
routeStrings := n.routesToStrings(allRoutes)
sort.Strings(routeStrings)
n.listener.OnNetworkChanged(strings.Join(routeStrings, ","))
go func(l listener.NetworkChangeListener) {
l.OnNetworkChanged(strings.Join(routeStrings, ","))
}(n.listener)
}
func filterStatic(routes []*route.Route) []*route.Route {
@@ -101,11 +102,16 @@ func (n *Notifier) routesToStrings(routes []*route.Route) []string {
}
func (n *Notifier) hasRouteDiff(a []*route.Route, b []*route.Route) bool {
as := n.routesToStrings(a)
bs := n.routesToStrings(b)
sort.Strings(as)
sort.Strings(bs)
return !slices.Equal(as, bs)
slices.SortFunc(a, func(x, y *route.Route) int {
return strings.Compare(x.NetString(), y.NetString())
})
slices.SortFunc(b, func(x, y *route.Route) int {
return strings.Compare(x.NetString(), y.NetString())
})
return !slices.EqualFunc(a, b, func(x, y *route.Route) bool {
return x.NetString() == y.NetString()
})
}
func (n *Notifier) GetInitialRouteRanges() []string {

View File

@@ -61,6 +61,8 @@ func EncodeNetworkMapEnvelope(in ComponentsEnvelopeInput) *proto.NetworkMapEnvel
return &proto.NetworkMapEnvelope{
Payload: &proto.NetworkMapEnvelope_Full{
Full: &proto.NetworkMapComponentsFull{
Serial: networkSerial(c.Network),
Network: toAccountNetwork(c.Network),
PeerConfig: in.PeerConfig,
// components.Peers always contains the target peer
Peers: []*proto.PeerCompact{toPeerCompact(c.Peers[c.PeerID])},

View File

@@ -758,6 +758,9 @@ func TestEncodeNetworkMapEnvelope_NilComponentsGracefulDegrade(t *testing.T) {
assert.Equal(t, "netbird.cloud", full.DnsDomain)
assert.Len(t, full.Peers, 1)
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) {
@@ -776,6 +779,12 @@ func TestEncodeNetworkMapEnvelope_AccountSettingsAlwaysEmitted(t *testing.T) {
func emptyNetworkMapComponents() *types.NetworkMapComponents {
return types.EmptyNetworkMapComponents(
&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,
},
},
)
}

View File

@@ -228,15 +228,17 @@ func DecodeEnvelope(env *proto.NetworkMapEnvelope) (*types.NetworkMapComponents,
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 {
n := &types.Network{}
if an == nil {
return nil
}
n := &types.Network{
Identifier: an.Identifier,
Dns: an.Dns,
Serial: an.Serial,
return n
}
n.Identifier = an.Identifier
n.Dns = an.Dns
n.Serial = an.Serial
if an.NetCidr != "" {
if _, ipnet, err := net.ParseCIDR(an.NetCidr); err == nil && ipnet != nil {
n.Net = *ipnet

View File

@@ -221,6 +221,66 @@ func TestEnvelopeRoundTrip_AllGroupShortCircuitParity(t *testing.T) {
"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
// group, 1 allow policy) plus the receiving peer's WG public key. Sufficient
// to validate the encode → marshal → decode → Calculate pipeline produces