Skip to content

Commit

Permalink
Remove semaphores from hardware key prompts
Browse files Browse the repository at this point in the history
  • Loading branch information
gzdunek committed Nov 12, 2024
1 parent 5506b80 commit c40578e
Showing 1 changed file with 14 additions and 16 deletions.
30 changes: 14 additions & 16 deletions lib/teleterm/daemon/hardwarekeyprompt.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,20 @@ import (

// NewHardwareKeyPromptConstructor returns a new hardware key prompt constructor
// for this service and the given root cluster URI.
// TODO(gzdunek): Improve multi-cluster and multi-hardware keys support.
// The code in yubikey.go doesn't really support using multiple hardware keys (like one per cluster):
// 1. We don't offer a choice which key should be used on the initial login.
// 2. Keys are cached per slot, not per physical key - it's not possible to use different keys with the same slot.
//
// Additionally, using the same hardware key for two clusters is not ideal too.
// Since we cache the keys per slot, if two clusters specify the same one,
// the user will always see the prompt for the same cluster URI. For example, if you are logged into both
// cluster-a and cluster-b, the prompt will always say "Unlock hardware key to access cluster-b."
// It seems that the better option would be to have a prompt per physical key, not per cluster.
// But I will leave that for the future, it's hard to say how common these scenarios will be in Connect.
//
// Because of the above, we don't have any mutex here. I don't expect receiving prompts
// from different hardware keys at the same time.
func (s *Service) NewHardwareKeyPromptConstructor(rootClusterURI uri.ResourceURI) keys.HardwareKeyPrompt {
return &hardwareKeyPrompter{s: s, rootClusterURI: rootClusterURI}
}
Expand All @@ -41,10 +55,6 @@ type hardwareKeyPrompter struct {

// Touch prompts the user to touch the hardware key.
func (h *hardwareKeyPrompter) Touch(ctx context.Context) error {
if err := h.s.singleImportantModalSemaphore.Acquire(ctx); err != nil {
return trace.Wrap(err)
}
defer h.s.singleImportantModalSemaphore.Release()
_, err := h.s.tshdEventsClient.PromptHardwareKeyTouch(ctx, &api.PromptHardwareKeyTouchRequest{
RootClusterUri: h.rootClusterURI.String(),
})
Expand All @@ -56,10 +66,6 @@ func (h *hardwareKeyPrompter) Touch(ctx context.Context) error {

// AskPIN prompts the user for a PIN.
func (h *hardwareKeyPrompter) AskPIN(ctx context.Context, requirement keys.PINPromptRequirement) (string, error) {
if err := h.s.singleImportantModalSemaphore.Acquire(ctx); err != nil {
return "", trace.Wrap(err)
}
defer h.s.singleImportantModalSemaphore.Release()
res, err := h.s.tshdEventsClient.PromptHardwareKeyPIN(ctx, &api.PromptHardwareKeyPINRequest{
RootClusterUri: h.rootClusterURI.String(),
PinOptional: requirement == keys.PINOptional,
Expand All @@ -74,10 +80,6 @@ func (h *hardwareKeyPrompter) AskPIN(ctx context.Context, requirement keys.PINPr
// The Electron app prompt must handle default values for PIN and PUK,
// preventing the user from submitting empty/default values.
func (h *hardwareKeyPrompter) ChangePIN(ctx context.Context) (*keys.PINAndPUK, error) {
if err := h.s.singleImportantModalSemaphore.Acquire(ctx); err != nil {
return nil, trace.Wrap(err)
}
defer h.s.singleImportantModalSemaphore.Release()
res, err := h.s.tshdEventsClient.PromptHardwareKeyPINChange(ctx, &api.PromptHardwareKeyPINChangeRequest{
RootClusterUri: h.rootClusterURI.String(),
})
Expand All @@ -93,10 +95,6 @@ func (h *hardwareKeyPrompter) ChangePIN(ctx context.Context) (*keys.PINAndPUK, e

// ConfirmSlotOverwrite asks the user if the slot's private key and certificate can be overridden.
func (h *hardwareKeyPrompter) ConfirmSlotOverwrite(ctx context.Context, message string) (bool, error) {
if err := h.s.singleImportantModalSemaphore.Acquire(ctx); err != nil {
return false, trace.Wrap(err)
}
defer h.s.singleImportantModalSemaphore.Release()
res, err := h.s.tshdEventsClient.ConfirmHardwareKeySlotOverwrite(ctx, &api.ConfirmHardwareKeySlotOverwriteRequest{
RootClusterUri: h.rootClusterURI.String(),
Message: message,
Expand Down

0 comments on commit c40578e

Please sign in to comment.