From 6aa0482abc7f86b3cd9a61205376cb99498975f2 Mon Sep 17 00:00:00 2001 From: Chris Schinnerl Date: Fri, 30 Aug 2024 10:04:58 +0200 Subject: [PATCH] bus: fix invalid private key --- bus/bus.go | 6 ++- bus/routes.go | 8 ++-- cmd/renterd/node.go | 2 +- internal/test/e2e/cluster.go | 2 +- internal/utils/keys.go | 66 ++++++++++++++++++++++++++++++++ internal/worker/accounts.go | 32 +++------------- internal/worker/accounts_test.go | 3 +- worker/worker.go | 25 ++---------- 8 files changed, 86 insertions(+), 58 deletions(-) create mode 100644 internal/utils/keys.go diff --git a/bus/bus.go b/bus/bus.go index 3d13c2eae..065912c15 100644 --- a/bus/bus.go +++ b/bus/bus.go @@ -31,6 +31,7 @@ import ( "go.sia.tech/renterd/internal/rhp" rhp2 "go.sia.tech/renterd/internal/rhp/v2" rhp3 "go.sia.tech/renterd/internal/rhp/v3" + "go.sia.tech/renterd/internal/utils" "go.sia.tech/renterd/object" "go.sia.tech/renterd/stores/sql" "go.sia.tech/renterd/webhooks" @@ -42,6 +43,7 @@ const ( defaultWalletRecordMetricInterval = 5 * time.Minute defaultPinUpdateInterval = 5 * time.Minute defaultPinRateWindow = 6 * time.Hour + lockingPriorityFunding = 40 lockingPriorityRenew = 80 stdTxnSize = 1200 // bytes ) @@ -297,7 +299,7 @@ type ( type Bus struct { startTime time.Time - masterKey types.PrivateKey + masterKey utils.MasterKey alerts alerts.Alerter alertMgr AlertManager @@ -326,7 +328,7 @@ type Bus struct { } // New returns a new Bus -func New(ctx context.Context, masterKey types.PrivateKey, am AlertManager, wm WebhooksManager, cm ChainManager, s Syncer, w Wallet, store Store, announcementMaxAge time.Duration, l *zap.Logger) (_ *Bus, err error) { +func New(ctx context.Context, masterKey [32]byte, am AlertManager, wm WebhooksManager, cm ChainManager, s Syncer, w Wallet, store Store, announcementMaxAge time.Duration, l *zap.Logger) (_ *Bus, err error) { l = l.Named("bus") b := &Bus{ diff --git a/bus/routes.go b/bus/routes.go index a45f27166..a9fe9df99 100644 --- a/bus/routes.go +++ b/bus/routes.go @@ -49,14 +49,14 @@ func (b *Bus) accountsFundHandler(jc jape.Context) { return } - const lockingPriorityFunding = 40 - // contract metadata cm, err := b.ms.Contract(jc.Request.Context(), req.ContractID) if jc.Check("failed to fetch contract metadata", err) != nil { return } + rk := b.masterKey.DeriveContractKey(cm.HostKey) + // acquire contract lockID, err := b.contractLocker.Acquire(jc.Request.Context(), lockingPriorityFunding, req.ContractID, math.MaxInt64) if jc.Check("failed to acquire lock", err) != nil { @@ -77,7 +77,7 @@ func (b *Bus) accountsFundHandler(jc jape.Context) { } // price table - pt, err := b.rhp3.PriceTable(jc.Request.Context(), cm.HostKey, cm.SiamuxAddr, rhp3.PreparePriceTableContractPayment(&rev, req.AccountID, b.masterKey)) + pt, err := b.rhp3.PriceTable(jc.Request.Context(), cm.HostKey, cm.SiamuxAddr, rhp3.PreparePriceTableContractPayment(&rev, req.AccountID, rk)) if jc.Check("failed to fetch price table", err) != nil { return } @@ -101,7 +101,7 @@ func (b *Bus) accountsFundHandler(jc jape.Context) { } // fund the account - err = b.rhp3.FundAccount(jc.Request.Context(), &rev, cm.HostKey, cm.SiamuxAddr, deposit, req.AccountID, pt.HostPriceTable, b.masterKey) + err = b.rhp3.FundAccount(jc.Request.Context(), &rev, cm.HostKey, cm.SiamuxAddr, deposit, req.AccountID, pt.HostPriceTable, rk) if jc.Check("failed to fund account", err) != nil { return } diff --git a/cmd/renterd/node.go b/cmd/renterd/node.go index 0bd525721..a9758439c 100644 --- a/cmd/renterd/node.go +++ b/cmd/renterd/node.go @@ -382,7 +382,7 @@ func newBus(ctx context.Context, cfg config.Config, pk types.PrivateKey, network // create bus announcementMaxAgeHours := time.Duration(cfg.Bus.AnnouncementMaxAgeHours) * time.Hour - b, err := bus.New(ctx, masterKey[:], alertsMgr, wh, cm, s, w, sqlStore, announcementMaxAgeHours, logger) + b, err := bus.New(ctx, masterKey, alertsMgr, wh, cm, s, w, sqlStore, announcementMaxAgeHours, logger) if err != nil { return nil, nil, fmt.Errorf("failed to create bus: %w", err) } diff --git a/internal/test/e2e/cluster.go b/internal/test/e2e/cluster.go index 4dd098a66..99381058f 100644 --- a/internal/test/e2e/cluster.go +++ b/internal/test/e2e/cluster.go @@ -587,7 +587,7 @@ func newTestBus(ctx context.Context, dir string, cfg config.Bus, cfgDb dbConfig, // create bus announcementMaxAgeHours := time.Duration(cfg.AnnouncementMaxAgeHours) * time.Hour - b, err := bus.New(ctx, masterKey[:], alertsMgr, wh, cm, s, w, sqlStore, announcementMaxAgeHours, logger) + b, err := bus.New(ctx, masterKey, alertsMgr, wh, cm, s, w, sqlStore, announcementMaxAgeHours, logger) if err != nil { return nil, nil, nil, nil, err } diff --git a/internal/utils/keys.go b/internal/utils/keys.go new file mode 100644 index 000000000..0122491a0 --- /dev/null +++ b/internal/utils/keys.go @@ -0,0 +1,66 @@ +package utils + +import ( + "fmt" + + "go.sia.tech/core/types" + "golang.org/x/crypto/blake2b" +) + +type ( + MasterKey [32]byte + AccountsKey types.PrivateKey +) + +// DeriveAccountsKey derives an accounts key from a masterkey which is used +// to derive individual account keys from. +func (key *MasterKey) DeriveAccountsKey(workerID string) AccountsKey { + keyPath := fmt.Sprintf("accounts/%s", workerID) + return AccountsKey(key.deriveSubKey(keyPath)) +} + +// DeriveContractKey derives a contract key from a masterkey which is used to +// form, renew and revise contracts. +func (key *MasterKey) DeriveContractKey(hostKey types.PublicKey) types.PrivateKey { + seed := blake2b.Sum256(append(key.deriveSubKey("renterkey"), hostKey[:]...)) + pk := types.NewPrivateKeyFromSeed(seed[:]) + for i := range seed { + seed[i] = 0 + } + return pk +} + +// deriveSubKey can be used to derive a sub-masterkey from the worker's +// masterkey to use for a specific purpose. Such as deriving more keys for +// ephemeral accounts. +func (key *MasterKey) deriveSubKey(purpose string) types.PrivateKey { + seed := blake2b.Sum256(append(key[:], []byte(purpose)...)) + pk := types.NewPrivateKeyFromSeed(seed[:]) + for i := range seed { + seed[i] = 0 + } + return pk +} + +// DeriveAccountKey derives an account plus key for a given host and worker. +// Each worker has its own account for a given host. That makes concurrency +// around keeping track of an accounts balance and refilling it a lot easier in +// a multi-worker setup. +func (key *AccountsKey) DeriveAccountKey(hk types.PublicKey) types.PrivateKey { + index := byte(0) // not used yet but can be used to derive more than 1 account per host + + // Append the host for which to create it and the index to the + // corresponding sub-key. + subKey := *key + data := make([]byte, 0, len(subKey)+len(hk)+1) + data = append(data, subKey[:]...) + data = append(data, hk[:]...) + data = append(data, index) + + seed := types.HashBytes(data) + pk := types.NewPrivateKeyFromSeed(seed[:]) + for i := range seed { + seed[i] = 0 + } + return pk +} diff --git a/internal/worker/accounts.go b/internal/worker/accounts.go index 881922b94..1022075f1 100644 --- a/internal/worker/accounts.go +++ b/internal/worker/accounts.go @@ -13,6 +13,7 @@ import ( "go.sia.tech/renterd/alerts" "go.sia.tech/renterd/api" rhp3 "go.sia.tech/renterd/internal/rhp/v3" + "go.sia.tech/renterd/internal/utils" "go.uber.org/zap" ) @@ -61,7 +62,7 @@ type ( dc DownloadContracts cs ConsensusState s AccountStore - key types.PrivateKey + key utils.AccountsKey logger *zap.SugaredLogger owner string refillInterval time.Duration @@ -91,7 +92,7 @@ type ( // NewAccountManager creates a new account manager. It will load all accounts // from the given store and mark the shutdown as unclean. When Shutdown is // called it will save all accounts. -func NewAccountManager(key types.PrivateKey, owner string, alerter alerts.Alerter, funder AccountFunder, syncer AccountSyncer, cs ConsensusState, dc DownloadContracts, s AccountStore, refillInterval time.Duration, l *zap.Logger) (*AccountMgr, error) { +func NewAccountManager(key utils.AccountsKey, owner string, alerter alerts.Alerter, funder AccountFunder, syncer AccountSyncer, cs ConsensusState, dc DownloadContracts, s AccountStore, refillInterval time.Duration, l *zap.Logger) (*AccountMgr, error) { logger := l.Named("accounts").Sugar() shutdownCtx, shutdownCancel := context.WithCancel(context.Background()) @@ -182,7 +183,7 @@ func (a *AccountMgr) account(hk types.PublicKey) *Account { defer a.mu.Unlock() // Derive account key. - accKey := deriveAccountKey(a.key, hk) + accKey := a.key.DeriveAccountKey(hk) accID := rhpv3.Account(accKey.PublicKey()) // Create account if it doesn't exist. @@ -244,7 +245,7 @@ func (a *AccountMgr) run() { a.mu.Lock() accounts := make(map[rhpv3.Account]*Account, len(saved)) for _, acc := range saved { - accKey := deriveAccountKey(a.key, acc.HostKey) + accKey := a.key.DeriveAccountKey(acc.HostKey) if rhpv3.Account(accKey.PublicKey()) != acc.ID { a.logger.Errorf("account key derivation mismatch %v != %v", accKey.PublicKey(), acc.ID) continue @@ -592,29 +593,6 @@ func (a *Account) setBalance(balance *big.Int) { zap.Stringer("drift", drift)) } -// deriveAccountKey derives an account plus key for a given host and worker. -// Each worker has its own account for a given host. That makes concurrency -// around keeping track of an accounts balance and refilling it a lot easier in -// a multi-worker setup. -func deriveAccountKey(mgrKey types.PrivateKey, hostKey types.PublicKey) types.PrivateKey { - index := byte(0) // not used yet but can be used to derive more than 1 account per host - - // Append the host for which to create it and the index to the - // corresponding sub-key. - subKey := mgrKey - data := make([]byte, 0, len(subKey)+len(hostKey)+1) - data = append(data, subKey[:]...) - data = append(data, hostKey[:]...) - data = append(data, index) - - seed := types.HashBytes(data) - pk := types.NewPrivateKeyFromSeed(seed[:]) - for i := range seed { - seed[i] = 0 - } - return pk -} - func newAccountRefillAlert(id rhpv3.Account, contract api.ContractMetadata, err error, keysAndValues ...string) alerts.Alert { data := map[string]interface{}{ "error": err.Error(), diff --git a/internal/worker/accounts_test.go b/internal/worker/accounts_test.go index dca1c3633..d33e67200 100644 --- a/internal/worker/accounts_test.go +++ b/internal/worker/accounts_test.go @@ -10,6 +10,7 @@ import ( "go.sia.tech/core/types" "go.sia.tech/renterd/alerts" "go.sia.tech/renterd/api" + "go.sia.tech/renterd/internal/utils" "go.uber.org/zap" ) @@ -59,7 +60,7 @@ func TestAccounts(t *testing.T) { }, }, } - mgr, err := NewAccountManager(types.GeneratePrivateKey(), "test", b, b, b, b, b, b, time.Second, zap.NewNop()) + mgr, err := NewAccountManager(utils.AccountsKey(types.GeneratePrivateKey()), "test", b, b, b, b, b, b, time.Second, zap.NewNop()) if err != nil { t.Fatal(err) } diff --git a/worker/worker.go b/worker/worker.go index 2f0381932..be27f2cc6 100644 --- a/worker/worker.go +++ b/worker/worker.go @@ -36,7 +36,6 @@ import ( "go.sia.tech/renterd/worker/client" "go.sia.tech/renterd/worker/s3" "go.uber.org/zap" - "golang.org/x/crypto/blake2b" ) const ( @@ -156,18 +155,6 @@ type ( } ) -// deriveSubKey can be used to derive a sub-masterkey from the worker's -// masterkey to use for a specific purpose. Such as deriving more keys for -// ephemeral accounts. -func (w *Worker) deriveSubKey(purpose string) types.PrivateKey { - seed := blake2b.Sum256(append(w.masterKey[:], []byte(purpose)...)) - pk := types.NewPrivateKeyFromSeed(seed[:]) - for i := range seed { - seed[i] = 0 - } - return pk -} - // TODO: deriving the renter key from the host key using the master key only // works if we persist a hash of the renter's master key in the database and // compare it on startup, otherwise there's no way of knowing the derived key is @@ -181,12 +168,7 @@ func (w *Worker) deriveSubKey(purpose string) types.PrivateKey { // TODO: instead of deriving a renter key use a randomly generated salt so we're // not limited to one key per host func (w *Worker) deriveRenterKey(hostKey types.PublicKey) types.PrivateKey { - seed := blake2b.Sum256(append(w.deriveSubKey("renterkey"), hostKey[:]...)) - pk := types.NewPrivateKeyFromSeed(seed[:]) - for i := range seed { - seed[i] = 0 - } - return pk + return w.masterKey.DeriveContractKey(hostKey) } // A worker talks to Sia hosts to perform contract and storage operations within @@ -200,7 +182,7 @@ type Worker struct { allowPrivateIPs bool id string bus Bus - masterKey [32]byte + masterKey utils.MasterKey startTime time.Time eventSubscriber iworker.EventSubscriber @@ -1590,8 +1572,7 @@ func (w *Worker) initAccounts(refillInterval time.Duration) (err error) { if w.accounts != nil { panic("priceTables already initialized") // developer error } - keyPath := fmt.Sprintf("accounts/%s", w.id) - w.accounts, err = iworker.NewAccountManager(w.deriveSubKey(keyPath), w.id, w.bus, w, w, w.bus, w.cache, w.bus, refillInterval, w.logger.Desugar()) + w.accounts, err = iworker.NewAccountManager(w.masterKey.DeriveAccountsKey(w.id), w.id, w.bus, w, w, w.bus, w.cache, w.bus, refillInterval, w.logger.Desugar()) return err }