diff --git a/client/internal/profilemanager/config.go b/client/internal/profilemanager/config.go index 036ed0e76..a4bc0ebbb 100644 --- a/client/internal/profilemanager/config.go +++ b/client/internal/profilemanager/config.go @@ -1432,7 +1432,18 @@ func ConfigToJSON(config *Config) (string, error) { // ConfigFromJSON deserializes a JSON string to a Config struct. // This is useful for restoring config from alternative storage mechanisms. -// After unmarshaling, defaults are applied to ensure the config is fully initialized. +// After unmarshaling, defaults are applied to ensure the config is fully +// initialized. The peer identity is not one of those defaults: a document +// carrying none is refused with ErrConfigWithoutIdentity. +// +// Provisioning one here would be worse than refusing. Both callers connect +// with what they get back — the iOS SDK's Client.SetConfigFromJSON keeps it as +// the preloaded config Run() uses, and Auth.SetConfigFromJSON as the config it +// authenticates with — and neither can hand a generated key back to the store +// the document came from, since Client exports no config at all. The peer +// would connect under an identity nothing persists and re-register on every +// launch. A document with no identity means nobody has logged in yet, and +// that is what the caller has to be told. func ConfigFromJSON(jsonStr string) (*Config, error) { config := &Config{} err := json.Unmarshal([]byte(jsonStr), config) @@ -1446,5 +1457,12 @@ func ConfigFromJSON(jsonStr string) (*Config, error) { return nil, fmt.Errorf("failed to apply defaults to config: %w", err) } + // Both keys, because both are dead ends when missing: an empty WireGuard + // key fails the management login on its size, and an empty SSH key fails + // ssh.GeneratePublicKey in ConnectClient before the engine starts. + if config.PrivateKey == "" || config.SSHKey == "" { + return nil, ErrConfigWithoutIdentity + } + return config, nil } diff --git a/client/internal/profilemanager/config_json_test.go b/client/internal/profilemanager/config_json_test.go new file mode 100644 index 000000000..3000ee8a0 --- /dev/null +++ b/client/internal/profilemanager/config_json_test.go @@ -0,0 +1,49 @@ +package profilemanager + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +// The serialized form is how the tvOS SDK stores a profile, and its callers +// connect with whatever comes back. A document with no identity used to be +// completed by apply() minting keys, which meant connecting as a peer nothing +// could persist; it is now refused, since the only honest answer to "restore +// this config" for a config that was never logged in is to say so. +func TestConfigFromJSONRefusesADocumentWithoutAnIdentity(t *testing.T) { + path := filepath.Join(t.TempDir(), "exported.json") + stored, err := UpdateOrCreateConfig(ConfigInput{ConfigPath: path, ManagementURL: DefaultManagementURL}) + require.NoError(t, err) + require.NotEmpty(t, stored.PrivateKey, "a provisioned config is the fixture this test needs") + require.NotEmpty(t, stored.SSHKey) + + exported, err := ConfigToJSON(stored) + require.NoError(t, err) + + restored, err := ConfigFromJSON(exported) + require.NoError(t, err, "a config exported after a login must load") + require.Equal(t, stored.PrivateKey, restored.PrivateKey, "the restored peer is not the stored one") + require.Equal(t, stored.SSHKey, restored.SSHKey) + + for _, missing := range []struct { + name string + strip func(*Config) + }{ + {"no WireGuard key", func(c *Config) { c.PrivateKey = "" }}, + {"no SSH key", func(c *Config) { c.SSHKey = "" }}, + {"no keys at all", func(c *Config) { c.PrivateKey = ""; c.SSHKey = "" }}, + } { + t.Run(missing.name, func(t *testing.T) { + incomplete := stored.clone() + missing.strip(incomplete) + + document, err := ConfigToJSON(incomplete) + require.NoError(t, err) + + _, err = ConfigFromJSON(document) + require.ErrorIs(t, err, ErrConfigWithoutIdentity) + }) + } +} diff --git a/client/internal/profilemanager/error.go b/client/internal/profilemanager/error.go index d83fe5c1c..b5a1ad7e0 100644 --- a/client/internal/profilemanager/error.go +++ b/client/internal/profilemanager/error.go @@ -6,4 +6,9 @@ var ( ErrProfileNotFound = errors.New("profile not found") ErrProfileAlreadyExists = errors.New("profile already exists") ErrNoActiveProfile = errors.New("no active profile set") + + // ErrConfigWithoutIdentity is returned for a serialized config that carries + // no WireGuard or SSH key. See ConfigFromJSON for why it is refused rather + // than provisioned. + ErrConfigWithoutIdentity = errors.New("config carries no peer identity: log in to provision one before loading a stored config") ) diff --git a/client/ios/NetBirdSDK/client.go b/client/ios/NetBirdSDK/client.go index bbbb969c9..5ffff52c8 100644 --- a/client/ios/NetBirdSDK/client.go +++ b/client/ios/NetBirdSDK/client.go @@ -128,7 +128,10 @@ func NewClient(cfgFile, stateFile, cacheDir, logFilePath, deviceName string, osV func (c *Client) SetConfigFromJSON(jsonStr string) error { cfg, err := profilemanager.ConfigFromJSON(jsonStr) if err != nil { - log.Errorf("SetConfigFromJSON: failed to parse config JSON: %v", err) + // Not only a parse error any more: a document with no peer identity is + // refused, because Run() would otherwise connect as a peer whose key + // this SDK has no way to hand back to the caller's store. + log.Errorf("SetConfigFromJSON: failed to load config JSON: %v", err) return err } c.preloadedConfig = cfg