frostfs: Fix invalid signatures issued by key from json

Ephemeral keys worked fine while keys loaded from filesystem would
generate invalid signatures. This was caused by destroying private key
material during calls to Wallet.Close() and Account.Close(). Since these
calls do nothing except wiping the private key, we omit them now.
Responsibility for private key security is delegated to caller of getKey()

Signed-off-by: Vitaliy Potyarkin <v.potyarkin@yadro.com>
This commit is contained in:
Vitaliy Potyarkin 2024-10-16 12:05:26 +03:00
parent 254983fbe2
commit 9ff9d5be25
2 changed files with 88 additions and 16 deletions

View file

@ -5,6 +5,7 @@ import (
"crypto/ecdsa" "crypto/ecdsa"
"crypto/elliptic" "crypto/elliptic"
"crypto/rand" "crypto/rand"
"errors"
"fmt" "fmt"
"time" "time"
@ -73,6 +74,9 @@ func (s *Storage) Save(ctx context.Context, data []byte, attr ...string) (oid st
if err != nil { if err != nil {
return "", fmt.Errorf("signing object: %w", err) return "", fmt.Errorf("signing object: %w", err)
} }
if !obj.VerifyIDSignature() {
return "", errors.New("signing object: invalid signature was generated")
}
ctx, cancel := context.WithTimeout(ctx, storageRequestTimeout) ctx, cancel := context.WithTimeout(ctx, storageRequestTimeout)
defer cancel() defer cancel()
@ -163,19 +167,21 @@ func keyval2attrs(attr ...string) ([]object.Attribute, error) {
} }
// Load private key from wallet file. // Load private key from wallet file.
func getKey(walletPath, walletAccount, walletPassword string) (*ecdsa.PrivateKey, error) { func getKey(walletPath, walletAccount, walletPassword string) (*ecdsa.PrivateKey, error) { //nolint:gocyclo
if walletPath == "" { if walletPath == "" {
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) // TODO: using ephemeral keys for now, later read from env vars key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
if err != nil { if err != nil {
return nil, fmt.Errorf("generating ephemeral key: %w", err) return nil, fmt.Errorf("generating ephemeral key: %w", err)
} }
return key, nil return key, nil
} }
// This function intentionally omits calls to w.Close() and account.Close()
// because that would destroy the underlying ecdsa.PrivateKey.
w, err := wallet.NewWalletFromFile(walletPath) w, err := wallet.NewWalletFromFile(walletPath)
if err != nil { if err != nil {
return nil, err return nil, err
} }
defer w.Close()
if len(w.Accounts) == 0 { if len(w.Accounts) == 0 {
return nil, fmt.Errorf("no accounts in wallet: %s", walletPath) return nil, fmt.Errorf("no accounts in wallet: %s", walletPath)
} }
@ -185,6 +191,12 @@ func getKey(walletPath, walletAccount, walletPassword string) (*ecdsa.PrivateKey
if err != nil { if err != nil {
return nil, fmt.Errorf("invalid account address: %w", err) return nil, fmt.Errorf("invalid account address: %w", err)
} }
if len(decode) != 21 {
return nil, fmt.Errorf("invalid account address length: %d bytes", len(decode))
}
if decode[0] != 0x35 {
return nil, fmt.Errorf("invalid account address first byte: %s -> %#x", walletAccount, decode[0])
}
hash, err := util.Uint160DecodeBytesBE(decode[1:]) hash, err := util.Uint160DecodeBytesBE(decode[1:])
if err != nil { if err != nil {
return nil, fmt.Errorf("invalid account hash: %w", err) return nil, fmt.Errorf("invalid account hash: %w", err)
@ -194,10 +206,11 @@ func getKey(walletPath, walletAccount, walletPassword string) (*ecdsa.PrivateKey
return nil, fmt.Errorf("account not found: %s", walletAccount) return nil, fmt.Errorf("account not found: %s", walletAccount)
} }
} }
defer account.Close() if account.PrivateKey() == nil {
err = account.Decrypt(walletPassword, w.Scrypt) err = account.Decrypt(walletPassword, w.Scrypt)
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to decrypt wallet: %w", err) return nil, fmt.Errorf("failed to decrypt wallet: %w", err)
}
} }
key := account.PrivateKey().PrivateKey key := account.PrivateKey().PrivateKey
return &key, nil return &key, nil

View file

