In LoadPrivateKey (pkg/vpn/crypto.go), the recovery path for a corrupted stored key returns a nil private key with a nil error:
privKey, err := crypto.UnmarshalPrivateKey(decodedPrivatKey)
if err != nil {
_, newPkBase64, err := generateNewSecp256k1PrivateKey()
utils.Check(err)
config.PrivateKey = newPkBase64
utils.SaveConfig(config)
LoadPrivateKey() // result discarded
}
return privKey, nil // privKey is nil here
Problems:
-
When UnmarshalPrivateKey fails, a new key is generated and saved, but the recursive LoadPrivateKey() result is discarded and the function falls through to return privKey, nil, returning a nil key and no error. SetNodeUp then passes that nil key to libp2p.New(libp2p.Identity(pk)), so the node either fails to start or starts with an identity that does not match the key just written to the config file.
-
The inner err from generateNewSecp256k1PrivateKey shadows the outer one, and crypto.ConfigDecodeKey's error goes through utils.Check, which calls log.Fatal, so a corrupted base64 value kills the process instead of triggering the regeneration path.
-
On any LoadConfiguration error the function silently generates a brand new identity and overwrites config.PrivateKey. A transient read error is enough to rotate the node identity, which permanently changes the peer ID.
Suggested fix: make the function return the freshly generated key (return LoadPrivateKey() or restructure without recursion), only regenerate when the stored key is actually invalid, and propagate errors instead of utils.Check/silent fallthrough.
In
LoadPrivateKey(pkg/vpn/crypto.go), the recovery path for a corrupted stored key returns a nil private key with a nil error:Problems:
When
UnmarshalPrivateKeyfails, a new key is generated and saved, but the recursiveLoadPrivateKey()result is discarded and the function falls through toreturn privKey, nil, returning a nil key and no error.SetNodeUpthen passes that nil key tolibp2p.New(libp2p.Identity(pk)), so the node either fails to start or starts with an identity that does not match the key just written to the config file.The inner
errfromgenerateNewSecp256k1PrivateKeyshadows the outer one, andcrypto.ConfigDecodeKey's error goes throughutils.Check, which callslog.Fatal, so a corrupted base64 value kills the process instead of triggering the regeneration path.On any
LoadConfigurationerror the function silently generates a brand new identity and overwritesconfig.PrivateKey. A transient read error is enough to rotate the node identity, which permanently changes the peer ID.Suggested fix: make the function return the freshly generated key (
return LoadPrivateKey()or restructure without recursion), only regenerate when the stored key is actually invalid, and propagate errors instead ofutils.Check/silent fallthrough.