From 88fd5c2b4797860c71bdc13ef3f8c733214e752d Mon Sep 17 00:00:00 2001 From: Krzysztof Tomecki <152964795+chris-4chain@users.noreply.github.com> Date: Wed, 24 Apr 2024 13:47:29 +0200 Subject: [PATCH 1/2] feat(SPV-679): directed secret --- contacts.go | 4 ++-- contacts_test.go | 56 +++++++++++++++++++++++++++----------------- totp.go | 34 +++++++++++++++++++-------- totp_test.go | 60 +++++++++++++++++++++++++++++++++++++++++------- 4 files changed, 113 insertions(+), 41 deletions(-) diff --git a/contacts.go b/contacts.go index 8ddc9b35..989270e4 100644 --- a/contacts.go +++ b/contacts.go @@ -30,8 +30,8 @@ func (b *WalletClient) RejectContact(ctx context.Context, paymail string) transp } // ConfirmContact will try to confirm the contact -func (b *WalletClient) ConfirmContact(ctx context.Context, contact *models.Contact, passcode string, period, digits uint) transports.ResponseError { - isTotpValid, err := b.ValidateTotpForContact(contact, passcode, period, digits) +func (b *WalletClient) ConfirmContact(ctx context.Context, contact *models.Contact, passcode, requesterPaymail string, period, digits uint) transports.ResponseError { + isTotpValid, err := b.ValidateTotpForContact(contact, passcode, requesterPaymail, period, digits) if err != nil { return transports.WrapError(fmt.Errorf("totp validation failed: %w", err)) } diff --git a/contacts_test.go b/contacts_test.go index 80226f98..3ed81fc3 100644 --- a/contacts_test.go +++ b/contacts_test.go @@ -2,10 +2,10 @@ package walletclient import ( "context" + "strings" "testing" "github.com/bitcoin-sv/spv-wallet-go-client/fixtures" - "github.com/bitcoin-sv/spv-wallet/models" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -57,17 +57,6 @@ func TestContactActionsRouting(t *testing.T) { return err }, }, - { - name: "ConfirmContact", - route: "/contact/confirmed/", - responsePayload: "{}", - f: func(c *WalletClient) error { - contact := models.Contact{PubKey: fixtures.PubKey} - - passcode, _ := c.GenerateTotpForContact(&contact, 30, 2) - return c.ConfirmContact(context.Background(), &contact, passcode, 30, 2) - }, - }, } for _, tc := range tcs { @@ -104,14 +93,18 @@ func TestConfirmContact(t *testing.T) { Client: WithHTTPClient, } - client := getTestWalletClientWithOpts(tmq, WithXPriv(fixtures.XPrivString)) + clientMaker := func(opts ...ClientOps) (*WalletClient, error) { + return getTestWalletClientWithOpts(tmq, opts...), nil + } + + alice := makeMockUser("alice", clientMaker) + bob := makeMockUser("bob", clientMaker) - contact := &models.Contact{PubKey: fixtures.PubKey} - totp, err := client.GenerateTotpForContact(contact, 30, 2) + totp, err := alice.client.GenerateTotpForContact(bob.contact, 30, 2) require.NoError(t, err) // when - result := client.ConfirmContact(context.Background(), contact, totp, 30, 2) + result := bob.client.ConfirmContact(context.Background(), alice.contact, totp, bob.paymail, 30, 2) // then require.Nil(t, result) @@ -127,19 +120,40 @@ func TestConfirmContact(t *testing.T) { Client: WithHTTPClient, } - client := getTestWalletClientWithOpts(tmq, WithXPriv(fixtures.XPrivString)) + clientMaker := func(opts ...ClientOps) (*WalletClient, error) { + return getTestWalletClientWithOpts(tmq, opts...), nil + } + + alice := makeMockUser("alice", clientMaker) + bob := makeMockUser("bob", clientMaker) - alice := &models.Contact{PubKey: "034252e5359a1de3b8ec08e6c29b80594e88fb47e6ae9ce65ee5a94f0d371d2cde"} - a_totp, err := client.GenerateTotpForContact(alice, 30, 2) + totp, err := alice.client.GenerateTotpForContact(bob.contact, 30, 2) require.NoError(t, err) - bob := &models.Contact{PubKey: "02dde493752f7bc89822ed8a13e0e4aa04550c6c4430800e4be1e5e5c2556cf65b"} + //make sure the wrongTotp is not the same as the generated one + wrongTotp := incrementDigits(totp) //the length should remain the same // when - result := client.ConfirmContact(context.Background(), bob, a_totp, 30, 2) + result := bob.client.ConfirmContact(context.Background(), alice.contact, wrongTotp, bob.paymail, 30, 2) // then require.NotNil(t, result) require.Equal(t, result.Error(), "totp is invalid") }) } + +// incrementDigits takes a string of digits and increments each digit by 1. +// Digits wrap around such that '9' becomes '0'. +func incrementDigits(input string) string { + var result strings.Builder + + for _, c := range input { + if c == '9' { + result.WriteRune('0') + } else { + result.WriteRune(c + 1) + } + } + + return result.String() +} diff --git a/totp.go b/totp.go index ef7c571a..559f6bf0 100644 --- a/totp.go +++ b/totp.go @@ -19,42 +19,51 @@ import ( var ErrClientInitNoXpriv = errors.New("init client with xPriv first") const ( - // Default number of seconds a TOTP is valid for. + // TotpDefaultPeriod - Default number of seconds a TOTP is valid for. TotpDefaultPeriod uint = 30 - // Default TOTP length + // TotpDefaultDigits - Default TOTP length TotpDefaultDigits uint = 2 ) +/* +Basic flow: +Alice generates passcodeForBob with (sharedSecret+(contact.Paymail as bobPaymail)) +Alice sends passcodeForBob to Bob (e.g. via email) +Bob validates passcodeForBob with (sharedSecret+(requesterPaymail as bobPaymail)) +The (sharedSecret+paymail) is a "directedSecret". It prevents that passcodeForBob-from-Alice != passcodeForAlice-from-Bob. +The flow looks the same for Bob generating passcodeForAlice. +*/ + // GenerateTotpForContact creates one time-based one-time password based on secret shared between the user and the contact func (b *WalletClient) GenerateTotpForContact(contact *models.Contact, period, digits uint) (string, error) { - secret, err := sharedSecret(b, contact) + sharedSecret, err := makeSharedSecret(b, contact) if err != nil { return "", err } opts := getTotpOpts(period, digits) - return totp.GenerateCodeCustom(string(secret), time.Now(), *opts) + return totp.GenerateCodeCustom(directedSecret(sharedSecret, contact.Paymail), time.Now(), *opts) } // ValidateTotpForContact validates one time-based one-time password based on secret shared between the user and the contact -func (b *WalletClient) ValidateTotpForContact(contact *models.Contact, passcode string, period, digits uint) (bool, error) { - secret, err := sharedSecret(b, contact) +func (b *WalletClient) ValidateTotpForContact(contact *models.Contact, passcode, requesterPaymail string, period, digits uint) (bool, error) { + sharedSecret, err := makeSharedSecret(b, contact) if err != nil { return false, err } opts := getTotpOpts(period, digits) - return totp.ValidateCustom(passcode, string(secret), time.Now(), *opts) + return totp.ValidateCustom(passcode, directedSecret(sharedSecret, requesterPaymail), time.Now(), *opts) } -func sharedSecret(b *WalletClient, c *models.Contact) (string, error) { +func makeSharedSecret(b *WalletClient, c *models.Contact) ([]byte, error) { privKey, pubKey, err := getSharedSecretFactors(b, c) if err != nil { - return "", err + return nil, err } x, _ := bec.S256().ScalarMult(pubKey.X, pubKey.Y, privKey.D.Bytes()) - return base32.StdEncoding.EncodeToString(x.Bytes()), nil + return x.Bytes(), nil } func getTotpOpts(period, digits uint) *totp.ValidateOpts { @@ -115,3 +124,8 @@ func convertPubKey(pubKey string) (*bec.PublicKey, error) { return bec.ParsePubKey(hex, bec.S256()) } + +// directedSecret appends a paymail to the secret and encodes it into base32 string +func directedSecret(sharedSecret []byte, paymail string) string { + return base32.StdEncoding.EncodeToString(append(sharedSecret, []byte(paymail)...)) +} diff --git a/totp_test.go b/totp_test.go index f528d5fc..0eccd6eb 100644 --- a/totp_test.go +++ b/totp_test.go @@ -1,10 +1,13 @@ package walletclient import ( + "encoding/hex" "testing" "github.com/bitcoin-sv/spv-wallet-go-client/fixtures" + "github.com/bitcoin-sv/spv-wallet-go-client/xpriv" "github.com/bitcoin-sv/spv-wallet/models" + "github.com/libsv/go-bk/bip32" "github.com/stretchr/testify/require" ) @@ -55,15 +58,18 @@ func TestGenerateTotpForContact(t *testing.T) { func TestValidateTotpForContact(t *testing.T) { t.Run("success", func(t *testing.T) { // given - sut, err := New(WithXPriv(fixtures.XPrivString), WithHTTP("localhost:3001")) - require.NoError(t, err) - - contact := models.Contact{PubKey: fixtures.PubKey} - pass, err := sut.GenerateTotpForContact(&contact, 30, 2) + clientMaker := func(opts ...ClientOps) (*WalletClient, error) { + allOptions := append(opts, WithHTTP("localhost:3001")) + return New(allOptions...) + } + alice := makeMockUser("alice", clientMaker) + bob := makeMockUser("bob", clientMaker) + + pass, err := alice.client.GenerateTotpForContact(bob.contact, 3600, 2) require.NoError(t, err) // when - result, err := sut.ValidateTotpForContact(&contact, pass, 30, 2) + result, err := bob.client.ValidateTotpForContact(alice.contact, pass, bob.paymail, 3600, 2) // then require.NoError(t, err) @@ -76,7 +82,7 @@ func TestValidateTotpForContact(t *testing.T) { require.NoError(t, err) // when - _, err = sut.ValidateTotpForContact(nil, "", 30, 2) + _, err = sut.ValidateTotpForContact(nil, "", fixtures.PaymailAddress, 30, 2) // then require.ErrorIs(t, err, ErrClientInitNoXpriv) @@ -90,10 +96,48 @@ func TestValidateTotpForContact(t *testing.T) { contact := models.Contact{PubKey: "invalid-pk-format"} // when - _, err = sut.ValidateTotpForContact(&contact, "", 30, 2) + _, err = sut.ValidateTotpForContact(&contact, "", fixtures.PaymailAddress, 30, 2) // then require.ErrorContains(t, err, "contact's PubKey is invalid:") }) } + +type mockUser struct { + contact *models.Contact + client *WalletClient + paymail string +} + +func makeMockUser(name string, clientMaker func(opts ...ClientOps) (*WalletClient, error)) mockUser { + keys, _ := xpriv.Generate() + paymail := name + "@example.com" + client, _ := clientMaker(WithXPriv(keys.XPriv())) + pki := makeMockPKI(keys.XPub().String()) + contact := models.Contact{PubKey: pki, Paymail: paymail} + return mockUser{ + contact: &contact, + client: client, + paymail: paymail, + } +} + +func makeMockPKI(xpub string) string { + xPub, _ := bip32.NewKeyFromString(xpub) + magicNumberOfInheritance := 3 //2+1; 2: because of the way spv-wallet stores xpubs in db; 1: to make a PKI + var err error + for i := 0; i < magicNumberOfInheritance; i++ { + xPub, err = xPub.Child(0) + if err != nil { + panic(err) + } + } + + pubKey, err := xPub.ECPubKey() + if err != nil { + panic(err) + } + + return hex.EncodeToString(pubKey.SerialiseCompressed()) +} From eed60769371069bae14df8fa9e9811c67e0b0928 Mon Sep 17 00:00:00 2001 From: Krzysztof Tomecki <152964795+chris-4chain@users.noreply.github.com> Date: Wed, 24 Apr 2024 13:52:41 +0200 Subject: [PATCH 2/2] feat(SPV-679): a comment adjustment --- totp.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/totp.go b/totp.go index 559f6bf0..eac2cee0 100644 --- a/totp.go +++ b/totp.go @@ -30,7 +30,7 @@ Basic flow: Alice generates passcodeForBob with (sharedSecret+(contact.Paymail as bobPaymail)) Alice sends passcodeForBob to Bob (e.g. via email) Bob validates passcodeForBob with (sharedSecret+(requesterPaymail as bobPaymail)) -The (sharedSecret+paymail) is a "directedSecret". It prevents that passcodeForBob-from-Alice != passcodeForAlice-from-Bob. +The (sharedSecret+paymail) is a "directedSecret". This ensures that passcodeForBob-from-Alice != passcodeForAlice-from-Bob. The flow looks the same for Bob generating passcodeForAlice. */