diff --git a/kms/yubikey/yubikey.go b/kms/yubikey/yubikey.go index aaf61385..09b3d929 100644 --- a/kms/yubikey/yubikey.go +++ b/kms/yubikey/yubikey.go @@ -9,9 +9,11 @@ import ( "crypto/x509" "encoding/asn1" "encoding/hex" + "io" "net/url" "strconv" "strings" + "sync" "github.com/go-piv/piv-go/piv" "github.com/pkg/errors" @@ -261,7 +263,9 @@ func (k *YubiKey) CreateSigner(req *apiv1.CreateSignerRequest) (crypto.Signer, e if !ok { return nil, errors.New("private key is not a crypto.Signer") } - return signer, nil + return &syncSigner{ + Signer: signer, + }, nil } // CreateDecrypter creates a crypto.Decrypter using the key present in the configured @@ -297,7 +301,9 @@ func (k *YubiKey) CreateDecrypter(req *apiv1.CreateDecrypterRequest) (crypto.Dec if !ok { return nil, errors.New("private key is not a crypto.Decrypter") } - return decrypter, nil + return &syncDecrypter{ + Decrypter: decrypter, + }, nil } // CreateAttestation creates an attestation certificate from a YubiKey slot. @@ -491,4 +497,39 @@ func getSerialNumber(cert *x509.Certificate) string { return "" } +// Common mutex used in syncSigner and syncDecrypter. A sync.Mutex cannot be +// copied after the first use. +// +// By using it, synchronization becomes easier and avoids conflicts between the +// two goroutines accessing the shared resources. +// +// This is not optimal if more than one YubiKey is used, but the overhead should +// be small. +var m sync.Mutex + +// syncSigner wraps a crypto.Signer with a mutex to avoid the error "smart card +// error 6982: security status not satisfied" with two concurrent signs. +type syncSigner struct { + crypto.Signer +} + +func (s *syncSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) ([]byte, error) { + m.Lock() + defer m.Unlock() + return s.Signer.Sign(rand, digest, opts) +} + +// syncDecrypter wraps a crypto.Decrypter with a mutex to avoid the error "smart +// card error 6a80: incorrect parameter in command data field" with two +// concurrent decryptions. +type syncDecrypter struct { + crypto.Decrypter +} + +func (s *syncDecrypter) Decrypt(rand io.Reader, msg []byte, opts crypto.DecrypterOpts) ([]byte, error) { + m.Lock() + defer m.Unlock() + return s.Decrypter.Decrypt(rand, msg, opts) +} + var _ apiv1.CertificateManager = (*YubiKey)(nil) diff --git a/kms/yubikey/yubikey_test.go b/kms/yubikey/yubikey_test.go index 14385538..48e13986 100644 --- a/kms/yubikey/yubikey_test.go +++ b/kms/yubikey/yubikey_test.go @@ -12,14 +12,17 @@ import ( "crypto/elliptic" "crypto/rand" "crypto/rsa" + "crypto/sha256" "crypto/x509" "crypto/x509/pkix" "encoding/asn1" + "errors" "reflect" "testing" "github.com/go-piv/piv-go/piv" - "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.step.sm/crypto/kms/apiv1" "go.step.sm/crypto/minica" ) @@ -850,10 +853,10 @@ func TestYubiKey_CreateSigner(t *testing.T) { }{ {"ok", fields{yk, "123456", piv.DefaultManagementKey}, args{&apiv1.CreateSignerRequest{ SigningKey: "yubikey:slot-id=9c", - }}, yk.signerMap[piv.SlotSignature].(crypto.Signer), false}, + }}, &syncSigner{Signer: yk.signerMap[piv.SlotSignature].(crypto.Signer)}, false}, {"ok with pin", fields{yk, "", piv.DefaultManagementKey}, args{&apiv1.CreateSignerRequest{ SigningKey: "yubikey:slot-id=9c?pin-value=123456", - }}, yk.signerMap[piv.SlotSignature].(crypto.Signer), false}, + }}, &syncSigner{Signer: yk.signerMap[piv.SlotSignature].(crypto.Signer)}, false}, {"fail getSlot", fields{yk, "123456", piv.DefaultManagementKey}, args{&apiv1.CreateSignerRequest{ SigningKey: "yubikey:slot-id=%%FF", }}, nil, true}, @@ -912,10 +915,10 @@ func TestYubiKey_CreateDecrypter(t *testing.T) { }{ {"ok", fields{yk, "123456", piv.DefaultManagementKey}, args{&apiv1.CreateDecrypterRequest{ DecryptionKey: "yubikey:slot-id=9c", - }}, yk.signerMap[piv.SlotSignature].(crypto.Decrypter), false}, + }}, &syncDecrypter{Decrypter: yk.signerMap[piv.SlotSignature].(crypto.Decrypter)}, false}, {"ok with pin", fields{yk, "", piv.DefaultManagementKey}, args{&apiv1.CreateDecrypterRequest{ DecryptionKey: "yubikey:slot-id=9c?pin-value=123456", - }}, yk.signerMap[piv.SlotSignature].(crypto.Decrypter), false}, + }}, &syncDecrypter{Decrypter: yk.signerMap[piv.SlotSignature].(crypto.Decrypter)}, false}, {"fail getSlot", fields{yk, "123456", piv.DefaultManagementKey}, args{&apiv1.CreateDecrypterRequest{ DecryptionKey: "yubikey:slot-id=%%FF", }}, nil, true}, @@ -1141,3 +1144,34 @@ func Test_getSignatureAlgorithm(t *testing.T) { }) } } + +func Test_syncSigner_Sign(t *testing.T) { + s, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + + signer := &syncSigner{Signer: s} + + sum := sha256.Sum256([]byte("the-data")) + sig, err := signer.Sign(rand.Reader, sum[:], crypto.SHA256) + require.NoError(t, err) + assert.True(t, ecdsa.VerifyASN1(&s.PublicKey, sum[:], sig)) +} + +func Test_syncDecrypter_Decrypt(t *testing.T) { + d, err := rsa.GenerateKey(rand.Reader, rsaKeySize) + require.NoError(t, err) + + label := []byte("label") + data := []byte("the-data") + + msg, err := rsa.EncryptOAEP(crypto.SHA256.New(), rand.Reader, &d.PublicKey, data, label) + require.NoError(t, err) + + decrypter := &syncDecrypter{Decrypter: d} + plain, err := decrypter.Decrypt(rand.Reader, msg, &rsa.OAEPOptions{ + Hash: crypto.SHA256, + Label: label, + }) + assert.NoError(t, err) + assert.Equal(t, data, plain) +}