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

Support custom hardware key prompt #47273

Merged
merged 9 commits into from
Oct 16, 2024
Merged

Conversation

gzdunek
Copy link
Contributor

@gzdunek gzdunek commented Oct 7, 2024

This PR adds an ability to pass a custom hardware key prompt to functions that talk to YubiKey.
tsh now uses a cliPrompt; Teleport Connect will have its own prompt, shown as a modal in the app.

The prompt interface is defined in yubikey_common.go. I extracted four methods from yubikey.go: AskPIN, Touch, ChangePIN and ConformSlotOverwrite. When the prompt message is generated dynamically (like in ConformSlotOverwrite) it can be passed to the method. Otherwise, the implementer has to provide the message.

Converting most of the prompts to work with the new interface was fairly straightforward. The only significant change is ChangePIN. Previously, it set PUK directly after prompting for it, now I have to gather all data upfront (PIN & PUK), and then call the yubkiey functions.
Because of that, I check on the implementer side if the provided PUK is empty or default, ask for it, and then set PINAndPUK.PUKChanged flag so that I know if the PUK on the key should be changed.

@gzdunek gzdunek added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v16 labels Oct 7, 2024
api/utils/keys/privatekey.go Outdated Show resolved Hide resolved
@gzdunek gzdunek marked this pull request as ready for review October 7, 2024 14:55
Copy link
Contributor

@Joerger Joerger left a comment

Choose a reason for hiding this comment

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

Nice add! LGTM just some minor nits

api/utils/keys/privatekey.go Outdated Show resolved Hide resolved
api/utils/keys/privatekey.go Outdated Show resolved Hide resolved
lib/client/keystore.go Show resolved Hide resolved
lib/client/keystore.go Show resolved Hide resolved
api/utils/keys/yubikey_common.go Show resolved Hide resolved
@gzdunek gzdunek requested a review from Joerger October 9, 2024 11:36
Base automatically changed from gzdunek/concurrent-yubikey-access to master October 16, 2024 08:20
@gzdunek gzdunek force-pushed the gzdunek/custom-hardware-key-prompt branch from c6e2be0 to b884781 Compare October 16, 2024 08:31
@gzdunek gzdunek added this pull request to the merge queue Oct 16, 2024
Merged via the queue into master with commit 95af4c1 Oct 16, 2024
40 checks passed
@gzdunek gzdunek deleted the gzdunek/custom-hardware-key-prompt branch October 16, 2024 10:17
@public-teleport-github-review-bot

@gzdunek See the table below for backport results.

Branch Result
branch/v16 Failed

mvbrock pushed a commit that referenced this pull request Oct 16, 2024
* Allow passing custom prompt to YubiKey

* Handle `prompt.Touch` cancellation

* Pass `HardwareKeyPrompt` through all the layers

* Add an empty `HardwareKeyPromptConstructor` to Connect

* Remove `ParsePrivateKeyWithCustomPrompt`

* Add missing godoc

* Fix teleterm tests

* Include `cliprompt.go` only for `go:build piv && !pivtest`

* Lint and test fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/branch/v16 no-changelog Indicates that a PR does not require a changelog entry size/md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants