bitcoin-sv / ts-sdk

Other
51 stars 12 forks source link

TOTP implementation #94

Closed chris-4chain closed 5 months ago

chris-4chain commented 5 months ago

Hello :) According to our conversation with @sirdeggen I'm putting here the entire TOTP for PIKE algorithm from spv-wallet-js-client. It's a draft which might be a good starting point to discuss what to move to ts-sdk and what to keep in spv-wallet-js-client (regarding TOTP stuff).

Context

We faced some issues during implementation of TOTP codes for javascript client. First attempt was to just use some OTP library, like otplib - and it worked fine, but only for node.js (and web - depends on what lib we were trying). However, it didn't work on react-native mobile app. There were issues with Buffer and usage of crypto/crypto-js. Of course, polyfills would solve these issues - but we didn't like this idea.

Implementation

Due to these challenges, and since ts-sdk already had the necessary core algorithms (SHA1/SHA256/SHA512) implemented, we decided to implement our own TOTP algorithm.

One thing was missing: HMAC for SHA1 algorithm, which is a default one for TOTP codes - so we implemented it, based on the code for SHA256HMAC and with usage of already implemented SHA1.

You can check the implementation here: https://github.com/bitcoin-sv/spv-wallet-js-client/tree/main/src/totp

What's inside

Adjustments

Just for the record. The code from spv-wallet-js-client slightly differs from what is in this PR, because I've made some adjustments to make it work within this project.

sirdeggen commented 5 months ago

@ty-everett keen for your input.

I think TOTP is a useful method to add to the SDK as it solves a core problem - validation of remote counterparties when using hosted services which are subject to man in the middle attacks.

I recommend we add totp.ts but leave the PIKE specific implementation to the SPV Wallet codebase since it uses xPrivs which I'd rather change there than introduce here.

I'll review and contribute any suggestions I have in the coming days.

ty-everett commented 5 months ago

I feel this may be beyond the scope of this project (scope creep), as it feels implementation specific. If we support it here, we might misalign with other ways of doing similar things, and we're also implicitly committing to maintain this across at least Python and Go in the future.

Edit: Please see my below comment, after further discussions I now believe this would make a sound addition to the library.