search

browserpass / browserpass-extension

Browserpass web extension
ISC License
825 stars 50 forks source link

[Info needed] Rework OTP support to support steam #349

Open r-maerz opened 4 weeks ago

r-maerz commented 4 weeks ago

Hi,

I am the maintainer of pass-extension-totp which uses plain openssl to implement standard TOTP as well as Steam's custom algorithm. I would like to port my work to browserpass for it to become multi-plattform.

I see a number of benefits for browserpass and its users by doing this:

  1. Independence of outdated dependency "otplib", which is abandonware by now
  2. Support for Steam TOTP
  3. No changes to existing functionality or behavior

To get started, I had a look through your code and via #229 found my way to background.js as well as helper.js. What I still need info on is the following two questions:

  1. Is it possible to pass the name / path of the pass-file into the makeTOTP function? The reason for this is that my algorith distinguishes "normal" TOTP and "steam" TOTP by path. It generates normal tokens by default unless the pass-file is located in a steam sub-folder, such as steam/accountname.gpg
  2. So far I was unable to locate where exactly you extract the *otp-secret-line from the pass-file. According to your readme, you already support "totp" and "otpauth" as keys for matching. I would like to add "totp_secret" as a recognized key to this list.

Tagging @erayd as the original implementer of browserpass' OTP support in hopes they see this.

Kind regards, Robert

maximbaz commented 4 weeks ago

Hello! Starting from the second question, all the parsing is happening in parseFields function, in particular I think you are looking for this block:

https://github.com/browserpass/browserpass-extension/blob/dc71d472a54ca588fb34f67fec7244595c62af74/src/background.js#L1034

For the first point, I'd love to avoid imposing specific rules for naming pass entries, could we instead recognize a new optional field inside the pass entry? For example totp-method: steam or something like that?

maximbaz commented 4 weeks ago

It's also important to consider that a runtime dependency on openssl would be a hard sell, as we don't control the packaging and have to support a large variety of OS, including Windows and macOS...

r-maerz commented 4 weeks ago

Hello! Starting from the second question, all the parsing is happening in parseFields function, in particular I think you are looking for this block:

https://github.com/browserpass/browserpass-extension/blob/dc71d472a54ca588fb34f67fec7244595c62af74/src/background.js#L1034

For the first point, I'd love to avoid imposing specific rules for naming pass entries, could we instead recognize a new optional field inside the pass entry? For example totp-method: steam or something like that?

Thank you very much for this! I like the idea of an optional field a lot, because that will result in even fewer changes to existing app behavior. I will have a look at the code you quoted and try to wriggle my way through. My JavaScript is quite rusty.

Regarding the runtime dependency, this can actually be shipped in pure JavaScript - see for example crypto-browserify - or via the Web Crypto API. No need to have openssl installed on the client itself.

maximbaz commented 4 weeks ago

Awesome! Looking forward to seeing what you come up with! 🥳