diff --git a/client/internal/profilemanager/config.go b/client/internal/profilemanager/config.go index 718ff722f..7d294505c 100644 --- a/client/internal/profilemanager/config.go +++ b/client/internal/profilemanager/config.go @@ -306,16 +306,29 @@ func newConfigSkeleton() *Config { } } -// createNewConfig creates a new config in memory, generating the keys that -// identify the peer. Writing it out is the caller's job. +// createNewConfig resolves a new config in memory, with no identity: whoever +// needs the peer's keys calls EnsureIdentity and persists the result, so a read +// that lands on a missing file cannot hand back a config carrying keys that +// nothing will ever write down. func createNewConfig(input ConfigInput) (*Config, error) { config := newConfigSkeleton() - if _, err := config.EnsureIdentity(); err != nil { + if _, err := config.apply(input); err != nil { return nil, err } - if _, err := config.apply(input); err != nil { + return config, nil +} + +// createProvisionedConfig is createNewConfig plus the peer's identity, for the +// callers that go on to persist the config or to connect with it. +func createProvisionedConfig(input ConfigInput) (*Config, error) { + config, err := createNewConfig(input) + if err != nil { + return nil, err + } + + if _, err := config.EnsureIdentity(); err != nil { return nil, err } @@ -1084,7 +1097,7 @@ func UpdateOrCreateConfig(input ConfigInput) (*Config, error) { } if !configExists { log.Infof("generating new config %s", input.ConfigPath) - cfg, err := createNewConfig(input) + cfg, err := createProvisionedConfig(input) if err != nil { return nil, err } @@ -1215,9 +1228,10 @@ func UpdateOldManagementURL(ctx context.Context, config *Config, configPath stri return newConfig, nil } -// CreateInMemoryConfig generate a new config but do not write out it to the store +// CreateInMemoryConfig generate a new config but do not write out it to the store. +// It carries an identity: callers connect with what they get back. func CreateInMemoryConfig(input ConfigInput) (*Config, error) { - return createNewConfig(input) + return createProvisionedConfig(input) } // ReadOrGenerateConfig reads the profile config at configPath if it exists, or @@ -1282,7 +1296,7 @@ func DirectUpdateOrCreateConfig(input ConfigInput) (*Config, error) { } if !configExists { log.Infof("generating new config %s", input.ConfigPath) - cfg, err := createNewConfig(input) + cfg, err := createProvisionedConfig(input) if err != nil { return nil, err } diff --git a/client/internal/profilemanager/config_would_change_test.go b/client/internal/profilemanager/config_would_change_test.go index 00d8fc4e8..8bc863fb7 100644 --- a/client/internal/profilemanager/config_would_change_test.go +++ b/client/internal/profilemanager/config_would_change_test.go @@ -302,3 +302,30 @@ func TestWouldChangeIgnoresRestatedCertificatePaths(t *testing.T) { require.NoError(t, err) require.True(t, changed, "a different certificate path is a change") } + +// A read that lands on a missing file must not hand back keys: nothing would +// write them down, so the caller would connect with an identity that changes on +// the next run and registers a second peer. +func TestReadOrGenerateConfigCarriesNoIdentity(t *testing.T) { + cfg, err := ReadOrGenerateConfig(filepath.Join(t.TempDir(), "absent.json")) + require.NoError(t, err) + + require.Empty(t, cfg.PrivateKey, "a read minted a WireGuard key") + require.Empty(t, cfg.SSHKey, "a read minted an SSH key") + + // So the caller's own EnsureIdentity is the one that reports the work, and + // therefore the one that triggers the write. + generated, err := cfg.EnsureIdentity() + require.NoError(t, err) + require.True(t, generated, "the provisioning caller could not tell it had to persist the identity") +} + +// CreateInMemoryConfig is the opposite contract: its callers connect with what +// they get back, so it does carry an identity. +func TestCreateInMemoryConfigCarriesAnIdentity(t *testing.T) { + cfg, err := CreateInMemoryConfig(ConfigInput{ManagementURL: "https://api.netbird.io:443"}) + require.NoError(t, err) + + require.NotEmpty(t, cfg.PrivateKey) + require.NotEmpty(t, cfg.SSHKey) +}