diff --git a/allow_list.go b/allow_list.go index 90e0de2..c574b6e 100644 --- a/allow_list.go +++ b/allow_list.go @@ -128,7 +128,6 @@ func newAllowList(k string, raw interface{}, handleKey func(key string, value in ipNet = netip.PrefixFrom(ipNet.Addr().Unmap(), ipNet.Bits()) - // TODO: should we error on duplicate CIDRs in the config? tree.Insert(ipNet, value) maskBits := ipNet.Bits() diff --git a/cmd/nebula-cert/ca_test.go b/cmd/nebula-cert/ca_test.go index d320443..9da0ad4 100644 --- a/cmd/nebula-cert/ca_test.go +++ b/cmd/nebula-cert/ca_test.go @@ -16,8 +16,6 @@ import ( "github.com/stretchr/testify/assert" ) -//TODO: test file permissions - func Test_caSummary(t *testing.T) { assert.Equal(t, "ca : create a self signed certificate authority", caSummary()) } diff --git a/cmd/nebula-cert/keygen_test.go b/cmd/nebula-cert/keygen_test.go index 18ceb4b..fcfd77b 100644 --- a/cmd/nebula-cert/keygen_test.go +++ b/cmd/nebula-cert/keygen_test.go @@ -9,8 +9,6 @@ import ( "github.com/stretchr/testify/assert" ) -//TODO: test file permissions - func Test_keygenSummary(t *testing.T) { assert.Equal(t, "keygen : create a public/private key pair. the public key can be passed to `nebula-cert sign`", keygenSummary()) } diff --git a/cmd/nebula-cert/main_test.go b/cmd/nebula-cert/main_test.go index 2502824..f332895 100644 --- a/cmd/nebula-cert/main_test.go +++ b/cmd/nebula-cert/main_test.go @@ -11,8 +11,6 @@ import ( "github.com/stretchr/testify/assert" ) -//TODO: all flag parsing continueOnError will print to stderr on its own currently - func Test_help(t *testing.T) { expected := "Usage of " + os.Args[0] + " :\n" + " Global flags:\n" + diff --git a/cmd/nebula-cert/sign_test.go b/cmd/nebula-cert/sign_test.go index b4fdc43..466cb8c 100644 --- a/cmd/nebula-cert/sign_test.go +++ b/cmd/nebula-cert/sign_test.go @@ -16,8 +16,6 @@ import ( "golang.org/x/crypto/ed25519" ) -//TODO: test file permissions - func Test_signSummary(t *testing.T) { assert.Equal(t, "sign : create and sign a certificate", signSummary()) } diff --git a/config/config_test.go b/config/config_test.go index fa94393..c3a1a73 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -38,9 +38,6 @@ func TestConfig_Load(t *testing.T) { "new": "hi", } assert.Equal(t, expected, c.Settings) - - //TODO: test symlinked file - //TODO: test symlinked directory } func TestConfig_Get(t *testing.T) { diff --git a/control.go b/control.go index 5302e56..80a095e 100644 --- a/control.go +++ b/control.go @@ -227,7 +227,6 @@ func (c *Control) CloseTunnel(vpnIp netip.Addr, localOnly bool) bool { // CloseAllTunnels is just like CloseTunnel except it goes through and shuts them all down, optionally you can avoid shutting down lighthouse tunnels // the int returned is a count of tunnels closed func (c *Control) CloseAllTunnels(excludeLighthouses bool) (closed int) { - //TODO: this is probably better as a function in ConnectionManager or HostMap directly shutdown := func(h *HostInfo) { if excludeLighthouses && c.f.lightHouse.IsAnyLighthouseAddr(h.vpnAddrs) { return diff --git a/e2e/handshakes_test.go b/e2e/handshakes_test.go index aab5445..2e7e6e4 100644 --- a/e2e/handshakes_test.go +++ b/e2e/handshakes_test.go @@ -94,7 +94,6 @@ func TestGoodHandshake(t *testing.T) { r.RenderHostmaps("Final hostmaps", myControl, theirControl) myControl.Stop() theirControl.Stop() - //TODO: assert hostmaps } func TestWrongResponderHandshake(t *testing.T) { @@ -152,8 +151,6 @@ func TestWrongResponderHandshake(t *testing.T) { return router.KeepRouting }) - //TODO: Assert pending hostmap - I should have a correct hostinfo for them now - t.Log("My cached packet should be received by them") myCachedPacket := theirControl.GetFromTun(true) assertUdpPacket(t, []byte("Hi from me"), myCachedPacket, myVpnIpNet[0].Addr(), theirVpnIpNet[0].Addr(), 80, 80) @@ -169,7 +166,6 @@ func TestWrongResponderHandshake(t *testing.T) { assert.Nil(t, myControl.GetHostInfoByVpnAddr(evilVpnIp[0].Addr(), true), "My pending hostmap should not contain evil") assert.Nil(t, myControl.GetHostInfoByVpnAddr(evilVpnIp[0].Addr(), false), "My main hostmap should not contain evil") - //TODO: assert hostmaps for everyone r.RenderHostmaps("Final hostmaps", myControl, theirControl, evilControl) t.Log("Success!") myControl.Stop() @@ -236,8 +232,6 @@ func TestWrongResponderHandshakeStaticHostMap(t *testing.T) { return router.KeepRouting }) - //TODO: Assert pending hostmap - I should have a correct hostinfo for them now - t.Log("My cached packet should be received by them") myCachedPacket := theirControl.GetFromTun(true) assertUdpPacket(t, []byte("Hi from me"), myCachedPacket, myVpnIpNet[0].Addr(), theirVpnIpNet[0].Addr(), 80, 80) @@ -254,7 +248,6 @@ func TestWrongResponderHandshakeStaticHostMap(t *testing.T) { assert.Nil(t, myControl.GetHostInfoByVpnAddr(evilVpnIp[0].Addr(), false), "My main hostmap should not contain evil") //NOTE: if evil lost the handshake race it may still have a tunnel since me would reject the handshake since the tunnel is complete - //TODO: assert hostmaps for everyone r.RenderHostmaps("Final hostmaps", myControl, theirControl, evilControl) t.Log("Success!") myControl.Stop() @@ -468,7 +461,6 @@ func TestRelays(t *testing.T) { r.Log("Assert the tunnel works") assertUdpPacket(t, []byte("Hi from me"), p, myVpnIpNet[0].Addr(), theirVpnIpNet[0].Addr(), 80, 80) r.RenderHostmaps("Final hostmaps", myControl, relayControl, theirControl) - //TODO: assert we actually used the relay even though it should be impossible for a tunnel to have occurred without it } func TestReestablishRelays(t *testing.T) { @@ -647,8 +639,6 @@ func TestStage1RaceRelays(t *testing.T) { myControl.Stop() theirControl.Stop() relayControl.Stop() - // - ////TODO: assert hostmaps } func TestStage1RaceRelays2(t *testing.T) { @@ -735,9 +725,6 @@ func TestStage1RaceRelays2(t *testing.T) { myControl.Stop() theirControl.Stop() relayControl.Stop() - - // - ////TODO: assert hostmaps } func TestRehandshakingRelays(t *testing.T) { @@ -1173,7 +1160,6 @@ func TestRaceRegression(t *testing.T) { myControl.InjectUDPPacket(theirStage1ForMe) theirControl.InjectUDPPacket(myStage1ForThem) - //TODO: ensure stage 2 t.Log("Get both stage 2") myStage2ForThem := myControl.GetFromUDP(true) theirStage2ForMe := theirControl.GetFromUDP(true) @@ -1237,9 +1223,3 @@ func TestV2NonPrimaryWithLighthouse(t *testing.T) { myControl.Stop() theirControl.Stop() } - -//TODO: test -// Race winner renews and handshakes -// Race loser renews and handshakes -// Does race winner repin the cert to old? -//TODO: add a test with many lies diff --git a/e2e/helpers_test.go b/e2e/helpers_test.go index f5df2fb..e71c42b 100644 --- a/e2e/helpers_test.go +++ b/e2e/helpers_test.go @@ -178,22 +178,6 @@ func assertHostInfoPair(t *testing.T, addrA, addrB netip.AddrPort, vpnNetsA, vpn // Check that our indexes match assert.Equal(t, hBinA.LocalIndex, hAinB.RemoteIndex, "Host B local index does not match host A remote index") assert.Equal(t, hBinA.RemoteIndex, hAinB.LocalIndex, "Host B remote index does not match host A local index") - - //TODO: Would be nice to assert this memory - //checkIndexes := func(name string, hm *HostMap, hi *HostInfo) { - // hBbyIndex := hmA.Indexes[hBinA.localIndexId] - // assert.NotNil(t, hBbyIndex, "Could not host info by local index in %s", name) - // assert.Equal(t, &hBbyIndex, &hBinA, "%s Indexes map did not point to the right host info", name) - // - // //TODO: remote indexes are susceptible to collision - // hBbyRemoteIndex := hmA.RemoteIndexes[hBinA.remoteIndexId] - // assert.NotNil(t, hBbyIndex, "Could not host info by remote index in %s", name) - // assert.Equal(t, &hBbyRemoteIndex, &hBinA, "%s RemoteIndexes did not point to the right host info", name) - //} - // - //// Check hostmap indexes too - //checkIndexes("hmA", hmA, hBinA) - //checkIndexes("hmB", hmB, hAinB) } func assertUdpPacket(t *testing.T, expected, b []byte, fromIp, toIp netip.Addr, fromPort, toPort uint16) { diff --git a/examples/config.yml b/examples/config.yml index 1a31283..1c3584e 100644 --- a/examples/config.yml +++ b/examples/config.yml @@ -250,7 +250,6 @@ tun: # in nebula configuration files. Default false, not reloadable. #use_system_route_table: false -# TODO # Configure logging level logging: # panic, fatal, error, warning, info, or debug. Default is info and is reloadable. @@ -342,9 +341,8 @@ firewall: # host: `any` or a literal hostname, ie `test-host` # group: `any` or a literal group name, ie `default-group` # groups: Same as group but accepts a list of values. Multiple values are AND'd together and a certificate would have to contain all groups to pass - # cidr: a remote CIDR, `0.0.0.0/0` is any ipv4 and `::/0` is any ipv6. //TODO: we have a problem, firewall needs to understand this and should probably allow `any` for both + # cidr: a remote CIDR, `0.0.0.0/0` is any ipv4 and `::/0` is any ipv6. # local_cidr: a local CIDR, `0.0.0.0/0` is any ipv4 and `::/0` is any ipv6. This could be used to filter destinations when using unsafe_routes. - # //TODO: probably should have an `any` that covers both ip versions # If no unsafe networks are present in the certificate(s) or `default_local_cidr_any` is true then the default is any ipv4 or ipv6 network. # Otherwise the default is any vpn network assigned to via the certificate. # `default_local_cidr_any` defaults to false and is deprecated, it will be removed in a future release. diff --git a/firewall_test.go b/firewall_test.go index c093a6e..a0d08ac 100644 --- a/firewall_test.go +++ b/firewall_test.go @@ -575,8 +575,6 @@ func BenchmarkLookup(b *testing.B) { ml(m, a) } }) - - //TODO: only way array lookup in array will help is if both are sorted, then maybe it's faster } func Test_parsePort(t *testing.T) { diff --git a/handshake_ix.go b/handshake_ix.go index 43c7946..c77145e 100644 --- a/handshake_ix.go +++ b/handshake_ix.go @@ -533,7 +533,6 @@ func ixHandshakeStage2(f *Interface, addr netip.AddrPort, via *ViaSender, hh *Ha // Create a new hostinfo/handshake for the intended vpn ip f.handshakeManager.StartHandshake(hostinfo.vpnAddrs[0], func(newHH *HandshakeHostInfo) { - //TODO: this doesnt know if its being added or is being used for caching a packet // Block the current used address newHH.hostinfo.remotes = hostinfo.remotes newHH.hostinfo.remotes.BlockRemote(addr) diff --git a/handshake_manager.go b/handshake_manager.go index 0e5c4fa..85ed173 100644 --- a/handshake_manager.go +++ b/handshake_manager.go @@ -224,7 +224,7 @@ func (hm *HandshakeManager) handleOutbound(vpnIp netip.Addr, lighthouseTriggered hh.lastRemotes = remotes - // TODO: this will generate a load of queries for hosts with only 1 ip + // This will generate a load of queries for hosts with only 1 ip // (such as ones registered to the lighthouse with only a private IP) // So we only do it one time after attempting 5 handshakes already. if len(remotes) <= 1 && hh.counter == 5 { diff --git a/hostmap.go b/hostmap.go index 99aeb53..2869c6b 100644 --- a/hostmap.go +++ b/hostmap.go @@ -780,8 +780,6 @@ func localAddrs(l *logrus.Logger, allowList *LocalAllowList) []netip.Addr { } addr = addr.Unmap() - //TODO: Filtering out link local for now, this is probably the most correct thing - //TODO: Would be nice to filter out SLAAC MAC based ips as well if addr.IsLoopback() == false && addr.IsLinkLocalUnicast() == false { isAllowed := allowList.Allow(addr) if l.Level >= logrus.TraceLevel { diff --git a/inside.go b/inside.go index 9119560..9629947 100644 --- a/inside.go +++ b/inside.go @@ -27,7 +27,6 @@ func (f *Interface) consumeInsidePacket(packet []byte, fwPacket *firewall.Packet } } - //TODO: seems like a huge bummer _, found := f.myVpnAddrsTable.Lookup(fwPacket.RemoteAddr) if found { // Immediately forward packets from self to self. @@ -264,7 +263,6 @@ func (f *Interface) SendVia(via *HostInfo, func (f *Interface) sendNoMetrics(t header.MessageType, st header.MessageSubType, ci *ConnectionState, hostinfo *HostInfo, remote netip.AddrPort, p, nb, out []byte, q int) { if ci.eKey == nil { - //TODO: log warning return } useRelay := !remote.IsValid() && !hostinfo.remote.IsValid() diff --git a/interface.go b/interface.go index 9b93156..19a6864 100644 --- a/interface.go +++ b/interface.go @@ -410,6 +410,8 @@ func (f *Interface) emitStats(ctx context.Context, i time.Duration) { udpStats := udp.NewUDPStatsEmitter(f.writers) certExpirationGauge := metrics.GetOrRegisterGauge("certificate.ttl_seconds", nil) + certDefaultVersion := metrics.GetOrRegisterGauge("certificate.default_version", nil) + certMaxVersion := metrics.GetOrRegisterGauge("certificate.max_version", nil) for { select { @@ -419,8 +421,18 @@ func (f *Interface) emitStats(ctx context.Context, i time.Duration) { f.firewall.EmitStats() f.handshakeManager.EmitStats() udpStats() - certExpirationGauge.Update(int64(f.pki.getCertState().GetDefaultCertificate().NotAfter().Sub(time.Now()) / time.Second)) - //TODO: we should also report the default certificate version + + certState := f.pki.getCertState() + defaultCrt := certState.GetDefaultCertificate() + certExpirationGauge.Update(int64(defaultCrt.NotAfter().Sub(time.Now()) / time.Second)) + certDefaultVersion.Update(int64(defaultCrt.Version())) + + // Report the max certificate version we are capable of using + if certState.v2Cert != nil { + certMaxVersion.Update(int64(certState.v2Cert.Version())) + } else { + certMaxVersion.Update(int64(certState.v1Cert.Version())) + } } } } diff --git a/lighthouse.go b/lighthouse.go index 59e8941..5c3b92f 100644 --- a/lighthouse.go +++ b/lighthouse.go @@ -241,7 +241,6 @@ func (lh *LightHouse) reload(c *config.C, initial bool) error { lh.remoteAllowList.Store(ral) if !initial { - //TODO: a diff will be annoyingly difficult lh.l.Info("lighthouse.remote_allow_list and/or lighthouse.remote_allow_ranges has changed") } } @@ -254,7 +253,6 @@ func (lh *LightHouse) reload(c *config.C, initial bool) error { lh.localAllowList.Store(lal) if !initial { - //TODO: a diff will be annoyingly difficult lh.l.Info("lighthouse.local_allow_list has changed") } } @@ -267,7 +265,6 @@ func (lh *LightHouse) reload(c *config.C, initial bool) error { lh.calculatedRemotes.Store(cr) if !initial { - //TODO: a diff will be annoyingly difficult lh.l.Info("lighthouse.calculated_remotes has changed") } } @@ -294,7 +291,6 @@ func (lh *LightHouse) reload(c *config.C, initial bool) error { lh.staticList.Store(&staticList) if !initial { - //TODO: we should remove any remote list entries for static hosts that were removed/modified? if c.HasChanged("static_host_map") { lh.l.Info("static_host_map has changed") } @@ -1007,7 +1003,6 @@ func (lhh *LightHouseHandler) resetMeta() *NebulaMeta { details.V6AddrPorts = details.V6AddrPorts[:0] details.RelayVpnAddrs = details.RelayVpnAddrs[:0] details.OldRelayVpnAddrs = details.OldRelayVpnAddrs[:0] - //TODO: these are unfortunate details.OldVpnAddr = 0 details.VpnAddr = nil lhh.meta.Details = details @@ -1077,7 +1072,6 @@ func (lhh *LightHouseHandler) handleHostQuery(n *NebulaMeta, fromVpnAddrs []neti return } - //TODO: Maybe instead of marshalling into n we marshal into a new `r` to not nuke our current request data found, ln, err := lhh.lh.queryAndPrepMessage(queryVpnAddr, func(c *cache) (int, error) { n = lhh.resetMeta() n.Type = NebulaMeta_HostQueryReply diff --git a/lighthouse_test.go b/lighthouse_test.go index 8eeb8ce..611d9f6 100644 --- a/lighthouse_test.go +++ b/lighthouse_test.go @@ -16,8 +16,6 @@ import ( "gopkg.in/yaml.v2" ) -//TODO: Add a test to ensure udpAddr is copied and not reused - func TestOldIPv4Only(t *testing.T) { // This test ensures our new ipv6 enabled LH protobuf IpAndPorts works with the old style to enable backwards compatibility b := []byte{8, 129, 130, 132, 80, 16, 10} diff --git a/main.go b/main.go index 1e77a47..7e94c32 100644 --- a/main.go +++ b/main.go @@ -256,8 +256,6 @@ func Main(c *config.C, configTest bool, buildVersion string, logger *logrus.Logg return nil, fmt.Errorf("failed to initialize interface: %s", err) } - // TODO: Better way to attach these, probably want a new interface in InterfaceConfig - // I don't want to make this initial commit too far-reaching though ifce.writers = udpConns lightHouse.ifce = ifce @@ -269,8 +267,6 @@ func Main(c *config.C, configTest bool, buildVersion string, logger *logrus.Logg go handshakeManager.Run(ctx) } - // TODO - stats third-party modules start uncancellable goroutines. Update those libs to accept - // a context so that they can exit when the context is Done. statsStart, err := startStats(l, c, buildVersion, configTest) if err != nil { return nil, util.ContextualizeIfNeeded("Failed to start stats emitter", err) @@ -280,7 +276,6 @@ func Main(c *config.C, configTest bool, buildVersion string, logger *logrus.Logg return nil, nil } - //TODO: check if we _should_ be emitting stats go ifce.emitStats(ctx, c.GetDuration("stats.interval", time.Second*10)) attachCommands(l, c, ssh, ifce) diff --git a/message_metrics.go b/message_metrics.go index 94bb02f..10e8472 100644 --- a/message_metrics.go +++ b/message_metrics.go @@ -7,8 +7,6 @@ import ( "github.com/slackhq/nebula/header" ) -//TODO: this can probably move into the header package - type MessageMetrics struct { rx [][]metrics.Counter tx [][]metrics.Counter diff --git a/outside.go b/outside.go index d4f97b2..f011c87 100644 --- a/outside.go +++ b/outside.go @@ -23,8 +23,6 @@ const ( func (f *Interface) readOutsidePackets(ip netip.AddrPort, via *ViaSender, out []byte, packet []byte, h *header.H, fwPacket *firewall.Packet, lhf *LightHouseHandler, nb []byte, q int, localCache firewall.ConntrackCache) { err := h.Parse(packet) if err != nil { - // TODO: best if we return this and let caller log - // TODO: Might be better to send the literal []byte("holepunch") packet and ignore that? // Hole punch packets are 0 or 1 byte big, so lets ignore printing those errors if len(packet) > 1 { f.l.WithField("packet", packet).Infof("Error while parsing inbound packet from %s: %s", ip, err) @@ -139,9 +137,6 @@ func (f *Interface) readOutsidePackets(ip netip.AddrPort, via *ViaSender, out [] hostinfo.logger(f.l).WithError(err).WithField("udpAddr", ip). WithField("packet", packet). Error("Failed to decrypt lighthouse packet") - - //TODO: maybe after build 64 is out? 06/14/2018 - NB - //f.sendRecvError(net.Addr(addr), header.RemoteIndex) return } @@ -160,9 +155,6 @@ func (f *Interface) readOutsidePackets(ip netip.AddrPort, via *ViaSender, out [] hostinfo.logger(f.l).WithError(err).WithField("udpAddr", ip). WithField("packet", packet). Error("Failed to decrypt test packet") - - //TODO: maybe after build 64 is out? 06/14/2018 - NB - //f.sendRecvError(net.Addr(addr), header.RemoteIndex) return } @@ -322,7 +314,6 @@ func parseV6(data []byte, incoming bool, fp *firewall.Packet) error { //fmt.Println(proto, protoAt) switch proto { case layers.IPProtocolICMPv6: - //TODO: we need a new protocol in config language "icmpv6" fp.Protocol = uint8(proto) fp.RemotePort = 0 fp.LocalPort = 0 @@ -463,8 +454,6 @@ func (f *Interface) decryptToTun(hostinfo *HostInfo, messageCounter uint64, out out, err = hostinfo.ConnectionState.dKey.DecryptDanger(out, packet[:header.Len], packet[header.Len:], messageCounter, nb) if err != nil { hostinfo.logger(f.l).WithError(err).Error("Failed to decrypt packet") - //TODO: maybe after build 64 is out? 06/14/2018 - NB - //f.sendRecvError(hostinfo.remote, header.RemoteIndex) return false } @@ -511,7 +500,6 @@ func (f *Interface) maybeSendRecvError(endpoint netip.AddrPort, index uint32) { func (f *Interface) sendRecvError(endpoint netip.AddrPort, index uint32) { f.messageMetrics.Tx(header.RecvError, 0, 1) - //TODO: this should be a signed message so we can trust that we should drop the index b := header.Encode(make([]byte, header.Len), header.Version, header.RecvError, 0, index, 0) f.outside.WriteTo(b, endpoint) if f.l.Level >= logrus.DebugLevel { @@ -547,25 +535,3 @@ func (f *Interface) handleRecvError(addr netip.AddrPort, h *header.H) { // We also delete it from pending hostmap to allow for fast reconnect. f.handshakeManager.DeleteHostInfo(hostinfo) } - -/* -func (f *Interface) sendMeta(ci *ConnectionState, endpoint *net.UDPAddr, meta *NebulaMeta) { - if ci.eKey != nil { - //TODO: log error? - return - } - - msg, err := proto.Marshal(meta) - if err != nil { - l.Debugln("failed to encode header") - } - - c := ci.messageCounter - b := HeaderEncode(nil, Version, uint8(metadata), 0, hostinfo.remoteIndexId, c) - ci.messageCounter++ - - msg := ci.eKey.EncryptDanger(b, nil, msg, c) - //msg := ci.eKey.EncryptDanger(b, nil, []byte(fmt.Sprintf("%d", counter)), c) - f.outside.WriteTo(msg, endpoint) -} -*/ diff --git a/overlay/tun_darwin.go b/overlay/tun_darwin.go index 1cff214..09af173 100644 --- a/overlay/tun_darwin.go +++ b/overlay/tun_darwin.go @@ -351,7 +351,7 @@ func (t *tun) RouteFor(ip netip.Addr) netip.Addr { } // Get the LinkAddr for the interface of the given name -// TODO: Is there an easier way to fetch this when we create the interface? +// Is there an easier way to fetch this when we create the interface? // Maybe SIOCGIFINDEX? but this doesn't appear to exist in the darwin headers. func getLinkAddr(name string) (*netroute.LinkAddr, error) { rib, err := netroute.FetchRIB(unix.AF_UNSPEC, unix.NET_RT_IFLIST, 0) diff --git a/relay_manager.go b/relay_manager.go index 49e069a..7565350 100644 --- a/relay_manager.go +++ b/relay_manager.go @@ -92,11 +92,24 @@ func AddRelay(l *logrus.Logger, relayHostInfo *HostInfo, hm *HostMap, vpnIp neti func (rm *relayManager) EstablishRelay(relayHostInfo *HostInfo, m *NebulaControl) (*Relay, error) { relay, ok := relayHostInfo.relayState.CompleteRelayByIdx(m.InitiatorRelayIndex, m.ResponderRelayIndex) if !ok { - //TODO: we need to handle possibly logging deprecated fields as well - rm.l.WithFields(logrus.Fields{"relay": relayHostInfo.vpnAddrs[0], + fields := logrus.Fields{ + "relay": relayHostInfo.vpnAddrs[0], "initiatorRelayIndex": m.InitiatorRelayIndex, - "relayFrom": m.RelayFromAddr, - "relayTo": m.RelayToAddr}).Info("relayManager failed to update relay") + } + + if m.RelayFromAddr == nil { + fields["relayFrom"] = m.OldRelayFromAddr + } else { + fields["relayFrom"] = m.RelayFromAddr + } + + if m.RelayToAddr == nil { + fields["relayTo"] = m.OldRelayToAddr + } else { + fields["relayTo"] = m.RelayToAddr + } + + rm.l.WithFields(fields).Info("relayManager failed to update relay") return nil, fmt.Errorf("unknown relay") } @@ -115,7 +128,6 @@ func (rm *relayManager) HandleControlMsg(h *HostInfo, d []byte, f *Interface) { if msg.OldRelayFromAddr > 0 || msg.OldRelayToAddr > 0 { v = cert.Version1 - //TODO: yeah this is junk but maybe its less junky than the other options b := [4]byte{} binary.BigEndian.PutUint32(b[:], msg.OldRelayFromAddr) msg.RelayFromAddr = netAddrToProtoAddr(netip.AddrFrom4(b)) @@ -181,7 +193,12 @@ func (rm *relayManager) handleCreateRelayResponse(v cert.Version, h *HostInfo, f if v == cert.Version1 { peer := peerHostInfo.vpnAddrs[0] if !peer.Is4() { - //TODO: log cant do it + rm.l.WithField("relayFrom", peer). + WithField("relayTo", target). + WithField("initiatorRelayIndex", resp.InitiatorRelayIndex). + WithField("responderRelayIndex", resp.ResponderRelayIndex). + WithField("vpnAddrs", peerHostInfo.vpnAddrs). + Error("Refusing to CreateRelayResponse for a v1 relay with an ipv6 address") return } @@ -303,7 +320,6 @@ func (rm *relayManager) handleCreateRelayRequest(v cert.Version, h *HostInfo, f } else { f.SendMessageToHostInfo(header.Control, 0, h, msg, make([]byte, 12), make([]byte, mtu)) rm.l.WithFields(logrus.Fields{ - //TODO: IPV6-WORK, this used to use the resp object but I am getting lazy now "relayFrom": from, "relayTo": target, "initiatorRelayIndex": resp.InitiatorRelayIndex, @@ -349,7 +365,12 @@ func (rm *relayManager) handleCreateRelayRequest(v cert.Version, h *HostInfo, f if v == cert.Version1 { if !h.vpnAddrs[0].Is4() { - //TODO: log it + rm.l.WithField("relayFrom", h.vpnAddrs[0]). + WithField("relayTo", target). + WithField("initiatorRelayIndex", req.InitiatorRelayIndex). + WithField("responderRelayIndex", req.ResponderRelayIndex). + WithField("vpnAddr", target). + Error("Refusing to CreateRelayRequest for a v1 relay with an ipv6 address") return } @@ -369,7 +390,6 @@ func (rm *relayManager) handleCreateRelayRequest(v cert.Version, h *HostInfo, f } else { f.SendMessageToHostInfo(header.Control, 0, peer, msg, make([]byte, 12), make([]byte, mtu)) rm.l.WithFields(logrus.Fields{ - //TODO: IPV6-WORK another lazy used to use the req object "relayFrom": h.vpnAddrs[0], "relayTo": target, "initiatorRelayIndex": req.InitiatorRelayIndex, diff --git a/remote_list.go b/remote_list.go index f59dbf2..6baed29 100644 --- a/remote_list.go +++ b/remote_list.go @@ -33,9 +33,6 @@ type Cache struct { Relay []netip.Addr `json:"relay"` } -//TODO: Seems like we should plop static host entries in here too since the are protected by the lighthouse from deletion -// We will never clean learned/reported information for them as it stands today - // cache is an internal struct that splits v4 and v6 addresses inside the cache map type cache struct { v4 *cacheV4 @@ -275,7 +272,6 @@ func (r *RemoteList) CopyAddrs(preferredRanges []netip.Prefix) []netip.AddrPort // LearnRemote locks and sets the learned slot for the owner vpn ip to the provided addr // Currently this is only needed when HostInfo.SetRemote is called as that should cover both handshaking and roaming. // It will mark the deduplicated address list as dirty, so do not call it unless new information is available -// TODO: this needs to support the allow list list func (r *RemoteList) LearnRemote(ownerVpnIp netip.Addr, remote netip.AddrPort) { r.Lock() defer r.Unlock() @@ -386,7 +382,6 @@ func (r *RemoteList) Rebuild(preferredRanges []netip.Prefix) { defer r.Unlock() // Only rebuild if the cache changed - //TODO: shouldRebuild is probably pointless as we don't check for actual change when lighthouse updates come in if r.shouldRebuild { r.unlockedCollect() r.shouldRebuild = false @@ -709,7 +704,6 @@ func minInt(a, b int) int { // isPreferred returns true of the ip is contained in the preferredRanges list func isPreferred(ip netip.Addr, preferredRanges []netip.Prefix) bool { - //TODO: this would be better in a CIDR6Tree for _, p := range preferredRanges { if p.Contains(ip) { return true diff --git a/ssh.go b/ssh.go index 1062a77..203166c 100644 --- a/ssh.go +++ b/ssh.go @@ -77,9 +77,6 @@ func wireSSHReload(l *logrus.Logger, ssh *sshd.SSHServer, c *config.C) { // that callers may invoke to run the configured ssh server. On // failure, it returns nil, error. func configSSH(l *logrus.Logger, ssh *sshd.SSHServer, c *config.C) (func(), error) { - //TODO conntrack list - //TODO print firewall rules or hash? - listen := c.GetString("sshd.listen", "") if listen == "" { return nil, fmt.Errorf("sshd.listen must be provided") @@ -93,7 +90,6 @@ func configSSH(l *logrus.Logger, ssh *sshd.SSHServer, c *config.C) (func(), erro return nil, fmt.Errorf("sshd.listen can not use port 22") } - //TODO: no good way to reload this right now hostKeyPathOrKey := c.GetString("sshd.host_key", "") if hostKeyPathOrKey == "" { return nil, fmt.Errorf("sshd.host_key must be provided") @@ -418,7 +414,6 @@ func attachCommands(l *logrus.Logger, c *config.C, ssh *sshd.SSHServer, f *Inter func sshListHostMap(hl controlHostLister, a interface{}, w sshd.StringWriter) error { fs, ok := a.(*sshListHostMapFlags) if !ok { - //TODO: error return nil } @@ -441,7 +436,6 @@ func sshListHostMap(hl controlHostLister, a interface{}, w sshd.StringWriter) er err := js.Encode(hm) if err != nil { - //TODO return nil } @@ -460,7 +454,6 @@ func sshListHostMap(hl controlHostLister, a interface{}, w sshd.StringWriter) er func sshListLighthouseMap(lightHouse *LightHouse, a interface{}, w sshd.StringWriter) error { fs, ok := a.(*sshListHostMapFlags) if !ok { - //TODO: error return nil } @@ -493,7 +486,6 @@ func sshListLighthouseMap(lightHouse *LightHouse, a interface{}, w sshd.StringWr err := js.Encode(addrMap) if err != nil { - //TODO return nil } @@ -564,7 +556,6 @@ func sshQueryLighthouse(ifce *Interface, fs interface{}, a []string, w sshd.Stri func sshCloseTunnel(ifce *Interface, fs interface{}, a []string, w sshd.StringWriter) error { flags, ok := fs.(*sshCloseTunnelFlags) if !ok { - //TODO: error return nil } @@ -605,7 +596,6 @@ func sshCloseTunnel(ifce *Interface, fs interface{}, a []string, w sshd.StringWr func sshCreateTunnel(ifce *Interface, fs interface{}, a []string, w sshd.StringWriter) error { flags, ok := fs.(*sshCreateTunnelFlags) if !ok { - //TODO: error return nil } @@ -651,7 +641,6 @@ func sshCreateTunnel(ifce *Interface, fs interface{}, a []string, w sshd.StringW func sshChangeRemote(ifce *Interface, fs interface{}, a []string, w sshd.StringWriter) error { flags, ok := fs.(*sshChangeRemoteFlags) if !ok { - //TODO: error return nil } @@ -781,7 +770,6 @@ func sshLogFormat(l *logrus.Logger, fs interface{}, a []string, w sshd.StringWri func sshPrintCert(ifce *Interface, fs interface{}, a []string, w sshd.StringWriter) error { args, ok := fs.(*sshPrintCertFlags) if !ok { - //TODO: error return nil } @@ -807,7 +795,6 @@ func sshPrintCert(ifce *Interface, fs interface{}, a []string, w sshd.StringWrit if args.Json || args.Pretty { b, err := cert.MarshalJSON() if err != nil { - //TODO: handle it return nil } @@ -816,7 +803,6 @@ func sshPrintCert(ifce *Interface, fs interface{}, a []string, w sshd.StringWrit err := json.Indent(buf, b, "", " ") b = buf.Bytes() if err != nil { - //TODO: handle it return nil } } @@ -827,7 +813,6 @@ func sshPrintCert(ifce *Interface, fs interface{}, a []string, w sshd.StringWrit if args.Raw { b, err := cert.MarshalPEM() if err != nil { - //TODO: handle it return nil } @@ -840,7 +825,6 @@ func sshPrintCert(ifce *Interface, fs interface{}, a []string, w sshd.StringWrit func sshPrintRelays(ifce *Interface, fs interface{}, a []string, w sshd.StringWriter) error { args, ok := fs.(*sshPrintTunnelFlags) if !ok { - //TODO: error w.WriteLine(fmt.Sprintf("sshPrintRelays failed to convert args type")) return nil } @@ -938,7 +922,6 @@ func sshPrintRelays(ifce *Interface, fs interface{}, a []string, w sshd.StringWr func sshPrintTunnel(ifce *Interface, fs interface{}, a []string, w sshd.StringWriter) error { args, ok := fs.(*sshPrintTunnelFlags) if !ok { - //TODO: error return nil } diff --git a/sshd/command.go b/sshd/command.go index 900b01e..66646a6 100644 --- a/sshd/command.go +++ b/sshd/command.go @@ -57,7 +57,6 @@ func execCommand(c *Command, args []string, w StringWriter) error { func dumpCommands(c *radix.Tree, w StringWriter) { err := w.WriteLine("Available commands:") if err != nil { - //TODO: log return } @@ -67,10 +66,7 @@ func dumpCommands(c *radix.Tree, w StringWriter) { } sort.Strings(cmds) - err = w.Write(strings.Join(cmds, "\n") + "\n\n") - if err != nil { - //TODO: log - } + _ = w.Write(strings.Join(cmds, "\n") + "\n\n") } func lookupCommand(c *radix.Tree, sCmd string) (*Command, error) { @@ -119,8 +115,6 @@ func helpCallback(commands *radix.Tree, a []string, w StringWriter) (err error) // We are printing a specific commands help text cmd, err := lookupCommand(commands, a[0]) if err != nil { - //TODO: handle error - //TODO: message the user return } diff --git a/sshd/server.go b/sshd/server.go index 9e8c721..c151f91 100644 --- a/sshd/server.go +++ b/sshd/server.go @@ -80,9 +80,7 @@ func NewSSHServer(l *logrus.Entry) (*SSHServer, error) { s.config = &ssh.ServerConfig{ PublicKeyCallback: cc.Authenticate, - //TODO: AuthLogCallback: s.authAttempt, - //TODO: version string - ServerVersion: fmt.Sprintf("SSH-2.0-Nebula???"), + ServerVersion: fmt.Sprintf("SSH-2.0-Nebula???"), } s.RegisterCommand(&Command{ diff --git a/sshd/session.go b/sshd/session.go index bba2a55..7c5869e 100644 --- a/sshd/session.go +++ b/sshd/session.go @@ -62,7 +62,6 @@ func (s *session) handleChannels(chans <-chan ssh.NewChannel) { func (s *session) handleRequests(in <-chan *ssh.Request, channel ssh.Channel) { for req := range in { var err error - //TODO: maybe support window sizing? switch req.Type { case "shell": if s.term == nil { @@ -89,9 +88,7 @@ func (s *session) handleRequests(in <-chan *ssh.Request, channel ssh.Channel) { req.Reply(true, nil) s.dispatchCommand(payload.Value, &stringWriter{channel}) - //TODO: Fix error handling and report the proper status back status := struct{ Status uint32 }{uint32(0)} - //TODO: I think this is how we shut down a shell as well? channel.SendRequest("exit-status", false, ssh.Marshal(status)) channel.Close() return @@ -110,7 +107,6 @@ func (s *session) handleRequests(in <-chan *ssh.Request, channel ssh.Channel) { } func (s *session) createTerm(channel ssh.Channel) *terminal.Terminal { - //TODO: PS1 with nebula cert name term := terminal.NewTerminal(channel, s.c.User()+"@nebula > ") term.AutoCompleteCallback = func(line string, pos int, key rune) (newLine string, newPos int, ok bool) { // key 9 is tab @@ -137,7 +133,6 @@ func (s *session) handleInput(channel ssh.Channel) { for { line, err := s.term.ReadLine() if err != nil { - //TODO: log break } @@ -148,7 +143,6 @@ func (s *session) handleInput(channel ssh.Channel) { func (s *session) dispatchCommand(line string, w StringWriter) { args, err := shlex.Split(line, true) if err != nil { - //todo: LOG IT return } @@ -159,13 +153,11 @@ func (s *session) dispatchCommand(line string, w StringWriter) { c, err := lookupCommand(s.commands, args[0]) if err != nil { - //TODO: handle the error return } if c == nil { err := w.WriteLine(fmt.Sprintf("did not understand: %s", line)) - //TODO: log error _ = err dumpCommands(s.commands, w) @@ -177,10 +169,7 @@ func (s *session) dispatchCommand(line string, w StringWriter) { return } - err = execCommand(c, args[1:], w) - if err != nil { - //TODO: log the error - } + _ = execCommand(c, args[1:], w) return } diff --git a/udp/udp_generic.go b/udp/udp_generic.go index 99a3eca..06a4d53 100644 --- a/udp/udp_generic.go +++ b/udp/udp_generic.go @@ -58,7 +58,7 @@ func (u *GenericConn) LocalAddr() (netip.AddrPort, error) { } func (u *GenericConn) ReloadConfig(c *config.C) { - // TODO + } func NewUDPStatsEmitter(udpConns []Conn) func() { diff --git a/udp/udp_linux.go b/udp/udp_linux.go index 36ab67c..32a567e 100644 --- a/udp/udp_linux.go +++ b/udp/udp_linux.go @@ -17,8 +17,6 @@ import ( "golang.org/x/sys/unix" ) -//TODO: make it support reload as best you can! - type StdConn struct { sysFd int isV4 bool @@ -57,7 +55,6 @@ func NewListener(l *logrus.Logger, ip netip.Addr, port int, multi bool, batch in } } - //TODO: support multiple listening IPs (for limiting ipv6) var sa unix.Sockaddr if ip.Is4() { sa4 := &unix.SockaddrInet4{Port: port} @@ -72,11 +69,6 @@ func NewListener(l *logrus.Logger, ip netip.Addr, port int, multi bool, batch in return nil, fmt.Errorf("unable to bind to socket: %s", err) } - //TODO: this may be useful for forcing threads into specific cores - //unix.SetsockoptInt(fd, unix.SOL_SOCKET, unix.SO_INCOMING_CPU, x) - //v, err := unix.GetsockoptInt(fd, unix.SOL_SOCKET, unix.SO_INCOMING_CPU) - //l.Println(v, err) - return &StdConn{sysFd: fd, isV4: ip.Is4(), l: l, batch: batch}, err } @@ -215,8 +207,6 @@ func (u *StdConn) writeTo6(b []byte, ip netip.AddrPort) error { return &net.OpError{Op: "sendto", Err: err} } - //TODO: handle incomplete writes - return nil } } @@ -246,8 +236,6 @@ func (u *StdConn) writeTo4(b []byte, ip netip.AddrPort) error { return &net.OpError{Op: "sendto", Err: err} } - //TODO: handle incomplete writes - return nil } } @@ -294,7 +282,6 @@ func (u *StdConn) getMemInfo(meminfo *[unix.SK_MEMINFO_VARS]uint32) error { } func (u *StdConn) Close() error { - //TODO: this will not interrupt the read loop return syscall.Close(u.sysFd) } diff --git a/udp/udp_linux_32.go b/udp/udp_linux_32.go index 523968c..de8f1cd 100644 --- a/udp/udp_linux_32.go +++ b/udp/udp_linux_32.go @@ -39,7 +39,6 @@ func (u *StdConn) PrepareRawMessages(n int) ([]rawMessage, [][]byte, [][]byte) { buffers[i] = make([]byte, MTU) names[i] = make([]byte, unix.SizeofSockaddrInet6) - //TODO: this is still silly, no need for an array vs := []iovec{ {Base: &buffers[i][0], Len: uint32(len(buffers[i]))}, } diff --git a/udp/udp_linux_64.go b/udp/udp_linux_64.go index 87a0de7..48c5a97 100644 --- a/udp/udp_linux_64.go +++ b/udp/udp_linux_64.go @@ -42,7 +42,6 @@ func (u *StdConn) PrepareRawMessages(n int) ([]rawMessage, [][]byte, [][]byte) { buffers[i] = make([]byte, MTU) names[i] = make([]byte, unix.SizeofSockaddrInet6) - //TODO: this is still silly, no need for an array vs := []iovec{ {Base: &buffers[i][0], Len: uint64(len(buffers[i]))}, }