Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
r-maerz opened this issue Jun 3, 2024 · 4 comments
Open

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

r-maerz opened this issue Jun 3, 2024 · 4 comments

Comments

@r-maerz
Copy link

r-maerz commented Jun 3, 2024

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
Copy link
Member

maximbaz commented Jun 4, 2024

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

if (helpers.getSetting("enableOTP", login, settings) && login.fields.hasOwnProperty("otp")) {

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
Copy link
Member

maximbaz commented Jun 4, 2024

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
Copy link
Author

r-maerz commented Jun 4, 2024

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

if (helpers.getSetting("enableOTP", login, settings) && login.fields.hasOwnProperty("otp")) {

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
Copy link
Member

maximbaz commented Jun 4, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants