Merge pull request #288 from smallstep/sync-yubikey

Create synchronized signers and decrypters for YubiKeys

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)

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)
+}