-
Notifications
You must be signed in to change notification settings - Fork 122
lightning: encrypt stored seed #4146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: staging-spark
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,7 +3,12 @@ | |
| package lightning | ||
|
|
||
| import ( | ||
| "crypto/aes" | ||
| "crypto/cipher" | ||
| "crypto/rand" | ||
| "encoding/base64" | ||
| "encoding/hex" | ||
| "io" | ||
| "net/http" | ||
| "os" | ||
| "path" | ||
|
|
@@ -29,12 +34,21 @@ const ( | |
| breezApiKeyUrl = "https://bitboxapp.shiftcrypto.dev/lightning/breez-api-key" | ||
| ) | ||
|
|
||
| // Keep this local to avoid importing backend.Environment and creating a package cycle. | ||
| type environment interface { | ||
| CanEncryptLightningMnemonic() bool | ||
| StoreLightningEncryptionKey(accountCode string, encryptionKey string) error | ||
| LoadLightningEncryptionKey(accountCode string) (string, error) | ||
| DeleteLightningEncryptionKey(accountCode string) error | ||
| } | ||
|
|
||
| // Lightning manages the Breez SDK lightning node. | ||
| type Lightning struct { | ||
| observable.Implementation | ||
|
|
||
| backendConfig *config.Config | ||
| cacheDirectoryPath string | ||
| environment environment | ||
| getKeystore func() keystore.Keystore | ||
| synced bool | ||
|
|
||
|
|
@@ -48,13 +62,15 @@ type Lightning struct { | |
| // NewLightning creates a new instance of the Lightning struct. | ||
| func NewLightning(config *config.Config, | ||
| cacheDirectoryPath string, | ||
| environment environment, | ||
| getKeystore func() keystore.Keystore, | ||
| httpClient *http.Client, | ||
| ratesUpdater *rates.RateUpdater, | ||
| btcCoin coin.Coin) *Lightning { | ||
| return &Lightning{ | ||
| backendConfig: config, | ||
| cacheDirectoryPath: cacheDirectoryPath, | ||
| environment: environment, | ||
| getKeystore: getKeystore, | ||
| log: logging.Get().WithGroup("lightning"), | ||
| synced: false, | ||
|
|
@@ -64,7 +80,8 @@ func NewLightning(config *config.Config, | |
| } | ||
| } | ||
|
|
||
| // Activate first creates a mnemonic from the keystore entropy then connects to instance. | ||
| // Activate first creates a mnemonic from the keystore entropy, persists it, and connects to the | ||
| // instance. | ||
| func (lightning *Lightning) Activate() error { | ||
| if lightning.Account() != nil { | ||
| return errp.New("Lightning accounts already configured") | ||
|
|
@@ -91,13 +108,25 @@ func (lightning *Lightning) Activate() error { | |
| return errp.New("Error generating mnemonic") | ||
| } | ||
|
|
||
| accountCode := types.Code(strings.Join([]string{"v0-", hex.EncodeToString(fingerprint), "-ln-0"}, "")) | ||
| sealedMnemonic, err := lightning.sealMnemonic(string(accountCode), entropyMnemonic) | ||
| if err != nil { | ||
| lightning.log.WithError(err).Warn("Error configuring Lightning secure storage") | ||
| return errp.New("Could not configure Lightning secure storage on this device") | ||
| } | ||
|
|
||
| lightningAccount := config.LightningAccountConfig{ | ||
| Mnemonic: entropyMnemonic, | ||
| Mnemonic: sealedMnemonic, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i would rename the It also seems that the memonic field value can contain the sealed mnemonic or the plaintext memonic depending on CanEncryptLightningMnemonic. If this split will remain for a while, I'd use two different fields, or a union-like struct for clarity. |
||
| RootFingerprint: fingerprint, | ||
| Code: types.Code(strings.Join([]string{"v0-", hex.EncodeToString(fingerprint), "-ln-0"}, "")), | ||
| Code: accountCode, | ||
| Number: 0, | ||
| } | ||
| if err = lightning.SetAccount(&lightningAccount); err != nil { | ||
| if lightning.environment.CanEncryptLightningMnemonic() { | ||
| if deleteErr := lightning.environment.DeleteLightningEncryptionKey(string(accountCode)); deleteErr != nil { | ||
| lightning.log.WithError(deleteErr).Warn("Error deleting lightning encryption key after activation failure") | ||
| } | ||
| } | ||
| return err | ||
| } | ||
|
|
||
|
|
@@ -149,6 +178,12 @@ func (lightning *Lightning) Deactivate() error { | |
| return err | ||
| } | ||
|
|
||
| if lightning.environment.CanEncryptLightningMnemonic() { | ||
| if err := lightning.environment.DeleteLightningEncryptionKey(string(account.Code)); err != nil { | ||
| lightning.log.WithError(err).Warn("Error deleting lightning encryption key") | ||
|
benma marked this conversation as resolved.
|
||
| } | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
|
|
@@ -172,7 +207,6 @@ func (lightning *Lightning) Balance() (*accounts.Balance, error) { | |
| // before returning the balance | ||
| EnsureSynced: &ensureSynced, | ||
| }) | ||
|
|
||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
@@ -201,9 +235,15 @@ func (lightning *Lightning) connect(_ bool) error { | |
| return err | ||
| } | ||
|
|
||
| mnemonic, err := lightning.unsealMnemonic(account) | ||
| if err != nil { | ||
| lightning.log.WithError(err).Warn("Error unlocking Lightning mnemonic") | ||
| return errp.New("Error unlocking Lightning mnemonic from the device") | ||
| } | ||
|
|
||
| // Construct the seed using mnemonic words or entropy bytes | ||
| var seed breez_sdk_spark.Seed = breez_sdk_spark.SeedMnemonic{ | ||
| Mnemonic: account.Mnemonic, | ||
| Mnemonic: mnemonic, | ||
| Passphrase: nil, | ||
| } | ||
|
|
||
|
|
@@ -219,7 +259,9 @@ func (lightning *Lightning) connect(_ bool) error { | |
| config.PrivateEnabledDefault = true | ||
| // Set the maximum fee to the fastest network recommended fee at the time of claim | ||
| // with a leeway of 1 sats/vbyte | ||
| networkRecommendedInterface := breez_sdk_spark.MaxFee(breez_sdk_spark.MaxFeeNetworkRecommended{LeewaySatPerVbyte: 1}) | ||
| networkRecommendedInterface := breez_sdk_spark.MaxFee( | ||
| breez_sdk_spark.MaxFeeNetworkRecommended{LeewaySatPerVbyte: 1}, | ||
| ) | ||
| config.MaxDepositClaimFee = &networkRecommendedInterface | ||
|
|
||
| connectRequest := breez_sdk_spark.ConnectRequest{ | ||
|
|
@@ -252,6 +294,96 @@ func (lightning *Lightning) connect(_ bool) error { | |
| return nil | ||
| } | ||
|
|
||
| func (lightning *Lightning) sealMnemonic(accountCode string, mnemonic string) (string, error) { | ||
| if !lightning.environment.CanEncryptLightningMnemonic() { | ||
| return mnemonic, nil | ||
| } | ||
|
|
||
| encryptionKey := make([]byte, 32) | ||
| if _, err := io.ReadFull(rand.Reader, encryptionKey); err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| sealedMnemonic, err := encryptMnemonic(mnemonic, encryptionKey) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| if err := lightning.environment.StoreLightningEncryptionKey( | ||
| accountCode, | ||
| base64.StdEncoding.EncodeToString(encryptionKey), | ||
| ); err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| return sealedMnemonic, nil | ||
| } | ||
|
|
||
| func (lightning *Lightning) unsealMnemonic(account *config.LightningAccountConfig) (string, error) { | ||
| if !lightning.environment.CanEncryptLightningMnemonic() { | ||
| return account.Mnemonic, nil | ||
| } | ||
|
|
||
| encryptionKeyBase64, err := lightning.environment.LoadLightningEncryptionKey(string(account.Code)) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| encryptionKey, err := base64.StdEncoding.DecodeString(encryptionKeyBase64) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| mnemonic, err := decryptMnemonic(account.Mnemonic, encryptionKey) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
|
|
||
| return mnemonic, nil | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| func encryptMnemonic(mnemonic string, encryptionKey []byte) (string, error) { | ||
| block, err := aes.NewCipher(encryptionKey) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| gcm, err := cipher.NewGCM(block) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| nonce := make([]byte, gcm.NonceSize()) | ||
| if _, err := io.ReadFull(rand.Reader, nonce); err != nil { | ||
| return "", err | ||
| } | ||
| ciphertext := gcm.Seal(nil, nonce, []byte(mnemonic), nil) | ||
| return base64.StdEncoding.EncodeToString(append(nonce, ciphertext...)), nil | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How about prepending a |
||
| } | ||
|
|
||
| func decryptMnemonic(sealedMnemonic string, encryptionKey []byte) (string, error) { | ||
| rawCiphertext, err := base64.StdEncoding.DecodeString(sealedMnemonic) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| block, err := aes.NewCipher(encryptionKey) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| gcm, err := cipher.NewGCM(block) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| if len(rawCiphertext) < gcm.NonceSize() { | ||
| return "", errp.New("ciphertext too short") | ||
| } | ||
| nonce := rawCiphertext[:gcm.NonceSize()] | ||
| ciphertext := rawCiphertext[gcm.NonceSize():] | ||
| plaintext, err := gcm.Open(nil, nonce, ciphertext, nil) | ||
| if err != nil { | ||
| return "", err | ||
| } | ||
| return string(plaintext), nil | ||
| } | ||
|
|
||
| func (lightning *Lightning) getBreezApiKey() (*string, error) { | ||
| _, breezApiKey, err := util.HTTPGet(lightning.httpClient, breezApiKeyUrl, "", int64(4096)) | ||
| if err != nil { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Silent defaults on data-critical store/load could mask misconfiguration.
If a future caller sets
CanEncryptLightningMnemonicFuncto returntruebut forgets to wireStoreLightningEncryptionKeyFuncorLoadLightningEncryptionKeyFunc,Lightning.sealMnemonicwill think storage succeeded andLightning.unsealMnemonicwill receive an empty key (then fail base64-decode or AES-GCM open with the generic "reactivation required" message). This is consistent with how the other delegates degrade, but those are UX-only; here the consequence is users locked out of their Lightning seed. Consider returning an error when the func is missing so the misconfiguration surfaces immediately during activation rather than later at unlock time.🛡️ Proposed defensive fix
// StoreLightningEncryptionKey implements backend.Environment. func (env *BackendEnvironment) StoreLightningEncryptionKey(accountCode string, encryptionKey string) error { if env.StoreLightningEncryptionKeyFunc != nil { return env.StoreLightningEncryptionKeyFunc(accountCode, encryptionKey) } - return nil + return errp.New("StoreLightningEncryptionKey not implemented in this environment") } // LoadLightningEncryptionKey implements backend.Environment. func (env *BackendEnvironment) LoadLightningEncryptionKey(accountCode string) (string, error) { if env.LoadLightningEncryptionKeyFunc != nil { return env.LoadLightningEncryptionKeyFunc(accountCode) } - return "", nil + return "", errp.New("LoadLightningEncryptionKey not implemented in this environment") }🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need this to allow using the seed without encryption in webdev mode. Lightning activation will be hidden in the UI for all the other platforms that don't support encryption in a future PR. I'll keep it as is for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Beerosagos, understood — sinceCanEncryptLightningMnemonic()returnsfalsefor webdev (and other non-supporting platforms), theStore/Loadpaths won't be reached during normal flow, and the future UI guard will provide an additional layer of protection. That makes the silent defaults a reasonable design choice here. Thanks for the context!✏️ Learnings added