Merge pull request #97 from aws/fix/pkcs12

Fix FileSystemSigner with PKCS#12 files

Makefile

@@ -211,8 +211,8 @@ tst/softhsm2.conf: tst/softhsm2.conf.template $(PKCS8KEYS) $(RSACERTS) $(ECCERTS
 		--mark-always-authenticate
 	mv $@.tmp $@
 
-.PHONY: test
-test: test-certs tst/softhsm2.conf
+.PHONY: test-all
+test-all: test-all-certs tst/softhsm2.conf
 	$(START_SWTPM)
 	SOFTHSM2_CONF=$(curdir)/tst/softhsm2.conf go test -v ./... || :
 	$(STOP_SWTPM)
@@ -226,6 +226,10 @@ test-tpm-signer: $(certsdir)/cert-bundle.pem $(TPMKEYS) $(TPMCERTS) $(TPMLOADEDK
 	go test ./... -run "TPM"
 	$(STOP_SWTPM)
 
+.PHONY: test
+test: test-certs
+	go test ./... -list . | grep -E '^Test[a-zA-Z0-9]+' | grep -vE 'TPMSigner|PKCS11Signer' | tr '\n' '|' | sed 's/|$$//' | xargs -t go test ./... -run
+
 define CERT_RECIPE
 	@SUBJ=$$(echo "$@" | sed 's^\(.*/\)\?\([^/]*\)-cert.pem^\2^'); \
 	[ "$${SUBJ#tpm-}" != "$${SUBJ}" ] && ENG="-provider tpm2 -provider default -propquery '?provider=tpm2'";  \
@@ -254,8 +258,6 @@ endef
 		-keypbe pbeWithSHA1And3-KeyTripleDES-CBC \
 		-inkey $${KEY} -out "$@" -in $${CERT}
 
-# And once again, it's hard to do a file-based rule for the contents of the certificate store. 
-# So just populate it as a side-effect of creating the p12 file.
 %-pass.p12: %-cert.pem
 	echo Creating $@...
 	ls -l $<
@@ -311,14 +313,20 @@ $(certsdir)/cert-bundle-with-comments.pem: $(RSACERTS) $(ECCERTS)
 		echo "Comment in bundle\n" >> $@; \
 	done
 
-KEYS := $(RSAKEYS) $(ECKEYS) $(TPMKEYS) $(PKCS8KEYS)
-CERTS := $(RSACERTS) $(ECCERTS) $(TPMCERTS)
+KEYS := $(RSAKEYS) $(ECKEYS) $(PKCS8KEYS)
+ALL_KEYS := $(KEYS) $(TPMKEYS)
+CERTS := $(RSACERTS) $(ECCERTS)
+ALL_CERTS := $(CERTS) $(TPMCERTS)
 COMBOS := $(patsubst %-cert.pem, %-combo.pem, $(CERTS))
+ALL_COMBOS := $(patsubst %-cert.pem, %-combo.pem, $(ALL_CERTS))
 
-.PHONY: test-certs
-test-certs: $(KEYS) $(CERTS) $(COMBOS) $(PKCS12CERTS) $(certsdir)/cert-bundle.pem $(certsdir)/cert-bundle-with-comments.pem tst/softhsm2.conf
+.PHONY: test-all-certs
+test-all-certs: $(ALL_KEYS) $(ALL_CERTS) $(ALL_COMBOS) $(PKCS12CERTS) $(certsdir)/cert-bundle.pem $(certsdir)/cert-bundle-with-comments.pem tst/softhsm2.conf
 	$(STOP_SWTPM_TCP) 2>/dev/null || :
 
+.PHONY: test-certs
+test-certs: $(KEYS) $(CERTS) $(COMBOS) $(PKCS12CERTS) $(certsdir)/cert-bundle.pem $(certsdir)/cert-bundle-with-comments.pem
+
 .PHONY: test-clean
 test-clean:
 	rm -f $(RSAKEYS) $(ECKEYS) $(HWTPMKEYS)

aws_signing_helper/file_system_signer.go

@@ -23,13 +23,13 @@ type FileSystemSigner struct {
 func (fileSystemSigner *FileSystemSigner) Public() crypto.PublicKey {
 	privateKey, _, _ := fileSystemSigner.readCertFiles()
 	{
-		privateKey, ok := privateKey.(ecdsa.PrivateKey)
+		privateKey, ok := privateKey.(*ecdsa.PrivateKey)
 		if ok {
 			return &privateKey.PublicKey
 		}
 	}
 	{
-		privateKey, ok := privateKey.(rsa.PrivateKey)
+		privateKey, ok := privateKey.(*rsa.PrivateKey)
 		if ok {
 			return &privateKey.PublicKey
 		}
@@ -56,17 +56,17 @@ func (fileSystemSigner *FileSystemSigner) Sign(rand io.Reader, digest []byte, op
 		return nil, ErrUnsupportedHash
 	}
 
-	ecdsaPrivateKey, ok := privateKey.(ecdsa.PrivateKey)
+	ecdsaPrivateKey, ok := privateKey.(*ecdsa.PrivateKey)
 	if ok {
-		sig, err := ecdsa.SignASN1(rand, &ecdsaPrivateKey, hash[:])
+		sig, err := ecdsa.SignASN1(rand, ecdsaPrivateKey, hash[:])
 		if err == nil {
 			return sig, nil
 		}
 	}
 
-	rsaPrivateKey, ok := privateKey.(rsa.PrivateKey)
+	rsaPrivateKey, ok := privateKey.(*rsa.PrivateKey)
 	if ok {
-		sig, err := rsa.SignPKCS1v15(rand, &rsaPrivateKey, opts.HashFunc(), hash[:])
+		sig, err := rsa.SignPKCS1v15(rand, rsaPrivateKey, opts.HashFunc(), hash[:])
 		if err == nil {
 			return sig, nil
 		}
@@ -91,11 +91,11 @@ func GetFileSystemSigner(privateKeyPath string, certPath string, bundlePath stri
 	fsSigner := &FileSystemSigner{bundlePath: bundlePath, certPath: certPath, isPkcs12: isPkcs12, privateKeyPath: privateKeyPath}
 	privateKey, _, _ := fsSigner.readCertFiles()
 	// Find the signing algorithm
-	_, isRsaKey := privateKey.(rsa.PrivateKey)
+	_, isRsaKey := privateKey.(*rsa.PrivateKey)
 	if isRsaKey {
 		signingAlgorithm = aws4_x509_rsa_sha256
 	}
-	_, isEcKey := privateKey.(ecdsa.PrivateKey)
+	_, isEcKey := privateKey.(*ecdsa.PrivateKey)
 	if isEcKey {
 		signingAlgorithm = aws4_x509_ecdsa_sha256
 	}

aws_signing_helper/pkcs11_signer_test.go

@@ -0,0 +1,110 @@
+package aws_signing_helper
+
+import (
+	"fmt"
+	"testing"
+)
+
+func TestPKCS11Signer(t *testing.T) {
+	testTable := []CredentialsOpts{}
+
+	pkcs11_objects := []string{"rsa-2048", "ec-prime256v1"}
+
+	for _, object := range pkcs11_objects {
+		base_pkcs11_uri := "pkcs11:token=credential-helper-test?pin-value=1234"
+		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: basic_pkcs11_uri,
+		})
+		testTable = append(testTable, CredentialsOpts{
+			PrivateKeyId: basic_pkcs11_uri,
+		})
+		testTable = append(testTable, CredentialsOpts{
+			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,
+			ReusePin:      true,
+		})
+		testTable = append(testTable, CredentialsOpts{
+			CertificateId: cert_file,
+			PrivateKeyId:  always_auth_pkcs11_uri,
+			ReusePin:      true,
+		})
+		// Note that for the below test case, there are two matching keys.
+		// Both keys will validate with the certificate, and one will be chosen
+		// (it doesn't matter which, since both are the exact same key - it's
+		// just that one has the CKA_ALWAYS_AUTHENTICATE attribute set).
+		testTable = append(testTable, CredentialsOpts{
+			CertificateId: cert_file,
+			PrivateKeyId:  base_pkcs11_uri,
+			ReusePin:      true,
+		})
+	}
+
+	RunSignTestWithTestTable(t, testTable)
+}
+
+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,
+		ReusePin:      true,
+	})
+	testTable = append(testTable, CredentialsOpts{
+		CertificateId: ec_generic_uri,
+		PrivateKeyId:  always_auth_rsa_uri,
+		ReusePin:      true,
+	})
+	testTable = append(testTable, CredentialsOpts{
+		CertificateId: "../tst/certs/ec-prime256v1-sha256-cert.pem",
+		PrivateKeyId:  always_auth_rsa_uri,
+		ReusePin:      true,
+	})
+	testTable = append(testTable, CredentialsOpts{
+		CertificateId: "../tst/certs/rsa-2048-sha256-cert.pem",
+		PrivateKeyId:  always_auth_ec_uri,
+		ReusePin:      true,
+	})
+
+	for _, credOpts := range testTable {
+		_, _, err := GetSigner(&credOpts)
+		if err == nil {
+			t.Log("Expected failure when creating PKCS#11 signer, but received none")
+			t.Fail()
+		}
+	}
+}

aws_signing_helper/signer.go

@@ -676,32 +676,32 @@ func ReadCertificateBundleData(certificateBundleId string) ([]*x509.Certificate,
 	return x509.ParseCertificates(derBytes)
 }
 
-func readECPrivateKey(privateKeyId string) (ecdsa.PrivateKey, error) {
+func readECPrivateKey(privateKeyId string) (*ecdsa.PrivateKey, error) {
 	block, err := parseDERFromPEM(privateKeyId, "EC PRIVATE KEY")
 	if err != nil {
-		return ecdsa.PrivateKey{}, errors.New("could not parse PEM data")
+		return nil, errors.New("could not parse PEM data")
 	}
 
 	privateKey, err := x509.ParseECPrivateKey(block.Bytes)
 	if err != nil {
-		return ecdsa.PrivateKey{}, errors.New("could not parse private key")
+		return nil, errors.New("could not parse private key")
 	}
 
-	return *privateKey, nil
+	return privateKey, nil
 }
 
-func readRSAPrivateKey(privateKeyId string) (rsa.PrivateKey, error) {
+func readRSAPrivateKey(privateKeyId string) (*rsa.PrivateKey, error) {
 	block, err := parseDERFromPEM(privateKeyId, "RSA PRIVATE KEY")
 	if err != nil {
-		return rsa.PrivateKey{}, errors.New("could not parse PEM data")
+		return nil, errors.New("could not parse PEM data")
 	}
 
 	privateKey, err := x509.ParsePKCS1PrivateKey(block.Bytes)
 	if err != nil {
-		return rsa.PrivateKey{}, errors.New("could not parse private key")
+		return nil, errors.New("could not parse private key")
 	}
 
-	return *privateKey, nil
+	return privateKey, nil
 }
 
 func readPKCS8PrivateKey(privateKeyId string) (crypto.PrivateKey, error) {
@@ -717,25 +717,26 @@ func readPKCS8PrivateKey(privateKeyId string) (crypto.PrivateKey, error) {
 
 	rsaPrivateKey, ok := privateKey.(*rsa.PrivateKey)
 	if ok {
-		return *rsaPrivateKey, nil
+		return rsaPrivateKey, nil
 	}
 
 	ecPrivateKey, ok := privateKey.(*ecdsa.PrivateKey)
 	if ok {
-		return *ecPrivateKey, nil
+		return ecPrivateKey, nil
 	}
 
 	return nil, errors.New("could not parse PKCS#8 private key")
 }
 
 // Reads and parses a PKCS#12 file (which should contain an end-entity
-// certificate, (optional) certificate chain, and the key associated with the
-// end-entity certificate). The end-entity certificate will be the first
-// certificate in the returned chain. This method assumes that there is
-// exactly one certificate that doesn't issue any others within the container
-// and treats that as the end-entity certificate. Also, the order of the other
-// certificates in the chain aren't guaranteed (it's also not guaranteed that
-// those certificates form a chain with the end-entity certificat either).
+// certificate (optional), certificate chain (optional), and the key
+// associated with the end-entity certificate). The end-entity certificate
+// will be the first certificate in the returned chain. This method assumes
+// that there is exactly one certificate that doesn't issue any others within
+// the container and treats that as the end-entity certificate. Also, the
+// order of the other certificates in the chain aren't guaranteed. It's
+// also not guaranteed that those certificates form a chain with the
+// end-entity certificate either.
 func ReadPKCS12Data(certificateId string) (certChain []*x509.Certificate, privateKey crypto.PrivateKey, err error) {
 	var (
 		bytes               []byte
@@ -775,7 +776,7 @@ func ReadPKCS12Data(certificateId string) (certChain []*x509.Certificate, privat
 
 	certMap = make(map[string]*x509.Certificate)
 	for _, cert := range parsedCerts {
-		// pkix.Name.String() roughly following the RFC 2253 Distinguished Names
+		// pkix.Name.String() roughly follows the RFC 2253 Distinguished Names
 		// syntax, so we assume that it's canonical.
 		issuer := cert.Issuer.String()
 		certMap[issuer] = cert
@@ -790,8 +791,8 @@ func ReadPKCS12Data(certificateId string) (certChain []*x509.Certificate, privat
 			break
 		}
 	}
-	if endEntityFoundIndex == -1 {
-		return nil, "", errors.New("no end-entity certificate found in PKCS#12 file")
+	if Debug {
+		log.Println("no end-entity certificate found in PKCS#12 file")
 	}
 
 	for i, cert := range parsedCerts {