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

Use proquints for recovery codes #41379

Merged
merged 1 commit into from
May 9, 2024

Conversation

espadolini
Copy link
Contributor

@espadolini espadolini commented May 9, 2024

The github.com/sethvargo/go-diceware library that we use to generate friendly recovery codes unconditionally causes a ~1.5MB allocation at runtime, since it uses a map in a var; while this is a relatively small cost for a long running teleport process, this cost is incurred even by binaries that are intended to be much leaner (such as tsh or tbot) and that might end up running in multiple copies (hundreds or thousands) on a given machine. That fixed cost also happens on every new re-execution of teleport to handle shell execution, port forwarding and various other bits of functionality that must happen in a separate process - which is pretty silly, considering that most of those are not in the auth service, which is the only component that needs to generate recovery codes.

Adding to the aforementioned issue, #5685 is still very much a problem, and trimming one more dependency is worth in and of itself.

This PR thus proposes to change the recovery code generation to use proquints (PRO-nouncable QUINT-uplets of alternating unambiguous consonants and vowels, arXiv:0901.4016 [cs.SE]), which we've been successfully using in Cloud to generate random pronounceable identifiers for a while now. As a bonus, this makes the recovery codes slightly less guessable (from ~103 to 128 bits of entropy each).

Example of the account recovery UI after the change:
proquints-recovery

Existing recovery codes are stored hashed in the user recoverycodes resource, and will continue working as normal after this change.

Copy link
Collaborator

@zmb3 zmb3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 9, 2024
@espadolini espadolini added this pull request to the merge queue May 9, 2024
Merged via the queue into master with commit 384654a May 9, 2024
45 of 46 checks passed
@espadolini espadolini deleted the espadolini/recovery-codes-proquints branch May 9, 2024 18:27
@public-teleport-github-review-bot

@espadolini See the table below for backport results.

Branch Result
branch/v13 Failed
branch/v14 Failed
branch/v15 Failed

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

Successfully merging this pull request may close these issues.

4 participants