feat: hardware key service by Joerger · Pull Request #53435 · gravitational/teleport

Joerger · 2025-03-26T01:20:14Z

Part of RFD 199

Structural changes:

Add the hardwarekey package with a new hardware key Service interface
- The hardware key service is held by the client store and passed during key parsing, rather than having the parsing logic default to the direct PIV implementation.
- Also moved attestation, hardware key prompt, and other strictly hardware key related code to this package
- In a follow up PR, I'll add the agent implementation
Add the piv package, which provides the PIV implementation of ^
- This was moved over from the previous yubikey.go direct implementation to fit into the service interface without any major changes, outside of those noted below.
Add hardwarekey.PrivateKey implementation of crypto.Signer. This is essentially a generalized version of the previous YubiKeyPrivateKey struct.

Functional changes:

Enrich the PEM encoded hardware private key file.
Add ctx to hardwarekey.Service to be used as the default context during signatures, since the crypto.Signer interface does not support ctx directly.
Replace global YubiKeyPrivateKey cache with a global YubiKey cache, which supports proper cross-cluster connection caching for Connect
Only attest the key when needed (at login) to improve performance. This shaves off ~500ms for each non-login command

Minor changes:

Add tests
Remove keys.HardwareSigner interface in favor of hardware.PrivateKey
Tidy up PIV slot handling with the PIVSlotKey and PIVSlotKeyString types

Follow ups (TODO comments):

Initialize client store and hardware key service at the start of tsh, tctl, and tshd, so that PIV connectionsand prompts can be shared (and without global caches/mutexes). Note that sharing the PIN caching will take place within the prompt, so sharing the prompt across the process will be necessary.
feat: Hardware Key Agent - set hardware key service in client store #53563
Remove rootClusterURI dependency from daemon service prompt, instead supplying this to the prompt from the key store (which has proxy/cluster/username known in the keyring index).

Note that the last couple commits were reverting the follow ups ^ in order to reduce the size of this PR.

* Replace CustomHardwareKeyPrompt in ParsePrivateKeyOpts with HardwareKeyService * Export CLIPrompt

…rdware key prompt in favor in favor of hardware key service with prompt field.

…rompt itself.

…ions noted below: * Replace global *YubiKeyPrivateKey cache with a global *YubiKey cache, which supports proper cross-cluster connection caching for Connect * Replace HardwareSigner interface with HardwarePrivateKey, since the service within the key is now the interface * Add context to signatures through the context passed to the hardware key service * Only attest the key when needed (login) to avoid its performance cost

…ateKey; Add tests for marshaling/parsing hardware private keys and hardware key methods.

…tual key info.

…nt store.

Joerger · 2025-03-31T16:47:58Z

+
+	attestationv1 "github.com/gravitational/teleport/api/gen/proto/go/attestation/v1"
+)
+


This file was moved from hardwaresigner.go, no changes.

Joerger · 2025-03-31T16:52:03Z

 	return privateKeyPolicyErrRegex.MatchString(err.Error())
 }
+
+// AttestationData is attested information about the hardware private key matching the public key.


Moved from hardwaresigner.go. Due to PrivateKeyPolicy dependency, it can't go in hardwarekey/attestation.go. We could consider moving PrivateKeyPolicy to the hardwarekey package in a follow up, 👍 if you'd like me to do that.

…Add todo for tsh test.

Joerger · 2025-04-01T20:43:20Z

@Tener @timothyb89 Friendly ping to review, tests should be passing now.

Tener · 2025-04-02T03:09:03Z

@Joerger sorry, I was travelling now is the first time I can take a look at this PR. Just looking at the description I feel it would make sense to split this PR to a bunch of smaller PRs to make the review easier and more focused. Let me know if this is makes sense; if not I can try to review this PR as is, but I feel it will actually make the overall review longer.

Joerger · 2025-04-02T19:21:25Z

@Joerger sorry, I was travelling now is the first time I can take a look at this PR. Just looking at the description I feel it would make sense to split this PR to a bunch of smaller PRs to make the review easier and more focused. Let me know if this is makes sense; if not I can try to review this PR as is, but I feel it will actually make the overall review longer.

Fair enough. A lot of the changes are interconnected, but I'll give a shot at splitting the change out further.

Joerger mentioned this pull request Mar 26, 2025

followup: remove hardwarekey package aliases #53467

Closed

Joerger force-pushed the joerger/hardware-key-service branch from f4eef27 to e53e243 Compare March 28, 2025 18:24

Joerger added 9 commits March 28, 2025 11:47

* Add HardwareKeyService interface and HardwarePrivateKey implementation

