Skip to content

Commit

Permalink
Fix some PKCS#11 "hybrid" mode bugs.
Browse files Browse the repository at this point in the history 
 * If a certificate is provided (even if it's just the path to the file
   that was provided as opposed to a URI), make sure that the private
   key matches it when creating a PKCS11Signer.

 * If there is no certificate provided, and there is exactly one private
   key object that matches the URI, use it to create the PKCS11Signer.

 * Save the context-specific PIN after signing operations (not just when
   trying to match the appropriate private key).

 * Add keys with the CKA_ALWAYS_AUTHENTICATE attribute set, and use them
   in testing.

 * Add tests to make sure that certificate and private key do match when
   creating the PKCS11Signer.
  • Loading branch information
@13ajay
13ajay committed Sep 6, 2023
1 parent 2cf71f7 commit 2ab24ab
Show file tree
Hide file tree
Showing 3 changed files with 107 additions and 16 deletions.
18 changes: 14 additions & 4 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,26 @@ tst/softhsm2.conf: tst/softhsm2.conf.template $(PKCS8KEYS) $(RSACERTS) $(ECCERTS
--so-pin 12345678 --pin 1234

$(SHM2_UTIL) --token credential-helper-test --pin 1234 \
--import $(certsdir)/rsa-2048-key-pkcs8.pem --label RSA --id 01
--import $(certsdir)/rsa-2048-key-pkcs8.pem --label rsa-2048 --id 01
$(P11TOOL) --load-certificate $(certsdir)/rsa-2048-sha256-cert.pem \
--no-mark-private --label RSA --id 01 --set-pin 1234 --login \
--no-mark-private --label rsa-2048 --id 01 --set-pin 1234 --login \
--write "pkcs11:token=credential-helper-test;pin-value=1234"

$(SHM2_UTIL) --token credential-helper-test --pin 1234 \
--import $(certsdir)/ec-prime256v1-key-pkcs8.pem --label EC --id 02
--import $(certsdir)/ec-prime256v1-key-pkcs8.pem --label ec-prime256v1 --id 02
$(P11TOOL) --load-certificate $(certsdir)/ec-prime256v1-sha256-cert.pem \
--no-mark-private --label EC --id 02 --set-pin 1234 --login \
--no-mark-private --label ec-prime256v1 --id 02 --set-pin 1234 --login \
--write "pkcs11:token=credential-helper-test;pin-value=1234"

$(P11TOOL) --load-privkey $(certsdir)/rsa-2048-key-pkcs8.pem \
--label rsa-2048-always-auth --id 03 --set-pin 1234 --login \
--write "pkcs11:token=credential-helper-test;pin-value=1234" \
--mark-always-authenticate

$(P11TOOL) --load-privkey $(certsdir)/ec-prime256v1-key-pkcs8.pem \
--label ec-prime256v1-always-auth --id 04 --set-pin 1234 --login \
--write "pkcs11:token=credential-helper-test;pin-value=1234" \
--mark-always-authenticate
mv $@.tmp $@

test: test-certs tst/softhsm2.conf
Expand Down
25 changes: 19 additions & 6 deletions aws_signing_helper/pkcs11_signer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -527,7 +527,7 @@ func pkcs11PasswordPrompt(module *pkcs11.Ctx, session pkcs11.SessionHandle, user
var pin string

parseErrMsg := fmt.Sprintf("unable to read PKCS#11 %s", passwordName)
prompt := fmt.Sprintf("Please enter your %s", passwordName)
prompt := fmt.Sprintf("Please enter your %s:", passwordName)

ttyPath := "/dev/tty"
if runtime.GOOS == "windows" {
Expand Down Expand Up @@ -640,7 +640,7 @@ func signHelper(module *pkcs11.Ctx, session pkcs11.SessionHandle, privateKeyHand
// the context-specific PIN for this object.
passwordName := "context-specific pin"
finalAuthErrMsg := "user re-authentication failed (%s)"
_, err = pkcs11PasswordPrompt(module, session, pkcs11.CKU_CONTEXT_SPECIFIC, passwordName, finalAuthErrMsg)
contextSpecificPin, err = pkcs11PasswordPrompt(module, session, pkcs11.CKU_CONTEXT_SPECIFIC, passwordName, finalAuthErrMsg)
if err != nil {
return "", nil, err
}
Expand Down Expand Up @@ -790,8 +790,14 @@ retry_search:
}

if certObj.cert == nil {
privateKeyHandle = curPrivateKeyHandle
break
if len(privateKeyObjects) == 1 {
privateKeyHandle = curPrivateKeyHandle
break
} else {
err = errors.New("multiple matching private keys, but" +
" no certificate provided to match with")
goto fail
}
}

curContextSpecificPin := contextSpecificPin
Expand Down Expand Up @@ -927,9 +933,11 @@ func (pkcs11Signer *PKCS11Signer) Sign(rand io.Reader, digest []byte, opts crypt
goto cleanUp
}

_, signature, err = signHelper(module, session, privateKeyHandle, alwaysAuth, false, userPin, contextSpecificPin, keyType, digest, hashFunc)
contextSpecificPin, signature, err = signHelper(module, session, privateKeyHandle, alwaysAuth, false, userPin, contextSpecificPin, keyType, digest, hashFunc)
if err != nil {
goto cleanUp
} else {
pkcs11Signer.contextSpecificPin = contextSpecificPin
}

// Note that the session should be logged out of and closed even if there
Expand Down Expand Up @@ -1180,6 +1188,9 @@ func GetPKCS11Signer(libPkcs11 string, cert *x509.Certificate, certChain []*x509
goto fail
}
}
} else if cert != nil {
// Populate certObj, so that it can be used to find the matching private key.
certObj = CertObjInfo{nil, nil, cert, 0}
}

// If an explicit private-key option was given, use it. Otherwise
Expand All @@ -1197,7 +1208,9 @@ func GetPKCS11Signer(libPkcs11 string, cert *x509.Certificate, certChain []*x509
keyUri.Parse(certUriStr)
noKeyUri = true
}
userPin, _ = keyUri.GetQueryAttribute("pin-value", false)
if _userPin, ok := keyUri.GetQueryAttribute("pin-value", false); ok {
userPin = _userPin
}

// If the certificate's PKCS#11 URI wasn't provided, enumerate slots.
if certificateId == "" {
Expand Down
80 changes: 74 additions & 6 deletions aws_signing_helper/signer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -242,20 +242,34 @@ func TestSign(t *testing.T) {
}
}

pkcs11_objects := []string{"RSA", "EC"}
pkcs11_objects := []string{"rsa-2048", "ec-prime256v1"}

for _, object := range pkcs11_objects {
pkcs11_uri := fmt.Sprintf("pkcs11:token=credential-helper-test;object=%s?pin-value=1234", object)
basic_pkcs11_uri := fmt.Sprintf("pkcs11:token=credential-helper-test;object=%s?pin-value=1234", object)
always_auth_pkcs11_uri := fmt.Sprintf("pkcs11:token=credential-helper-test;object=%s-always-auth?pin-value=1234", object)
cert_file := fmt.Sprintf("../tst/certs/%s-sha256-cert.pem", object)

testTable = append(testTable, CredentialsOpts{
CertificateId: pkcs11_uri,
CertificateId: basic_pkcs11_uri,
})
testTable = append(testTable, CredentialsOpts{
PrivateKeyId: pkcs11_uri,
PrivateKeyId: basic_pkcs11_uri,
})
testTable = append(testTable, CredentialsOpts{
CertificateId: pkcs11_uri,
PrivateKeyId: pkcs11_uri,
CertificateId: basic_pkcs11_uri,
PrivateKeyId: basic_pkcs11_uri,
})
testTable = append(testTable, CredentialsOpts{
CertificateId: cert_file,
PrivateKeyId: basic_pkcs11_uri,
})
testTable = append(testTable, CredentialsOpts{
CertificateId: basic_pkcs11_uri,
PrivateKeyId: always_auth_pkcs11_uri,
})
testTable = append(testTable, CredentialsOpts{
CertificateId: cert_file,
PrivateKeyId: always_auth_pkcs11_uri,
})
}

Expand Down Expand Up @@ -287,6 +301,9 @@ func TestSign(t *testing.T) {

for _, digest := range digestList {
signatureBytes, err := signer.Sign(rand.Reader, []byte(msg), digest)
// Try signing again to make sure that a context-specific PIN, if
// needed, was cached.
signer.Sign(rand.Reader, []byte(msg), digest)
if err != nil {
t.Log("Failed to sign the input message")
t.Fail()
Expand Down Expand Up @@ -403,6 +420,57 @@ func TestCertStoreSignerCreationFails(t *testing.T) {
}
}

func TestPKCS11SignerCreationFails(t *testing.T) {
testTable := []CredentialsOpts{}

template_uri := "pkcs11:token=credential-helper-test;object=%s?pin-value=1234"
rsa_generic_uri := fmt.Sprintf(template_uri, "rsa-2048")
ec_generic_uri := fmt.Sprintf(template_uri, "ec-prime256v1")
// always_auth_rsa_uri := fmt.Sprintf(template_uri, "rsa-2048-always-auth")
// always_auth_ec_uri := fmt.Sprintf(template_uri, "ec-prime256v1-always-auth")

testTable = append(testTable, CredentialsOpts{
CertificateId: rsa_generic_uri,
PrivateKeyId: ec_generic_uri,
})
testTable = append(testTable, CredentialsOpts{
CertificateId: ec_generic_uri,
PrivateKeyId: rsa_generic_uri,
})
testTable = append(testTable, CredentialsOpts{
CertificateId: "../tst/certs/ec-prime256v1-sha256-cert.pem",
PrivateKeyId: rsa_generic_uri,
})
testTable = append(testTable, CredentialsOpts{
CertificateId: "../tst/certs/rsa-2048-sha256-cert.pem",
PrivateKeyId: ec_generic_uri,
})
/* testTable = append(testTable, CredentialsOpts{
CertificateId: rsa_generic_uri,
PrivateKeyId: always_auth_ec_uri,
})
testTable = append(testTable, CredentialsOpts{
CertificateId: ec_generic_uri,
PrivateKeyId: always_auth_rsa_uri,
})
testTable = append(testTable, CredentialsOpts{
CertificateId: "../tst/certs/ec-prime256v1-sha256-cert.pem",
PrivateKeyId: always_auth_rsa_uri,
})
testTable = append(testTable, CredentialsOpts{
CertificateId: "../tst/certs/rsa-2048-sha256-cert.pem",
PrivateKeyId: always_auth_ec_uri,
}) */

for _, credOpts := range testTable {
_, _, err := GetSigner(&credOpts)
if err == nil {
t.Log("Expected failure when creating certificate store signer, but received none")
t.Fail()
}
}
}

func TestUpdate(t *testing.T) {
testTable := []struct {
name string
Expand Down

0 comments on commit 2ab24ab

Please sign in to comment.