Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix FileSystemSigner with PKCS#12 files #97

Merged
merged 1 commit into from
Feb 13, 2025
Merged

Fix FileSystemSigner with PKCS#12 files #97

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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^'); \
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sed command does not seem to remove the -cert.pem suffix from the filename

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me when using GNU sed (maybe you're using BSD sed?). As an example:

% echo "tst/certs/rsa-1024-md5-cert.pem" | sed 's^\(.*/\)\?\([^/]*\)-cert.pem^\2^'
rsa-1024-md5

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. I just confirmed that it works when using GNU sed.

[ "$${SUBJ#tpm-}" != "$${SUBJ}" ] && ENG="-provider tpm2 -provider default -propquery '?provider=tpm2'"; \
Expand Down Expand Up @@ -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 $<
Expand Down Expand Up @@ -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)
Expand Down
16 changes: 8 additions & 8 deletions aws_signing_helper/file_system_signer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down
110 changes: 110 additions & 0 deletions aws_signing_helper/pkcs11_signer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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()
}
}
}
41 changes: 21 additions & 20 deletions aws_signing_helper/signer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down
Loading
Loading