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

Encrypt TOTP keys #103

Merged
merged 13 commits into from
Apr 20, 2023
Merged

Encrypt TOTP keys #103

merged 13 commits into from
Apr 20, 2023

Conversation

dd32
Copy link
Member

@dd32 dd32 commented Apr 12, 2023

Fixes #101

Depends upon WordPress/wporg-mu-plugins#390
Made possible by WordPress/two-factor#546

NOTE: if you test this on a sandbox (and then remove this), you will be unable to login with TOTP again without deactivating TOTP.

Testing Instructions:

  • Make sure you're running a version of two-factor with Add a filter to filter the classname used for a provider two-factor#546 merged
  • Define WPORG_ENCRYPTION_KEY and optionally WPORG_TWO_FACTOR_ENCRYPTION_KEY, with a value from \WordPressdotorg\MU_Plugins\Encryption\generate_encryption_key()
  • View _two_factor_totp_key value. Validate that it appears plaintext.
  • Login with TOTP.
  • View _two_factor_totp_key value. Validate that it appears to be encrypted (Long, and prefixed with $t1$).
  • Deactivate TOTP.
  • Setup TOTP.
  • Validate everything worked as expected.

TODO:

  • Unit Tests.
  • Add Encryption methods. wporg-mu-plugins#390 (This is not required prior to this merging, but is required for it to actually encrypt)
  • Test once 390 is merged
  • Test on production sandbox, verify TOTPs are upgraded upon login.

@dd32 dd32 requested review from pkevan and iandunn and removed request for pkevan April 12, 2023 07:46
@dd32 dd32 self-assigned this Apr 12, 2023
@dd32 dd32 requested a review from pkevan April 12, 2023 07:46
tests/test-wporg-two-factor.php Outdated Show resolved Hide resolved
@pkevan
Copy link
Contributor

pkevan commented Apr 12, 2023

Tested on a sandbox, and confirmed the TOTP key is encrypted, and you can log in once it has been encrypted.

@dd32
Copy link
Member Author

dd32 commented Apr 13, 2023

I've updated this to include the User ID as part of the encryption, such that user_meta for one user is invalid for another.
(I've 'upgraded' the existing encrypted TOTP keys on WordPress.org).

I've updated the unit tests to test that the key is stored in a format different than the key (ie. encrypted) but left the validation of the encryption functions to the Encryption library PR.
edit: Of course this doesn't actually pass yet.. as the encryption methods aren't merged.. 3d89b20 can be reverted once the upstream PR is merged.

@dd32 dd32 merged commit aae7ec9 into trunk Apr 20, 2023
@dd32 dd32 deleted the add/encrypted-totp branch April 20, 2023 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace TOTP class with our own, to encrypt all the things
2 participants