@ -3,27 +3,32 @@ package frostfs
// Tests for our FrostFS client code // Tests for our FrostFS client code
import ( import (
"bytes"
"context" "context"
"crypto/ecdsa" "crypto/ecdsa"
"crypto/elliptic" "crypto/sha256"
"crypto/rand"
"fmt"
"os" "os"
"sync" "sync"
"testing" "testing"
"git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user" "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/user"
"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
"github.com/nspcc-dev/neo-go/pkg/wallet"
) )
// Initialize storage backend for tests. // Initialize storage backend for tests.
func openStorage(t *testing.T) *Storage { func openStorage(t *testing.T) *Storage {
t.Helper() t.Helper()
cid := os.Getenv("FROSTFS_CID") // sample: "348WWfBKbS79Wbmm38MRE3oBoEDM5Ga1XXbGKGNyisDM" cid := os.Getenv("FROSTFS_CONTAINER") // sample: "348WWfBKbS79Wbmm38MRE3oBoEDM5Ga1XXbGKGNyisDM"
endpoint := os.Getenv("FROSTFS_ENDPOINT") // sample: "grpc://localhost:8802" endpoint := os.Getenv("FROSTFS_ENDPOINT") // sample: "grpc://localhost:8802"
if cid == "" || endpoint == "" { if cid == "" || endpoint == "" {
t.Skipf("one or more environment variables not set: FROSTFS_ENDPOINT, FROSTFS_CID") t.Skipf("one or more environment variables not set: FROSTFS_ENDPOINT, FROSTFS_CONTAINER")
} }
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) // TODO: using ephemeral keys for now, later read from env vars key, err := getKey(
os.Getenv("FROSTFS_WALLET"),
os.Getenv("FROSTFS_WALLET_ACCOUNT"),
os.Getenv("FROSTFS_WALLET_PASSWORD"),
)
if err != nil { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@ -76,7 +81,8 @@ func TestMulipleObjects(t *testing.T) {
continue continue
} }
if obj != baseline { if obj != baseline {
panic(fmt.Errorf("non-identical object id: %s != %s", baseline, obj)) t.Errorf("non-identical object id: %s != %s", baseline, obj)
return
} }
} }
} }
@ -90,11 +96,18 @@ func TestMulipleObjects(t *testing.T) {
defer wg.Done() defer wg.Done()
oid, err := storage.Save(ctx, payload, "FileName", "CAS.txt", "Tag", "test") oid, err := storage.Save(ctx, payload, "FileName", "CAS.txt", "Tag", "test")
if err != nil { if err != nil {
panic(err) t.Error(err)
return
} }
err = storage.Delete(ctx, oid) err = storage.Delete(ctx, oid)
if err != nil { if err != nil {
panic(err) t.Error(err)
return
}
select {
case objects <- oid:
case <-ctx.Done():
return
} }
t.Log(oid) t.Log(oid)
}() }()
@ -129,3 +142,49 @@ func key2addr(k *ecdsa.PrivateKey) string {
user.IDFromKey(&owner, k.PublicKey) user.IDFromKey(&owner, k.PublicKey)
return owner.String() return owner.String()
} }
// Check that loaded wallet key generates valid signature.
func TestLoadWalletAndSign(t *testing.T) {
const (
walletPath = "client_test_wallet.json"
walletAccount = "NWZnjbTKbzwtX6w1q5R3kbEKrnJ5bp1kn7"
walletAccountPassword = ""
walletAccountIndex = 0
dataString = "some data to sign in this test"
)
w, err := wallet.NewWalletFromFile(walletPath)
if err != nil {
t.Fatalf("wallet from file: %v", err)
}
accountFromWallet := w.Accounts[walletAccountIndex]
err = accountFromWallet.Decrypt(walletAccountPassword, w.Scrypt)
if err != nil {
t.Fatalf("wallet decrypt: %v", err)
}
key, err := getKey(walletPath, walletAccount, walletAccountPassword)
if err != nil {
t.Fatal(err)
}
wrappedKey := &keys.PrivateKey{PrivateKey: *key}
accountFromKey := wallet.NewAccountFromPrivateKey(wrappedKey)
if !accountFromKey.PublicKey().Equal(accountFromWallet.PublicKey()) {
t.Fatalf("corrupted key: want %s, got %s", accountFromWallet.PublicKey().Address(), accountFromKey.PublicKey().Address())
}
data := []byte(dataString)
sig := accountFromKey.PrivateKey().Sign(data)
hash := sha256.Sum256(data)
hashSig := accountFromKey.PrivateKey().SignHash(hash)
if !bytes.Equal(sig, hashSig) {
t.Fatalf("different signatures for data (%x) and for hash (%x)", sig, hashSig)
}
ok := accountFromKey.PublicKey().Verify(sig, hash[:])
if !ok {
t.Errorf("signature %x (%d bytes): check failed: signing key", sig, len(sig))
}
ok = accountFromWallet.PublicKey().Verify(sig, hash[:])
if !ok {
t.Errorf("signature %x (%d bytes): check failed: wallet key", sig, len(sig))
}
}