Skip to content

Commit

Permalink
[v14] Cache PIV connections to share across the program execution (#4…
Browse files Browse the repository at this point in the history
…7954)

* Cache PIV connections to share across the program execution (#47091)

* Cache yubikey objects.

* Cache PIV connections to share across the program execution.

* Do not release the connection until `sign` returns

* Do not ignore errors

* Perform a "warm up" call to YubiKey

* Fix tests

* Use a specific interface to check if the key can be "warmed up"

* Allow abandoning `signer.Sign` call when context is canceled

* Make sure that the cached key is valid for the given private key policy

The reason for adding this check was failing `invalid key policies` test.

* Make `hardwareKeyWarmer` private

* Force callers to release connection

* Improve comments

* Fix lint

* Improve `connect` comment

* Fix race condition

* Simplify `release` logic

* Trigger license/cla

---------

Co-authored-by: joerger <[email protected]>

(cherry picked from commit bd6fdbf)

* Sign a hashed message in hardware key warmup call (#48206)

Otherwise, signing may fail with "input must be a hashed message" error.

(cherry picked from commit 47494db)

* Remove delayed closing of yubikey connection to prevent the connection from leaking after program execution. (#48414)

(cherry picked from commit b7c0e79)

---------

Co-authored-by: Brian Joerger <[email protected]>
  • Loading branch information
gzdunek and Joerger authored Nov 21, 2024
1 parent 1a723b9 commit 53a7e23
Show file tree
Hide file tree
Showing 3 changed files with 412 additions and 203 deletions.
25 changes: 23 additions & 2 deletions api/client/proxy/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ type ClientConfig struct {

// CheckAndSetDefaults ensures required options are present and
// sets the default value of any that are omitted.
func (c *ClientConfig) CheckAndSetDefaults() error {
func (c *ClientConfig) CheckAndSetDefaults(ctx context.Context) error {
if c.ProxyAddress == "" {
return trace.BadParameter("missing required parameter ProxyAddress")
}
Expand Down Expand Up @@ -116,6 +116,21 @@ func (c *ClientConfig) CheckAndSetDefaults() error {
// not to send the client certificate by looking at certificate request.
if len(tlsCfg.Certificates) > 0 {
cert := tlsCfg.Certificates[0]

// When a hardware key is used to store the private key, the user may fail to provide
// a PIN or touch before a gRPC dial timeout occurs.
// The resulting "dial timeout" error is generic and doesn't indicate an issue with the
// hardware key itself (since YubiKey is treated like any other key).
// To avoid this, we perform a "warm-up" call to the key, ensuring it is ready
// before initiating the gRPC dial.
// This approach works because the connection is cached for a few seconds,
// allowing subsequent calls without requiring additional user action.
if priv, ok := cert.PrivateKey.(hardwareKeyWarmer); ok {
err := priv.WarmupHardwareKey(ctx)
if err != nil {
return nil, trace.Wrap(err)
}
}
tlsCfg.Certificates = nil
tlsCfg.GetClientCertificate = func(_ *tls.CertificateRequestInfo) (*tls.Certificate, error) {
return &cert, nil
Expand Down Expand Up @@ -183,7 +198,7 @@ const protocolProxySSHGRPC string = "teleport-proxy-ssh-grpc"
// of the caller, then prefer to use NewSSHClient instead which omits
// the gRPC dialing altogether.
func NewClient(ctx context.Context, cfg ClientConfig) (*Client, error) {
if err := cfg.CheckAndSetDefaults(); err != nil {
if err := cfg.CheckAndSetDefaults(ctx); err != nil {
return nil, trace.Wrap(err)
}

Expand Down Expand Up @@ -439,3 +454,9 @@ func (c *Client) Ping(ctx context.Context) error {
_, _ = clt.Ping(ctx, &authpb.PingRequest{})
return nil
}

// hardwareKeyWarmer performs a bogus call to the hardware key,
// to proactively prompt the user for a PIN/touch (if needed).
type hardwareKeyWarmer interface {
WarmupHardwareKey(ctx context.Context) error
}
Loading

0 comments on commit 53a7e23

Please sign in to comment.