4779b47

* Replace CustomHardwareKeyPrompt in ParsePrivateKeyOpts with HardwareKeyService * Export CLIPrompt

Propogate hardware key service through client store; Remove custom ha…

66eab80

…rdware key prompt in favor in favor of hardware key service with prompt field.

Add key info to HardwareKeyPrompt args instead of setting it in the p…

7a21dc3

…rompt itself.

Add hardwarekey and piv packages.

2a36bb6

Cleanup.

8d7c4d8

Query hardware key for missing login key ref details.

72de3a1

Cleanup piv slot constants; Combine NewPrivateKey and NewSoftwarePriv…

596ef2b

…ateKey; Add tests for marshaling/parsing hardware private keys and hardware key methods.

Cleanup.

c2aae79

Joerger force-pushed the joerger/hardware-key-service branch 3 times, most recently from 4aacc7a to e3a0d9c Compare March 28, 2025 21:22

Joerger marked this pull request as ready for review March 28, 2025 21:27

github-actions Bot requested review from Tener and timothyb89 March 28, 2025 21:28

github-actions Bot added size/lg tsh tsh - Teleport's command line tool for logging into nodes running Teleport. labels Mar 28, 2025

Joerger force-pushed the joerger/hardware-key-service branch from e3a0d9c to 6bb5317 Compare March 28, 2025 21:37

Joerger added the no-changelog Indicates that a PR does not require a changelog entry label Mar 28, 2025

Joerger mentioned this pull request Mar 28, 2025

feat: Hardware Key Agent - set hardware key service in client store #53563

Merged

Revert setting shared hardware key service in teleterm; revert contex…

abbf6c4

…tual key info.

Joerger force-pushed the joerger/hardware-key-service branch from 6bb5317 to 596bac7 Compare March 28, 2025 23:55

Joerger added 3 commits March 28, 2025 16:59

Revert renaming NewSoftwarePrivateKey to NewPrivateKey.

159dc8b

Revert replacing prompt in keystore with hardware key service in clie…

01c05bd

…nt store.

Fix tests.

0552e33

Joerger force-pushed the joerger/hardware-key-service branch from 596bac7 to 0552e33 Compare March 29, 2025 00:30

Joerger commented Mar 31, 2025

View reviewed changes

Rename piv service files.

d8daa45

Joerger mentioned this pull request Mar 31, 2025

Rename NewSoftwarePrivateKey to NewPrivateKey #53598

Merged

Joerger added 2 commits March 31, 2025 15:59

Add MockHardwareKeyService.

66521ce

Use mock service in privatekey_test.go and remove pivtest build tag; …

7fe86f4

…Add todo for tsh test.

Joerger force-pushed the joerger/hardware-key-service branch from 5c23246 to 7fe86f4 Compare March 31, 2025 22:59

Add more tests using the mocked hardware key service implementation.

172ee50

Joerger force-pushed the joerger/hardware-key-service branch from ef7b2ff to d14db39 Compare April 2, 2025 00:39

fix or remove broken tests.

cfcb9fb

Joerger force-pushed the joerger/hardware-key-service branch from d14db39 to cfcb9fb Compare April 2, 2025 01:03

Joerger mentioned this pull request Apr 2, 2025

feat: Hardware Key Agent - Add api/utils/keys/hardwarekey package #53671

Merged

Joerger closed this Apr 2, 2025

Joerger mentioned this pull request Apr 2, 2025

feat: Hardware Key Agent - Add hardwarekey.Service interface with adapted PIV implementation #53674

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: hardware key service#53435

feat: hardware key service#53435
Joerger wants to merge 18 commits intomasterfrom
joerger/hardware-key-service

Joerger commented Mar 26, 2025 •

edited

Loading

Uh oh!

Joerger Mar 31, 2025 •

edited

Loading

Uh oh!

Joerger Mar 31, 2025

Uh oh!

Joerger commented Apr 1, 2025

Uh oh!

Tener commented Apr 2, 2025 •

edited

Loading

Uh oh!

Joerger commented Apr 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		attestationv1 "github.com/gravitational/teleport/api/gen/proto/go/attestation/v1"
		)

Conversation

Joerger commented Mar 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Joerger Mar 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Joerger Mar 31, 2025

Choose a reason for hiding this comment

Uh oh!

Joerger commented Apr 1, 2025

Uh oh!

Tener commented Apr 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Joerger commented Apr 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Joerger commented Mar 26, 2025 •

edited

Loading

Joerger Mar 31, 2025 •

edited

Loading

Tener commented Apr 2, 2025 •

edited

Loading