[client] Let a logged-out profile deserialize again (review item 2)

ConfigFromJSON refused a document with no WireGuard or SSH key. A config
legitimately has none between a logout and the next login: mobile
LogoutProfile clears both in place and writes the profile back, so the peer
re-registers on the next login instead of returning as itself.

So the refusal broke the mobile flows it was meant to protect. On iOS and
tvOS the stored JSON of a logged-out profile stopped loading through
Client.SetConfigFromJSON and Auth.SetConfigFromJSON, and copyConfig — which
round-trips a Config through JSON to take an in-memory copy before applying
the MDM overlay — failed on the same document. Where the old code silently
minted a key, this returned an error, which is worse for logout and profile
switching alike: neither is asking to connect.

The deserializer now stays out of the identity question in both directions:
it does not generate one (a read cannot hand back keys nothing will write
down) and does not refuse one that is absent. Whoever goes on to connect is
where an absent identity has to be answered — and it already is, by the
login path that provisions and persists.

ErrConfigWithoutIdentity goes with it; nothing else used it.
This commit is contained in:
riccardom
2026-09-11 14:53:04 +02:00
parent 222ad91c4d
commit 07dc9cb1c8
4 changed files with 32 additions and 52 deletions
+8 -17
View File
@@ -1443,17 +1443,15 @@ func ConfigToJSON(config *Config) (string, error) {
// ConfigFromJSON deserializes a JSON string to a Config struct. // ConfigFromJSON deserializes a JSON string to a Config struct.
// This is useful for restoring config from alternative storage mechanisms. // This is useful for restoring config from alternative storage mechanisms.
// After unmarshaling, defaults are applied to ensure the config is fully // After unmarshaling, defaults are applied to ensure the config is fully
// initialized. The peer identity is not one of those defaults: a document // initialized.
// carrying none is refused with ErrConfigWithoutIdentity.
// //
// Provisioning one here would be worse than refusing. Both callers connect // The peer identity is deliberately none of its business, in either direction.
// with what they get back — the iOS SDK's Client.SetConfigFromJSON keeps it as // It does not generate one: a read cannot hand back keys that nothing will
// the preloaded config Run() uses, and Auth.SetConfigFromJSON as the config it // write down (see ReadOrGenerateConfig). Nor does it refuse a document that
// authenticates with — and neither can hand a generated key back to the store // carries none, because a config legitimately has no identity between a logout
// the document came from, since Client exports no config at all. The peer // and the next login — mobile logout clears both keys in place — and this is
// would connect under an identity nothing persists and re-register on every // also the deserializer the iOS SDK copies a config through. Whoever goes on
// launch. A document with no identity means nobody has logged in yet, and // to connect is where an absent identity has to be answered.
// that is what the caller has to be told.
func ConfigFromJSON(jsonStr string) (*Config, error) { func ConfigFromJSON(jsonStr string) (*Config, error) {
config := &Config{} config := &Config{}
err := json.Unmarshal([]byte(jsonStr), config) err := json.Unmarshal([]byte(jsonStr), config)
@@ -1467,12 +1465,5 @@ func ConfigFromJSON(jsonStr string) (*Config, error) {
return nil, fmt.Errorf("failed to apply defaults to config: %w", err) 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 return config, nil
} }
@@ -7,16 +7,16 @@ import (
"github.com/stretchr/testify/require" "github.com/stretchr/testify/require"
) )
// The serialized form is how the tvOS SDK stores a profile, and its callers // The serialized form is how the tvOS SDK stores a profile and how the iOS SDK
// connect with whatever comes back. A document with no identity used to be // copies one in memory, so it must round-trip whatever a profile legitimately
// completed by apply() minting keys, which meant connecting as a peer nothing // holds — including no identity at all, which is the state mobile logout leaves
// could persist; it is now refused, since the only honest answer to "restore // behind when it clears both keys in place. Refusing that document here broke
// this config" for a config that was never logged in is to say so. // logout, profile switching and the login that follows them.
func TestConfigFromJSONRefusesADocumentWithoutAnIdentity(t *testing.T) { func TestConfigFromJSONRoundTripsALoggedOutProfile(t *testing.T) {
path := filepath.Join(t.TempDir(), "exported.json") path := filepath.Join(t.TempDir(), "exported.json")
stored, err := UpdateOrCreateConfig(ConfigInput{ConfigPath: path, ManagementURL: DefaultManagementURL}) stored, err := UpdateOrCreateConfig(ConfigInput{ConfigPath: path, ManagementURL: DefaultManagementURL})
require.NoError(t, err) require.NoError(t, err)
require.NotEmpty(t, stored.PrivateKey, "a provisioned config is the fixture this test needs") require.NotEmpty(t, stored.PrivateKey, "a provisioned config is the fixture this test starts from")
require.NotEmpty(t, stored.SSHKey) require.NotEmpty(t, stored.SSHKey)
exported, err := ConfigToJSON(stored) exported, err := ConfigToJSON(stored)
@@ -27,23 +27,18 @@ func TestConfigFromJSONRefusesADocumentWithoutAnIdentity(t *testing.T) {
require.Equal(t, stored.PrivateKey, restored.PrivateKey, "the restored peer is not the stored one") require.Equal(t, stored.PrivateKey, restored.PrivateKey, "the restored peer is not the stored one")
require.Equal(t, stored.SSHKey, restored.SSHKey) require.Equal(t, stored.SSHKey, restored.SSHKey)
for _, missing := range []struct { // What mobile logout leaves on disk.
name string loggedOut := stored.clone()
strip func(*Config) loggedOut.PrivateKey = ""
}{ loggedOut.SSHKey = ""
{"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) document, err := ConfigToJSON(loggedOut)
require.NoError(t, err) require.NoError(t, err)
_, err = ConfigFromJSON(document) reloaded, err := ConfigFromJSON(document)
require.ErrorIs(t, err, ErrConfigWithoutIdentity) require.NoError(t, err, "a logged-out profile must still load")
}) require.Empty(t, reloaded.PrivateKey, "loading must not mint a key nothing will write down")
} require.Empty(t, reloaded.SSHKey)
require.Equal(t, stored.ManagementURL.String(), reloaded.ManagementURL.String(),
"the rest of the profile survives the logout")
} }
-5
View File
@@ -6,9 +6,4 @@ var (
ErrProfileNotFound = errors.New("profile not found") ErrProfileNotFound = errors.New("profile not found")
ErrProfileAlreadyExists = errors.New("profile already exists") ErrProfileAlreadyExists = errors.New("profile already exists")
ErrNoActiveProfile = errors.New("no active profile set") 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")
) )
+5 -6
View File
@@ -130,13 +130,12 @@ func NewClient(cfgFile, stateFile, cacheDir, logFilePath, deviceName string, osV
// SetConfigFromJSON stores the JSON config that later loads resolve instead of the config file (tvOS). // SetConfigFromJSON stores the JSON config that later loads resolve instead of the config file (tvOS).
func (c *Client) SetConfigFromJSON(jsonStr string) error { func (c *Client) SetConfigFromJSON(jsonStr string) error {
// Parsed only to reject a bad document early; the JSON itself is what is // Parsed only to reject an unreadable document early; the JSON itself is
// stored, and every load re-parses it. Not only a parse error any more: a // what is stored, and every load re-parses it. A document carrying no peer
// document with no peer identity is refused too, because Run() would // identity is readable and accepted: that is a logged-out profile, and the
// otherwise connect as a peer whose key this SDK has no way to hand back // login that follows provisions the keys.
// to the caller's store.
if _, err := profilemanager.ConfigFromJSON(jsonStr); err != nil { if _, err := profilemanager.ConfigFromJSON(jsonStr); err != nil {
log.Errorf("SetConfigFromJSON: failed to load config JSON: %v", err) log.Errorf("SetConfigFromJSON: failed to parse config JSON: %v", err)
return err return err
} }
c.preloadedConfigJSON.Store(&jsonStr) c.preloadedConfigJSON.Store(&jsonStr)