Merge pull request #182 from safing/triage/hub-missing-transports

Triage missing hub transports and other weird things

captain/client.go

@@ -61,6 +61,7 @@ var (
 	clientHealthCheckTimeout               = 15 * time.Second
 
 	clientHealthCheckTrigger = make(chan struct{}, 1)
+	lastHealthCheck          time.Time
 )
 
 func triggerClientHealthCheck() {
@@ -145,6 +146,9 @@ reconnect:
 		module.TriggerEvent(SPNConnectedEvent, nil)
 		module.StartWorker("update quick setting countries", navigator.Main.UpdateConfigQuickSettings)
 
+		// Reset last health check value, as we have just connected.
+		lastHealthCheck = time.Now()
+
 		// Back off before starting initial health checks.
 		select {
 		case <-time.After(clientInitialHealthCheckDelay):
@@ -456,11 +460,20 @@ func clientCheckHomeHubConnection(ctx context.Context) clientComponentResult {
 
 		// Reset all failing states, as these might have been caused by the failing home hub.
 		navigator.Main.ResetFailingStates(ctx)
+
+		// If the last health check is clearly too long ago, assume that the device was sleeping and do not set the home node to failing yet.
+		if time.Since(lastHealthCheck) > clientHealthCheckTickDuration+
+			clientHealthCheckTickDurationSleepMode+
+			(clientHealthCheckTimeout*2) {
+			return clientResultReconnect
+		}
+
 		// Mark the home hub itself as failing, as we want to try to connect to somewhere else.
 		home.MarkAsFailingFor(5 * time.Minute)
 
 		return clientResultReconnect
 	}
+	lastHealthCheck = time.Now()
 
 	log.Debugf("spn/captain: pinged home hub in %s", latency)
 	return clientResultOk

captain/module.go

@@ -17,6 +17,7 @@ import (
 	"github.com/safing/portmaster/network/netutils"
 	"github.com/safing/spn/conf"
 	"github.com/safing/spn/crew"
+	"github.com/safing/spn/navigator"
 	"github.com/safing/spn/patrol"
 	"github.com/safing/spn/ships"
 	_ "github.com/safing/spn/sluice"
@@ -162,6 +163,21 @@ func start() error {
 	// client + home hub manager
 	if conf.Client() {
 		module.StartServiceWorker("client manager", 0, clientManager)
+
+		// Reset failing hubs when the network changes while not connected.
+		if err := module.RegisterEventHook(
+			"netenv",
+			"network changed",
+			"reset failing hubs",
+			func(_ context.Context, _ interface{}) error {
+				if ready.IsNotSet() {
+					navigator.Main.ResetFailingStates(module.Ctx)
+				}
+				return nil
+			},
+		); err != nil {
+			return err
+		}
 	}
 
 	return nil

hub/errors.go

@@ -4,14 +4,14 @@ import "errors"
 
 var (
 	// ErrMissingInfo signifies that the hub is missing the HubAnnouncement.
-	ErrMissingInfo = errors.New("hub is missing Announcement")
+	ErrMissingInfo = errors.New("hub has no announcement")
 
 	// ErrMissingTransports signifies that the hub announcement did not specify any transports.
-	ErrMissingTransports = errors.New("hub announcement is missing transports")
+	ErrMissingTransports = errors.New("hub announcement has no transports")
 
 	// ErrMissingIPs signifies that the hub announcement did not specify any IPs,
 	// or none of the IPs is supported by the client.
-	ErrMissingIPs = errors.New("hub announcement is missing (supported) IPs")
+	ErrMissingIPs = errors.New("hub announcement has no (supported) IPs")
 
 	// ErrTemporaryValidationError is returned when a validation error might be temporary.
 	ErrTemporaryValidationError = errors.New("temporary validation error")

hub/hub.go

@@ -356,33 +356,29 @@ func (a *Announcement) prepare(force bool) error {
 	var err error
 
 	// Parse policies.
-	if a.entryPolicy == nil || force {
+	if len(a.entryPolicy) == 0 || force {
 		if a.entryPolicy, err = endpoints.ParseEndpoints(a.Entry); err != nil {
 			return fmt.Errorf("failed to parse entry policy: %w", err)
 		}
 	}
-	if a.exitPolicy == nil || force {
+	if len(a.exitPolicy) == 0 || force {
 		if a.exitPolicy, err = endpoints.ParseEndpoints(a.Exit); err != nil {
 			return fmt.Errorf("failed to parse exit policy: %w", err)
 		}
 	}
 
 	// Parse transports.
-	if a.parsedTransports == nil || force {
-		a.parsedTransports = make([]*Transport, 0, len(a.Transports))
-		for _, t := range a.Transports {
-			pt, err := ParseTransport(t)
-			if err != nil {
-				log.Warningf("hub: Hub %s (%s) has configured an unknown or invalid transport %q: %s", a.Name, a.ID, t, err)
-			} else {
-				a.parsedTransports = append(a.parsedTransports, pt)
-			}
+	if len(a.parsedTransports) == 0 || force {
+		parsed, errs := ParseTransports(a.Transports)
+		// Log parsing warnings.
+		for _, err := range errs {
+			log.Warningf("hub: Hub %s (%s) has configured an %s", a.Name, a.ID, err)
 		}
-		SortTransports(a.parsedTransports)
 		// Check if there are any valid transports.
-		if len(a.parsedTransports) == 0 {
+		if len(parsed) == 0 {
 			return ErrMissingTransports
 		}
+		a.parsedTransports = parsed
 	}
 
 	return nil

hub/transport.go

@@ -30,6 +30,25 @@ type Transport struct {
 	Option   string
 }
 
+// ParseTransports returns a list of parsed transports and errors from parsing
+// the given definitions.
+func ParseTransports(definitions []string) (transports []*Transport, errs []error) {
+	transports = make([]*Transport, 0, len(definitions))
+	for _, definition := range definitions {
+		parsed, err := ParseTransport(definition)
+		if err != nil {
+			errs = append(errs, fmt.Errorf(
+				"unknown or invalid transport %q: %w", definition, err,
+			))
+		} else {
+			transports = append(transports, parsed)
+		}
+	}
+
+	SortTransports(transports)
+	return transports, errs
+}
+
 // ParseTransport parses a transport definition.
 func ParseTransport(definition string) (*Transport, error) {
 	u, err := url.Parse(definition)

ships/launch.go

@@ -23,6 +23,13 @@ func Launch(ctx context.Context, h *hub.Hub, transport *hub.Transport, ip net.IP
 			return nil, hub.ErrMissingInfo
 		}
 		transports = h.Info.ParsedTransports()
+		// If there are no transports, check if they were parsed.
+		if len(transports) == 0 && len(h.Info.Transports) > 0 {
+			log.Errorf("ships: %s has no parsed transports, but transports are %v", h, h.Info.Transports)
+			// Attempt to parse transports now.
+			transports, _ = hub.ParseTransports(h.Info.Transports)
+		}
+		// Fail if there are not transports.
 		if len(transports) == 0 {
 			return nil, hub.ErrMissingTransports
 		}