From 75d16e134dcb3746a698c12a4cd47116e65c3a57 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Wed, 4 Sep 2019 13:31:27 +0100 Subject: [PATCH 01/36] Support test skipping for AWS CloudHSM --- README.md | 9 +++++++++ certificates_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/README.md b/README.md index dea155e..7f23370 100644 --- a/README.md +++ b/README.md @@ -60,6 +60,15 @@ A minimal configuration file looks like this: Testing Guidance ================ +Disabling tests +--------------- + +To disable specific tests, set the environment variable `CRYPTO11_SKIP=` where `` is a comma-separated +list of the following options: + +* `CERTS` - disables certificate-related tests. Needed for AWS CloudHSM, which doesn't support certificates. + + Testing with SoftHSM2 --------------------- diff --git a/certificates_test.go b/certificates_test.go index 49840de..ced8e4d 100644 --- a/certificates_test.go +++ b/certificates_test.go @@ -28,6 +28,8 @@ import ( "crypto/x509/pkix" "encoding/asn1" "math/big" + "os" + "strings" "testing" "time" @@ -35,7 +37,25 @@ import ( "github.com/stretchr/testify/require" ) +const skipTestEnv = "CRYPTO11_SKIP" + +const skipTestCert = "CERTS" + +// skipTest tests whether the CRYPTO11_SKIP environment variable contains +// flagName. If so, it skips the test. +func skipTest(t *testing.T, flagName string) { + thingsToSkip := strings.Split(os.Getenv(skipTestEnv), ",") + for _, s := range thingsToSkip { + if s == flagName { + t.Logf("Skipping test due to %s flag", flagName) + t.SkipNow() + } + } +} + func TestCertificate(t *testing.T) { + skipTest(t, skipTestCert) + ctx, err := ConfigureFromFile("config") require.NoError(t, err) @@ -70,6 +90,8 @@ func TestCertificate(t *testing.T) { // Test that provided attributes override default values func TestCertificateAttributes(t *testing.T) { + skipTest(t, skipTestCert) + ctx, err := ConfigureFromFile("config") require.NoError(t, err) @@ -103,6 +125,8 @@ func TestCertificateAttributes(t *testing.T) { } func TestCertificateRequiredArgs(t *testing.T) { + skipTest(t, skipTestCert) + ctx, err := ConfigureFromFile("config") require.NoError(t, err) From 85a968661f8e3d86801984049d5665ea4d3a3ef4 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Wed, 4 Sep 2019 13:40:50 +0100 Subject: [PATCH 02/36] Close context if key gen fails --- close_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/close_test.go b/close_test.go index d3e7cfc..662d5e9 100644 --- a/close_test.go +++ b/close_test.go @@ -42,9 +42,11 @@ func TestClose(t *testing.T) { const pSize = dsa.L1024N160 id := randomBytes() - key, err := ctx.GenerateDSAKeyPair(id, dsaSizes[pSize]) - require.NoError(t, err) - require.NotNil(t, key) + _, err = ctx.GenerateDSAKeyPair(id, dsaSizes[pSize]) + if err != nil { + _ = ctx.Close() + t.Fatal(err) + } require.NoError(t, ctx.Close()) @@ -61,7 +63,7 @@ func TestClose(t *testing.T) { err = key2.Delete() require.NoError(t, err) } - + require.NoError(t, ctx.Close()) } } From ccb1c536b66cf2265b2e764af654f659d3d724f6 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Wed, 4 Sep 2019 13:51:53 +0100 Subject: [PATCH 03/36] Add more failure info to test --- dsa_test.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/dsa_test.go b/dsa_test.go index b155a01..bd25daf 100644 --- a/dsa_test.go +++ b/dsa_test.go @@ -115,9 +115,8 @@ func TestHardDSA(t *testing.T) { label := randomBytes() key, err := ctx.GenerateDSAKeyPairWithLabel(id, label, params) - require.NoError(t, err) - require.NotNil(t, key) - defer key.Delete() + require.NoError(t, err, "Failed for key size %s", parameterSizeToString(pSize)) + defer func() { _ = key.Delete() }() testDsaSigning(t, key, pSize, "hard1") @@ -131,6 +130,21 @@ func TestHardDSA(t *testing.T) { } } +func parameterSizeToString(s dsa.ParameterSizes) string { + switch s { + case dsa.L1024N160: + return "L1024N160" + case dsa.L2048N224: + return "L2048N224" + case dsa.L2048N256: + return "L2048N256" + case dsa.L3072N256: + return "L3072N256" + default: + return "unknown" + } +} + func testDsaSigning(t *testing.T, key crypto.Signer, psize dsa.ParameterSizes, what string) { testDsaSigningWithHash(t, key, crypto.SHA1, psize, what) testDsaSigningWithHash(t, key, crypto.SHA224, psize, what) From 21e7244338255968c8efeae03ba2310d47844ccf Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Mon, 9 Sep 2019 11:15:38 +0100 Subject: [PATCH 04/36] Only test with 2048-bit keys --- rsa_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rsa_test.go b/rsa_test.go index 9934c1e..adfbd92 100644 --- a/rsa_test.go +++ b/rsa_test.go @@ -36,7 +36,7 @@ import ( "github.com/stretchr/testify/require" ) -var rsaSizes = []int{1024, 2048} +var rsaSizes = []int{2048} func TestNativeRSA(t *testing.T) { @@ -70,6 +70,7 @@ func TestHardRSA(t *testing.T) { }() for _, nbits := range rsaSizes { + id := randomBytes() label := randomBytes() From 3105a1d02e4f9a8599ff88eb9f7d631e409d7074 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Mon, 9 Sep 2019 13:50:56 +0100 Subject: [PATCH 05/36] Simplify OAEP code --- rsa.go | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/rsa.go b/rsa.go index c6fb989..234fd0a 100644 --- a/rsa.go +++ b/rsa.go @@ -27,7 +27,6 @@ import ( "errors" "io" "math/big" - "unsafe" "github.com/miekg/pkcs11" ) @@ -210,29 +209,25 @@ func decryptPKCS1v15(session *pkcs11Session, key *pkcs11PrivateKeyRSA, ciphertex return session.ctx.Decrypt(session.handle, ciphertext) } -func decryptOAEP(session *pkcs11Session, key *pkcs11PrivateKeyRSA, ciphertext []byte, hashFunction crypto.Hash, label []byte) ([]byte, error) { - var err error - var hMech, mgf, sourceData, sourceDataLen uint - if hMech, mgf, _, err = hashToPKCS11(hashFunction); err != nil { +func decryptOAEP(session *pkcs11Session, key *pkcs11PrivateKeyRSA, ciphertext []byte, hashFunction crypto.Hash, + label []byte) ([]byte, error) { + + hashAlg, mgfAlg, _, err := hashToPKCS11(hashFunction) + if err != nil { return nil, err } - if len(label) > 0 { - sourceData = uint(uintptr(unsafe.Pointer(&label[0]))) - sourceDataLen = uint(len(label)) - } - parameters := concat(ulongToBytes(hMech), - ulongToBytes(mgf), - ulongToBytes(pkcs11.CKZ_DATA_SPECIFIED), - ulongToBytes(sourceData), - ulongToBytes(sourceDataLen)) - mech := []*pkcs11.Mechanism{pkcs11.NewMechanism(pkcs11.CKM_RSA_PKCS_OAEP, parameters)} - if err = session.ctx.DecryptInit(session.handle, mech, key.handle); err != nil { + + mech := pkcs11.NewMechanism(pkcs11.CKM_RSA_PKCS_OAEP, + pkcs11.NewOAEPParams(hashAlg, mgfAlg, pkcs11.CKZ_DATA_SPECIFIED, label)) + + err = session.ctx.DecryptInit(session.handle, []*pkcs11.Mechanism{mech}, key.handle) + if err != nil { return nil, err } return session.ctx.Decrypt(session.handle, ciphertext) } -func hashToPKCS11(hashFunction crypto.Hash) (uint, uint, uint, error) { +func hashToPKCS11(hashFunction crypto.Hash) (hashAlg uint, mgfAlg uint, hashLen uint, err error) { switch hashFunction { case crypto.SHA1: return pkcs11.CKM_SHA_1, pkcs11.CKG_MGF1_SHA1, 20, nil From bfbf8d7dcb7c5950083ee0b880bb6c9749dbcdf5 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Mon, 9 Sep 2019 14:01:14 +0100 Subject: [PATCH 06/36] Add new OAEP_LABEL option for skipping tests --- README.md | 4 +- certificates_test.go | 18 ------- rsa_test.go | 120 ++++++++++++++++++++----------------------- skip_test.go | 30 +++++++++++ 4 files changed, 89 insertions(+), 83 deletions(-) create mode 100644 skip_test.go diff --git a/README.md b/README.md index 7f23370..1afeb6c 100644 --- a/README.md +++ b/README.md @@ -66,7 +66,9 @@ Disabling tests To disable specific tests, set the environment variable `CRYPTO11_SKIP=` where `` is a comma-separated list of the following options: -* `CERTS` - disables certificate-related tests. Needed for AWS CloudHSM, which doesn't support certificates. +* `CERTS` - disables certificate-related tests. Needed for AWS CloudHSM, which doesn't support certificates. +* `OAEP_LABEL` - disables RSA OAEP encryption tests that use source data encoding parameter (also known as a 'label' +in some crypto libraries). Needed for AWS CloudHSM. Testing with SoftHSM2 diff --git a/certificates_test.go b/certificates_test.go index ced8e4d..cade98f 100644 --- a/certificates_test.go +++ b/certificates_test.go @@ -28,8 +28,6 @@ import ( "crypto/x509/pkix" "encoding/asn1" "math/big" - "os" - "strings" "testing" "time" @@ -37,22 +35,6 @@ import ( "github.com/stretchr/testify/require" ) -const skipTestEnv = "CRYPTO11_SKIP" - -const skipTestCert = "CERTS" - -// skipTest tests whether the CRYPTO11_SKIP environment variable contains -// flagName. If so, it skips the test. -func skipTest(t *testing.T, flagName string) { - thingsToSkip := strings.Split(os.Getenv(skipTestEnv), ",") - for _, s := range thingsToSkip { - if s == flagName { - t.Logf("Skipping test due to %s flag", flagName) - t.SkipNow() - } - } -} - func TestCertificate(t *testing.T) { skipTest(t, skipTestCert) diff --git a/rsa_test.go b/rsa_test.go index adfbd92..7c4e9df 100644 --- a/rsa_test.go +++ b/rsa_test.go @@ -29,14 +29,14 @@ import ( _ "crypto/sha1" _ "crypto/sha256" _ "crypto/sha512" - "fmt" "testing" "github.com/miekg/pkcs11" "github.com/stretchr/testify/require" ) -var rsaSizes = []int{2048} +// Set to 2048, as most tokens will support this. 1024 not supported by some tokens (e.g. Amazon CloudHSM). +const rsaSize = 2048 func TestNativeRSA(t *testing.T) { @@ -47,66 +47,55 @@ func TestNativeRSA(t *testing.T) { require.NoError(t, ctx.Close()) }() - for _, nbits := range rsaSizes { - t.Run(fmt.Sprintf("%v", nbits), func(t *testing.T) { - key, err := rsa.GenerateKey(rand.Reader, nbits) - require.NoError(t, err) + key, err := rsa.GenerateKey(rand.Reader, rsaSize) + require.NoError(t, err) - err = key.Validate() - require.NoError(t, err) + err = key.Validate() + require.NoError(t, err) - t.Run("Sign", func(t *testing.T) { testRsaSigning(t, key, nbits, true) }) - t.Run("Encrypt", func(t *testing.T) { testRsaEncryption(t, key, nbits, true) }) - }) - } + t.Run("Sign", func(t *testing.T) { testRsaSigning(t, key, rsaSize, true) }) + t.Run("Encrypt", func(t *testing.T) { testRsaEncryption(t, key, rsaSize, true) }) } func TestHardRSA(t *testing.T) { ctx, err := ConfigureFromFile("config") require.NoError(t, err) - defer func() { require.NoError(t, ctx.Close()) }() - for _, nbits := range rsaSizes { - - id := randomBytes() - label := randomBytes() - - t.Run(fmt.Sprintf("%v", nbits), func(t *testing.T) { - - key, err := ctx.GenerateRSAKeyPairWithLabel(id, label, nbits) - require.NoError(t, err) - require.NotNil(t, key) - defer key.Delete() - - var key2, key3 crypto.PrivateKey - - t.Run("Sign", func(t *testing.T) { testRsaSigning(t, key, nbits, false) }) - t.Run("Encrypt", func(t *testing.T) { testRsaEncryption(t, key, nbits, false) }) - t.Run("FindId", func(t *testing.T) { - key2, err = ctx.FindKeyPair(id, nil) - require.NoError(t, err) - }) - t.Run("SignId", func(t *testing.T) { - if key2 == nil { - t.SkipNow() - } - testRsaSigning(t, key2.(*pkcs11PrivateKeyRSA), nbits, false) - }) - t.Run("FindLabel", func(t *testing.T) { - key3, err = ctx.FindKeyPair(nil, label) - require.NoError(t, err) - }) - t.Run("SignLabel", func(t *testing.T) { - if key3 == nil { - t.SkipNow() - } - testRsaSigning(t, key3.(crypto.Signer), nbits, false) - }) - }) - } + id := randomBytes() + label := randomBytes() + + key, err := ctx.GenerateRSAKeyPairWithLabel(id, label, rsaSize) + require.NoError(t, err) + require.NotNil(t, key) + defer key.Delete() + + var key2, key3 crypto.PrivateKey + + t.Run("Sign", func(t *testing.T) { testRsaSigning(t, key, rsaSize, false) }) + t.Run("Encrypt", func(t *testing.T) { testRsaEncryption(t, key, rsaSize, false) }) + t.Run("FindId", func(t *testing.T) { + key2, err = ctx.FindKeyPair(id, nil) + require.NoError(t, err) + }) + t.Run("SignId", func(t *testing.T) { + if key2 == nil { + t.SkipNow() + } + testRsaSigning(t, key2.(*pkcs11PrivateKeyRSA), rsaSize, false) + }) + t.Run("FindLabel", func(t *testing.T) { + key3, err = ctx.FindKeyPair(nil, label) + require.NoError(t, err) + }) + t.Run("SignLabel", func(t *testing.T) { + if key3 == nil { + t.SkipNow() + } + testRsaSigning(t, key3.(crypto.Signer), rsaSize, false) + }) } func testRsaSigning(t *testing.T, key crypto.Signer, nbits int, native bool) { @@ -181,19 +170,22 @@ func testRsaEncryption(t *testing.T, key crypto.Decrypter, nbits int, native boo t.Skipf("key too small for SHA512") } }) - t.Run("OAEPSHA1Label", func(t *testing.T) { testRsaEncryptionOAEP(t, key, crypto.SHA1, []byte{1, 2, 3, 4}, native) }) - t.Run("OAEPSHA224Label", func(t *testing.T) { testRsaEncryptionOAEP(t, key, crypto.SHA224, []byte{5, 6, 7, 8}, native) }) - t.Run("OAEPSHA256Label", func(t *testing.T) { testRsaEncryptionOAEP(t, key, crypto.SHA256, []byte{9}, native) }) - t.Run("OAEPSHA384Label", func(t *testing.T) { - testRsaEncryptionOAEP(t, key, crypto.SHA384, []byte{10, 11, 12, 13, 14, 15}, native) - }) - t.Run("OAEPSHA512Label", func(t *testing.T) { - if nbits > 1024 { - testRsaEncryptionOAEP(t, key, crypto.SHA512, []byte{16, 17, 18}, native) - } else { - t.Skipf("key too small for SHA512") - } - }) + + if !shouldSkipTest(skipTestOAEPLabel) { + t.Run("OAEPSHA1Label", func(t *testing.T) { testRsaEncryptionOAEP(t, key, crypto.SHA1, []byte{1, 2, 3, 4}, native) }) + t.Run("OAEPSHA224Label", func(t *testing.T) { testRsaEncryptionOAEP(t, key, crypto.SHA224, []byte{5, 6, 7, 8}, native) }) + t.Run("OAEPSHA256Label", func(t *testing.T) { testRsaEncryptionOAEP(t, key, crypto.SHA256, []byte{9}, native) }) + t.Run("OAEPSHA384Label", func(t *testing.T) { + testRsaEncryptionOAEP(t, key, crypto.SHA384, []byte{10, 11, 12, 13, 14, 15}, native) + }) + t.Run("OAEPSHA512Label", func(t *testing.T) { + if nbits > 1024 { + testRsaEncryptionOAEP(t, key, crypto.SHA512, []byte{16, 17, 18}, native) + } else { + t.Skipf("key too small for SHA512") + } + }) + } } func testRsaEncryptionPKCS1v15(t *testing.T, key crypto.Decrypter) { diff --git a/skip_test.go b/skip_test.go new file mode 100644 index 0000000..fbe60cc --- /dev/null +++ b/skip_test.go @@ -0,0 +1,30 @@ +package crypto11 + +import ( + "os" + "strings" + "testing" +) + +const skipTestEnv = "CRYPTO11_SKIP" +const skipTestCert = "CERTS" +const skipTestOAEPLabel = "OAEP_LABEL" + +// skipTest tests whether the CRYPTO11_SKIP environment variable contains +// flagName. If so, it skips the test. +func skipTest(t *testing.T, flagName string) { + if shouldSkipTest(flagName) { + t.Logf("Skipping test due to %s flag", flagName) + t.SkipNow() + } +} + +func shouldSkipTest(flagName string) bool { + thingsToSkip := strings.Split(os.Getenv(skipTestEnv), ",") + for _, s := range thingsToSkip { + if s == flagName { + return true + } + } + return false +} From 59f3d0936ffe8d2bd29de9e22872c7bc27b82d49 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Mon, 9 Sep 2019 14:20:37 +0100 Subject: [PATCH 07/36] Tidy up RSA test file --- rsa_test.go | 42 ++++++++++++------------------------------ 1 file changed, 12 insertions(+), 30 deletions(-) diff --git a/rsa_test.go b/rsa_test.go index 7c4e9df..8fa7d91 100644 --- a/rsa_test.go +++ b/rsa_test.go @@ -53,8 +53,8 @@ func TestNativeRSA(t *testing.T) { err = key.Validate() require.NoError(t, err) - t.Run("Sign", func(t *testing.T) { testRsaSigning(t, key, rsaSize, true) }) - t.Run("Encrypt", func(t *testing.T) { testRsaEncryption(t, key, rsaSize, true) }) + t.Run("Sign", func(t *testing.T) { testRsaSigning(t, key, true) }) + t.Run("Encrypt", func(t *testing.T) { testRsaEncryption(t, key, true) }) } func TestHardRSA(t *testing.T) { @@ -70,12 +70,12 @@ func TestHardRSA(t *testing.T) { key, err := ctx.GenerateRSAKeyPairWithLabel(id, label, rsaSize) require.NoError(t, err) require.NotNil(t, key) - defer key.Delete() + defer func() { _ = key.Delete() }() var key2, key3 crypto.PrivateKey - t.Run("Sign", func(t *testing.T) { testRsaSigning(t, key, rsaSize, false) }) - t.Run("Encrypt", func(t *testing.T) { testRsaEncryption(t, key, rsaSize, false) }) + t.Run("Sign", func(t *testing.T) { testRsaSigning(t, key, false) }) + t.Run("Encrypt", func(t *testing.T) { testRsaEncryption(t, key, false) }) t.Run("FindId", func(t *testing.T) { key2, err = ctx.FindKeyPair(id, nil) require.NoError(t, err) @@ -84,7 +84,7 @@ func TestHardRSA(t *testing.T) { if key2 == nil { t.SkipNow() } - testRsaSigning(t, key2.(*pkcs11PrivateKeyRSA), rsaSize, false) + testRsaSigning(t, key2.(*pkcs11PrivateKeyRSA), false) }) t.Run("FindLabel", func(t *testing.T) { key3, err = ctx.FindKeyPair(nil, label) @@ -94,11 +94,11 @@ func TestHardRSA(t *testing.T) { if key3 == nil { t.SkipNow() } - testRsaSigning(t, key3.(crypto.Signer), rsaSize, false) + testRsaSigning(t, key3.(crypto.Signer), false) }) } -func testRsaSigning(t *testing.T, key crypto.Signer, nbits int, native bool) { +func testRsaSigning(t *testing.T, key crypto.Signer, native bool) { t.Run("SHA1", func(t *testing.T) { testRsaSigningPKCS1v15(t, key, crypto.SHA1) }) t.Run("SHA224", func(t *testing.T) { testRsaSigningPKCS1v15(t, key, crypto.SHA224) }) t.Run("SHA256", func(t *testing.T) { testRsaSigningPKCS1v15(t, key, crypto.SHA256) }) @@ -108,13 +108,7 @@ func testRsaSigning(t *testing.T, key crypto.Signer, nbits int, native bool) { t.Run("PSSSHA224", func(t *testing.T) { testRsaSigningPSS(t, key, crypto.SHA224, native) }) t.Run("PSSSHA256", func(t *testing.T) { testRsaSigningPSS(t, key, crypto.SHA256, native) }) t.Run("PSSSHA384", func(t *testing.T) { testRsaSigningPSS(t, key, crypto.SHA384, native) }) - t.Run("PSSSHA512", func(t *testing.T) { - if nbits > 1024 { - testRsaSigningPSS(t, key, crypto.SHA512, native) - } else { - t.Skipf("key too smol for SHA512 with sLen=hLen") - } - }) + t.Run("PSSSHA512", func(t *testing.T) { testRsaSigningPSS(t, key, crypto.SHA512, native) }) } func testRsaSigningPKCS1v15(t *testing.T, key crypto.Signer, hashFunction crypto.Hash) { @@ -157,19 +151,13 @@ func testRsaSigningPSS(t *testing.T, key crypto.Signer, hashFunction crypto.Hash require.NoError(t, err) } -func testRsaEncryption(t *testing.T, key crypto.Decrypter, nbits int, native bool) { +func testRsaEncryption(t *testing.T, key crypto.Decrypter, native bool) { t.Run("PKCS1v15", func(t *testing.T) { testRsaEncryptionPKCS1v15(t, key) }) t.Run("OAEPSHA1", func(t *testing.T) { testRsaEncryptionOAEP(t, key, crypto.SHA1, []byte{}, native) }) t.Run("OAEPSHA224", func(t *testing.T) { testRsaEncryptionOAEP(t, key, crypto.SHA224, []byte{}, native) }) t.Run("OAEPSHA256", func(t *testing.T) { testRsaEncryptionOAEP(t, key, crypto.SHA256, []byte{}, native) }) t.Run("OAEPSHA384", func(t *testing.T) { testRsaEncryptionOAEP(t, key, crypto.SHA384, []byte{}, native) }) - t.Run("OAEPSHA512", func(t *testing.T) { - if nbits > 1024 { - testRsaEncryptionOAEP(t, key, crypto.SHA512, []byte{}, native) - } else { - t.Skipf("key too small for SHA512") - } - }) + t.Run("OAEPSHA512", func(t *testing.T) { testRsaEncryptionOAEP(t, key, crypto.SHA512, []byte{}, native) }) if !shouldSkipTest(skipTestOAEPLabel) { t.Run("OAEPSHA1Label", func(t *testing.T) { testRsaEncryptionOAEP(t, key, crypto.SHA1, []byte{1, 2, 3, 4}, native) }) @@ -178,13 +166,7 @@ func testRsaEncryption(t *testing.T, key crypto.Decrypter, nbits int, native boo t.Run("OAEPSHA384Label", func(t *testing.T) { testRsaEncryptionOAEP(t, key, crypto.SHA384, []byte{10, 11, 12, 13, 14, 15}, native) }) - t.Run("OAEPSHA512Label", func(t *testing.T) { - if nbits > 1024 { - testRsaEncryptionOAEP(t, key, crypto.SHA512, []byte{16, 17, 18}, native) - } else { - t.Skipf("key too small for SHA512") - } - }) + t.Run("OAEPSHA512Label", func(t *testing.T) { testRsaEncryptionOAEP(t, key, crypto.SHA512, []byte{16, 17, 18}, native) }) } } From 735d67690dad20a603bb7fb1b0f25b9925d415af Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Mon, 9 Sep 2019 14:23:55 +0100 Subject: [PATCH 08/36] Use 2048 bits keys (and handle errors) --- keys_test.go | 38 +++++++++++++++++++------------------- thread_test.go | 4 ++-- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/keys_test.go b/keys_test.go index 3fb0798..fe95475 100644 --- a/keys_test.go +++ b/keys_test.go @@ -44,15 +44,15 @@ func TestFindingKeysWithAttributes(t *testing.T) { key, err := ctx.GenerateSecretKey(id, 128, CipherAES) require.NoError(t, err) - defer key.Delete() + defer func() { _ = key.Delete() }() key, err = ctx.GenerateSecretKey(id2, 128, CipherAES) require.NoError(t, err) - defer key.Delete() + defer func() { _ = key.Delete() }() key, err = ctx.GenerateSecretKey(id2, 256, CipherAES) require.NoError(t, err) - defer key.Delete() + defer func() { _ = key.Delete() }() attrs, err := NewAttributeSetWithID(id) require.NoError(t, err) @@ -87,17 +87,17 @@ func TestFindingKeyPairsWithAttributes(t *testing.T) { id := randomBytes() id2 := randomBytes() - key, err := ctx.GenerateRSAKeyPair(id, 1024) + key, err := ctx.GenerateRSAKeyPair(id, rsaSize) require.NoError(t, err) - defer key.Delete() + defer func() { _ = key.Delete() }() - key, err = ctx.GenerateRSAKeyPair(id2, 1024) + key, err = ctx.GenerateRSAKeyPair(id2, rsaSize) require.NoError(t, err) - defer key.Delete() + defer func() { _ = key.Delete() }() - key, err = ctx.GenerateRSAKeyPair(id2, 2048) + key, err = ctx.GenerateRSAKeyPair(id2, rsaSize) require.NoError(t, err) - defer key.Delete() + defer func() { _ = key.Delete() }() attrs, err := NewAttributeSetWithID(id) require.NoError(t, err) @@ -127,7 +127,7 @@ func TestFindingAllKeys(t *testing.T) { key, err := ctx.GenerateSecretKey(id, 128, CipherAES) require.NoError(t, err) - defer key.Delete() + defer func() { _ = key.Delete() }() } keys, err := ctx.FindAllKeys() @@ -142,10 +142,10 @@ func TestFindingAllKeyPairs(t *testing.T) { withContext(t, func(ctx *Context) { for i := 1; i <= 5; i++ { id := randomBytes() - key, err := ctx.GenerateRSAKeyPair(id, 1024) + key, err := ctx.GenerateRSAKeyPair(id, rsaSize) require.NoError(t, err) - defer key.Delete() + defer func() { _ = key.Delete() }() } keys, err := ctx.FindAllKeyPairs() @@ -160,9 +160,9 @@ func TestGettingPrivateKeyAttributes(t *testing.T) { withContext(t, func(ctx *Context) { id := randomBytes() - key, err := ctx.GenerateRSAKeyPair(id, 1024) + key, err := ctx.GenerateRSAKeyPair(id, rsaSize) require.NoError(t, err) - defer key.Delete() + defer func() { _ = key.Delete() }() attrs, err := ctx.GetAttributes(key, []AttributeType{CkaModulus}) require.NoError(t, err) @@ -177,16 +177,16 @@ func TestGettingPublicKeyAttributes(t *testing.T) { withContext(t, func(ctx *Context) { id := randomBytes() - key, err := ctx.GenerateRSAKeyPair(id, 1024) + key, err := ctx.GenerateRSAKeyPair(id, rsaSize) require.NoError(t, err) - defer key.Delete() + defer func() { _ = key.Delete() }() attrs, err := ctx.GetPubAttributes(key, []AttributeType{CkaModulusBits}) require.NoError(t, err) require.NotNil(t, attrs) require.Len(t, attrs, 1) - require.Equal(t, uint(1024), bytesToUlong(attrs[CkaModulusBits].Value)) + require.Equal(t, uint(rsaSize), bytesToUlong(attrs[CkaModulusBits].Value)) }) } @@ -196,7 +196,7 @@ func TestGettingSecretKeyAttributes(t *testing.T) { key, err := ctx.GenerateSecretKey(id, 128, CipherAES) require.NoError(t, err) - defer key.Delete() + defer func() { _ = key.Delete() }() attrs, err := ctx.GetAttributes(key, []AttributeType{CkaValueLen}) require.NoError(t, err) @@ -209,7 +209,7 @@ func TestGettingSecretKeyAttributes(t *testing.T) { func TestGettingUnsupportedKeyTypeAttributes(t *testing.T) { withContext(t, func(ctx *Context) { - key, err := rsa.GenerateKey(rand.Reader, 1024) + key, err := rsa.GenerateKey(rand.Reader, rsaSize) require.NoError(t, err) _, err = ctx.GetAttributes(key, []AttributeType{CkaModulusBits}) diff --git a/thread_test.go b/thread_test.go index 7398644..2632466 100644 --- a/thread_test.go +++ b/thread_test.go @@ -42,9 +42,9 @@ func TestThreadedRSA(t *testing.T) { }() id := randomBytes() - key, err := ctx.GenerateRSAKeyPair(id, 1024) + key, err := ctx.GenerateRSAKeyPair(id, rsaSize) require.NoError(t, err) - defer key.Delete() + defer func() { _ = key.Delete() }() done := make(chan int) started := time.Now() From 57907c199fb72b79972f87890983e8c4cc429f39 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Mon, 9 Sep 2019 14:28:24 +0100 Subject: [PATCH 09/36] Add DSA flag to disable DSA tests --- README.md | 1 + dsa_test.go | 2 ++ skip_test.go | 1 + 3 files changed, 4 insertions(+) diff --git a/README.md b/README.md index 1afeb6c..427adb8 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,7 @@ list of the following options: * `CERTS` - disables certificate-related tests. Needed for AWS CloudHSM, which doesn't support certificates. * `OAEP_LABEL` - disables RSA OAEP encryption tests that use source data encoding parameter (also known as a 'label' in some crypto libraries). Needed for AWS CloudHSM. +* `DSA` - disables DSA tests. Needed for AWS CloudHSM (and any other tokens not supporting DSA). Testing with SoftHSM2 diff --git a/dsa_test.go b/dsa_test.go index bd25daf..c988504 100644 --- a/dsa_test.go +++ b/dsa_test.go @@ -101,6 +101,8 @@ func TestNativeDSA(t *testing.T) { } func TestHardDSA(t *testing.T) { + skipTest(t, skipTestDSA) + ctx, err := ConfigureFromFile("config") require.NoError(t, err) diff --git a/skip_test.go b/skip_test.go index fbe60cc..40a0f5f 100644 --- a/skip_test.go +++ b/skip_test.go @@ -9,6 +9,7 @@ import ( const skipTestEnv = "CRYPTO11_SKIP" const skipTestCert = "CERTS" const skipTestOAEPLabel = "OAEP_LABEL" +const skipTestDSA = "DSA" // skipTest tests whether the CRYPTO11_SKIP environment variable contains // flagName. If so, it skips the test. From 1cabb4d4b3408fa24348dfaae84a85280b7a0eae Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Mon, 9 Sep 2019 14:40:16 +0100 Subject: [PATCH 10/36] Fix bug with test key deletion --- close_test.go | 7 ++----- keys_test.go | 24 ++++++++++++------------ thread_test.go | 2 +- 3 files changed, 15 insertions(+), 18 deletions(-) diff --git a/close_test.go b/close_test.go index 662d5e9..c66f6f3 100644 --- a/close_test.go +++ b/close_test.go @@ -24,7 +24,6 @@ package crypto11 import ( "crypto/dsa" "crypto/elliptic" - "fmt" "math/rand" "testing" "time" @@ -40,9 +39,8 @@ func TestClose(t *testing.T) { ctx, err := ConfigureFromFile("config") require.NoError(t, err) - const pSize = dsa.L1024N160 id := randomBytes() - _, err = ctx.GenerateDSAKeyPair(id, dsaSizes[pSize]) + _, err = ctx.GenerateRSAKeyPair(id, rsaSize) if err != nil { _ = ctx.Close() t.Fatal(err) @@ -57,8 +55,7 @@ func TestClose(t *testing.T) { key2, err := ctx.FindKeyPair(id, nil) require.NoError(t, err) - testDsaSigning(t, key2.(*pkcs11PrivateKeyDSA), pSize, fmt.Sprintf("close%d", i)) - + testRsaSigning(t, key2, false) if i == 4 { err = key2.Delete() require.NoError(t, err) diff --git a/keys_test.go b/keys_test.go index fe95475..243d70c 100644 --- a/keys_test.go +++ b/keys_test.go @@ -44,15 +44,15 @@ func TestFindingKeysWithAttributes(t *testing.T) { key, err := ctx.GenerateSecretKey(id, 128, CipherAES) require.NoError(t, err) - defer func() { _ = key.Delete() }() + defer func(k *SecretKey) { _ = k.Delete() }(key) key, err = ctx.GenerateSecretKey(id2, 128, CipherAES) require.NoError(t, err) - defer func() { _ = key.Delete() }() + defer func(k *SecretKey) { _ = k.Delete() }(key) key, err = ctx.GenerateSecretKey(id2, 256, CipherAES) require.NoError(t, err) - defer func() { _ = key.Delete() }() + defer func(k *SecretKey) { _ = k.Delete() }(key) attrs, err := NewAttributeSetWithID(id) require.NoError(t, err) @@ -89,15 +89,15 @@ func TestFindingKeyPairsWithAttributes(t *testing.T) { key, err := ctx.GenerateRSAKeyPair(id, rsaSize) require.NoError(t, err) - defer func() { _ = key.Delete() }() + defer func(k Signer) { _ = k.Delete() }(key) key, err = ctx.GenerateRSAKeyPair(id2, rsaSize) require.NoError(t, err) - defer func() { _ = key.Delete() }() + defer func(k Signer) { _ = k.Delete() }(key) key, err = ctx.GenerateRSAKeyPair(id2, rsaSize) require.NoError(t, err) - defer func() { _ = key.Delete() }() + defer func(k Signer) { _ = k.Delete() }(key) attrs, err := NewAttributeSetWithID(id) require.NoError(t, err) @@ -127,7 +127,7 @@ func TestFindingAllKeys(t *testing.T) { key, err := ctx.GenerateSecretKey(id, 128, CipherAES) require.NoError(t, err) - defer func() { _ = key.Delete() }() + defer func(k *SecretKey) { _ = k.Delete() }(key) } keys, err := ctx.FindAllKeys() @@ -145,7 +145,7 @@ func TestFindingAllKeyPairs(t *testing.T) { key, err := ctx.GenerateRSAKeyPair(id, rsaSize) require.NoError(t, err) - defer func() { _ = key.Delete() }() + defer func(k Signer) { _ = k.Delete() }(key) } keys, err := ctx.FindAllKeyPairs() @@ -162,14 +162,14 @@ func TestGettingPrivateKeyAttributes(t *testing.T) { key, err := ctx.GenerateRSAKeyPair(id, rsaSize) require.NoError(t, err) - defer func() { _ = key.Delete() }() + defer func(k Signer) { _ = k.Delete() }(key) attrs, err := ctx.GetAttributes(key, []AttributeType{CkaModulus}) require.NoError(t, err) require.NotNil(t, attrs) require.Len(t, attrs, 1) - require.Len(t, attrs[CkaModulus].Value, 128) + require.Len(t, attrs[CkaModulus].Value, 256) }) } @@ -179,7 +179,7 @@ func TestGettingPublicKeyAttributes(t *testing.T) { key, err := ctx.GenerateRSAKeyPair(id, rsaSize) require.NoError(t, err) - defer func() { _ = key.Delete() }() + defer func(k Signer) { _ = k.Delete() }(key) attrs, err := ctx.GetPubAttributes(key, []AttributeType{CkaModulusBits}) require.NoError(t, err) @@ -196,7 +196,7 @@ func TestGettingSecretKeyAttributes(t *testing.T) { key, err := ctx.GenerateSecretKey(id, 128, CipherAES) require.NoError(t, err) - defer func() { _ = key.Delete() }() + defer func(k *SecretKey) { _ = k.Delete() }(key) attrs, err := ctx.GetAttributes(key, []AttributeType{CkaValueLen}) require.NoError(t, err) diff --git a/thread_test.go b/thread_test.go index 2632466..472c2ee 100644 --- a/thread_test.go +++ b/thread_test.go @@ -44,7 +44,7 @@ func TestThreadedRSA(t *testing.T) { id := randomBytes() key, err := ctx.GenerateRSAKeyPair(id, rsaSize) require.NoError(t, err) - defer func() { _ = key.Delete() }() + defer func(k Signer) { _ = k.Delete() }(key) done := make(chan int) started := time.Now() From fa1aa078ed47e7a8e0c78ee57a3117c63597ab3f Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Mon, 9 Sep 2019 15:14:50 +0100 Subject: [PATCH 11/36] Remove unnecessary loop from test --- close_test.go | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/close_test.go b/close_test.go index c66f6f3..60173b2 100644 --- a/close_test.go +++ b/close_test.go @@ -48,21 +48,15 @@ func TestClose(t *testing.T) { require.NoError(t, ctx.Close()) - for i := 0; i < 5; i++ { - ctx, err := ConfigureFromFile("config") - require.NoError(t, err) - - key2, err := ctx.FindKeyPair(id, nil) - require.NoError(t, err) + ctx, err = ConfigureFromFile("config") + require.NoError(t, err) - testRsaSigning(t, key2, false) - if i == 4 { - err = key2.Delete() - require.NoError(t, err) - } + key2, err := ctx.FindKeyPair(id, nil) + require.NoError(t, err) - require.NoError(t, ctx.Close()) - } + testRsaSigning(t, key2, false) + _ = key2.Delete() + require.NoError(t, ctx.Close()) } // randomBytes returns 32 random bytes. From fe44dd995427f70562f43c9afc379bbf0f7b678c Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Tue, 10 Sep 2019 12:57:02 +0100 Subject: [PATCH 12/36] Handle cases where empty attributes are forbidden --- keys.go | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/keys.go b/keys.go index 88e3d8c..70dd822 100644 --- a/keys.go +++ b/keys.go @@ -23,6 +23,7 @@ package crypto11 import ( "crypto" + "github.com/miekg/pkcs11" "github.com/pkg/errors" ) @@ -122,11 +123,39 @@ func (c *Context) makeKeyPair(session *pkcs11Session, privHandle *pkcs11.ObjectH return nil, errNoCkaId } + var pubHandle *pkcs11.ObjectHandle + // Find the public half which has a matching CKA_ID - pubHandle, err := findKey(session, id, label, uintPtr(pkcs11.CKO_PUBLIC_KEY), &keyType) + pubHandle, err = findKey(session, id, label, uintPtr(pkcs11.CKO_PUBLIC_KEY), &keyType) if err != nil { - return nil, err + p11Err, ok := err.(pkcs11.Error) + + if len(label) == 0 && ok && p11Err == pkcs11.CKR_TEMPLATE_INCONSISTENT { + // This probably means we are using a token that doesn't like us passing empty attributes in a template. + // For instance CloudHSM cannot search for a key with CKA_LABEL="". So if the private key doesn't have a + // label, we need to pass nil into findKeys, then match against the first key without a label. + + pubHandles, err := findKeys(session, id, nil, uintPtr(pkcs11.CKO_PUBLIC_KEY), &keyType) + if err != nil { + return nil, err + } + + for _, handle := range pubHandles { + template := []*pkcs11.Attribute{pkcs11.NewAttribute(pkcs11.CKA_LABEL, nil)} + template, err = session.ctx.GetAttributeValue(session.handle, handle, template) + if err != nil { + return nil, err + } + if len(template[0].Value) == 0 { + pubHandle = &handle + break + } + } + } else { + return nil, err + } } + if pubHandle == nil { // We can't return a Signer if we don't have private and public key. Treat it as an error. return nil, errNoPublicHalf @@ -563,4 +592,4 @@ func (c *Context) GetPubAttribute(key interface{}, attribute AttributeType) (a * } return set[attribute], nil -} \ No newline at end of file +} From c0f7454991ec4ec59ab1a2bb488e50b29f57a5d5 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Tue, 10 Sep 2019 14:09:41 +0100 Subject: [PATCH 13/36] Remove redundant test (which used DSA) --- close_test.go | 39 --------------------------------------- crypto11_test.go | 45 +++++++++++++++++++++++---------------------- 2 files changed, 23 insertions(+), 61 deletions(-) diff --git a/close_test.go b/close_test.go index 60173b2..c1acfc9 100644 --- a/close_test.go +++ b/close_test.go @@ -24,52 +24,13 @@ package crypto11 import ( "crypto/dsa" "crypto/elliptic" - "math/rand" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func TestClose(t *testing.T) { - // Verify that close and re-open works. - - ctx, err := ConfigureFromFile("config") - require.NoError(t, err) - - id := randomBytes() - _, err = ctx.GenerateRSAKeyPair(id, rsaSize) - if err != nil { - _ = ctx.Close() - t.Fatal(err) - } - - require.NoError(t, ctx.Close()) - - ctx, err = ConfigureFromFile("config") - require.NoError(t, err) - - key2, err := ctx.FindKeyPair(id, nil) - require.NoError(t, err) - - testRsaSigning(t, key2, false) - _ = key2.Delete() - require.NoError(t, ctx.Close()) -} - -// randomBytes returns 32 random bytes. -func randomBytes() []byte { - result := make([]byte, 32) - rand.Read(result) - return result -} - -func init() { - rand.Seed(time.Now().UnixNano()) -} - func TestErrorAfterClosed(t *testing.T) { ctx, err := ConfigureFromFile("config") require.NoError(t, err) diff --git a/crypto11_test.go b/crypto11_test.go index 2c1297d..0a4cce4 100644 --- a/crypto11_test.go +++ b/crypto11_test.go @@ -22,7 +22,6 @@ package crypto11 import ( - "crypto/dsa" "encoding/json" "fmt" "log" @@ -39,37 +38,28 @@ import ( ) func TestKeysPersistAcrossContexts(t *testing.T) { - ctx, err := configureWithPin(t) + // Verify that close and re-open works. + ctx, err := ConfigureFromFile("config") require.NoError(t, err) - defer func() { - err = ctx.Close() - require.NoError(t, err) - }() - - // Generate a key and and close a session - const pSize = dsa.L1024N160 id := randomBytes() - key, err := ctx.GenerateDSAKeyPair(id, dsaSizes[pSize]) - require.NoError(t, err) - require.NotNil(t, key) + _, err = ctx.GenerateRSAKeyPair(id, rsaSize) + if err != nil { + _ = ctx.Close() + t.Fatal(err) + } - err = ctx.Close() - require.NoError(t, err) + require.NoError(t, ctx.Close()) - // Reopen a session and try to find a key. - // Valid session must enlist a key. - // If login is not performed than it will fail. - ctx, err = configureWithPin(t) + ctx, err = ConfigureFromFile("config") require.NoError(t, err) key2, err := ctx.FindKeyPair(id, nil) require.NoError(t, err) - testDsaSigning(t, key2.(*pkcs11PrivateKeyDSA), pSize, fmt.Sprintf("close%d", 0)) - - err = key2.Delete() - require.NoError(t, err) + testRsaSigning(t, key2, false) + _ = key2.Delete() + require.NoError(t, ctx.Close()) } func configureWithPin(t *testing.T) (*Context, error) { @@ -287,3 +277,14 @@ func TestNoLogin(t *testing.T) { assert.Equal(t, pkcs11.Error(pkcs11.CKR_USER_NOT_LOGGED_IN), p11Err) } + +// randomBytes returns 32 random bytes. +func randomBytes() []byte { + result := make([]byte, 32) + rand.Read(result) + return result +} + +func init() { + rand.Seed(time.Now().UnixNano()) +} From 62c948e87f87c0caf81e73e14a22d7e63b0fe55f Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Tue, 10 Sep 2019 14:44:03 +0100 Subject: [PATCH 14/36] Use assertions to get more info from test --- ecdsa_test.go | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/ecdsa_test.go b/ecdsa_test.go index dde920e..194cd9d 100644 --- a/ecdsa_test.go +++ b/ecdsa_test.go @@ -31,6 +31,8 @@ import ( _ "crypto/sha512" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) @@ -50,11 +52,11 @@ func TestNativeECDSA(t *testing.T) { t.Errorf("crypto.ecdsa.GenerateKey: %v", err) return } - testEcdsaSigning(t, key, crypto.SHA1) - testEcdsaSigning(t, key, crypto.SHA224) - testEcdsaSigning(t, key, crypto.SHA256) - testEcdsaSigning(t, key, crypto.SHA384) - testEcdsaSigning(t, key, crypto.SHA512) + testEcdsaSigning(t, key, crypto.SHA1, curve.Params().Name, "SHA-1") + testEcdsaSigning(t, key, crypto.SHA224, curve.Params().Name, "SHA-224") + testEcdsaSigning(t, key, crypto.SHA256, curve.Params().Name, "SHA-256") + testEcdsaSigning(t, key, crypto.SHA384, curve.Params().Name, "SHA-384") + testEcdsaSigning(t, key, crypto.SHA512, curve.Params().Name, "SHA-512") } } @@ -76,32 +78,37 @@ func TestHardECDSA(t *testing.T) { require.NotNil(t, key) defer key.Delete() - testEcdsaSigning(t, key, crypto.SHA1) - testEcdsaSigning(t, key, crypto.SHA224) - testEcdsaSigning(t, key, crypto.SHA256) - testEcdsaSigning(t, key, crypto.SHA384) - testEcdsaSigning(t, key, crypto.SHA512) + testEcdsaSigning(t, key, crypto.SHA1, curve.Params().Name, "SHA-1") + testEcdsaSigning(t, key, crypto.SHA224, curve.Params().Name, "SHA-224") + testEcdsaSigning(t, key, crypto.SHA256, curve.Params().Name, "SHA-256") + testEcdsaSigning(t, key, crypto.SHA384, curve.Params().Name, "SHA-384") + testEcdsaSigning(t, key, crypto.SHA512, curve.Params().Name, "SHA-512") key2, err := ctx.FindKeyPair(id, nil) require.NoError(t, err) - testEcdsaSigning(t, key2.(*pkcs11PrivateKeyECDSA), crypto.SHA256) + testEcdsaSigning(t, key2.(*pkcs11PrivateKeyECDSA), crypto.SHA256, curve.Params().Name, "SHA-256") key3, err := ctx.FindKeyPair(nil, label) require.NoError(t, err) - testEcdsaSigning(t, key3.(crypto.Signer), crypto.SHA384) + testEcdsaSigning(t, key3.(crypto.Signer), crypto.SHA384, curve.Params().Name, "SHA-384") } } -func testEcdsaSigning(t *testing.T, key crypto.Signer, hashFunction crypto.Hash) { +func testEcdsaSigning(t *testing.T, key crypto.Signer, hashFunction crypto.Hash, curveName, hashName string) { plaintext := []byte("sign me with ECDSA") h := hashFunction.New() _, err := h.Write(plaintext) require.NoError(t, err) - plaintextHash := h.Sum([]byte{}) // weird API + plaintextHash := h.Sum(nil) sigDER, err := key.Sign(rand.Reader, plaintextHash, nil) - require.NoError(t, err) + assert.NoErrorf(t, err, "Sign failed for curve %s and hash %s", curveName, hashName) + if err != nil { + // We assert and return, so that errors are more informative over a range of curves + // and hashes. + return + } var sig dsaSignature err = sig.unmarshalDER(sigDER) From 9461330c06d010d509c65acb143e6c02f83a0ffb Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Tue, 10 Sep 2019 15:06:30 +0100 Subject: [PATCH 15/36] Skip ECDSA keys that the token cannot handle --- ecdsa_test.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ecdsa_test.go b/ecdsa_test.go index 194cd9d..43407e3 100644 --- a/ecdsa_test.go +++ b/ecdsa_test.go @@ -31,6 +31,8 @@ import ( _ "crypto/sha512" "testing" + "github.com/miekg/pkcs11" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -76,7 +78,7 @@ func TestHardECDSA(t *testing.T) { key, err := ctx.GenerateECDSAKeyPairWithLabel(id, label, curve) require.NoError(t, err) require.NotNil(t, key) - defer key.Delete() + defer func(k Signer) { _ = k.Delete() }(key) testEcdsaSigning(t, key, crypto.SHA1, curve.Params().Name, "SHA-1") testEcdsaSigning(t, key, crypto.SHA224, curve.Params().Name, "SHA-224") @@ -103,6 +105,14 @@ func testEcdsaSigning(t *testing.T, key crypto.Signer, hashFunction crypto.Hash, plaintextHash := h.Sum(nil) sigDER, err := key.Sign(rand.Reader, plaintextHash, nil) + + p11Err, ok := err.(pkcs11.Error) + if ok && p11Err == pkcs11.CKR_KEY_SIZE_RANGE { + // Returned by CloudHSM (at least), for key sizes it doesn't support. + t.Logf("Skipping unsupported curve %s and hash %s", curveName, hashName) + return + } + assert.NoErrorf(t, err, "Sign failed for curve %s and hash %s", curveName, hashName) if err != nil { // We assert and return, so that errors are more informative over a range of curves From 6173964f8ad4aa8cdc408a67a9bc0d5e8480cea5 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Tue, 10 Sep 2019 15:22:48 +0100 Subject: [PATCH 16/36] Retry HMAC key generation on AWS CloudHSM --- symmetric.go | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/symmetric.go b/symmetric.go index bb61a08..485986c 100644 --- a/symmetric.go +++ b/symmetric.go @@ -294,7 +294,7 @@ func (c *Context) GenerateSecretKeyWithAttributes(template AttributeSet, bits in // CKK_*_HMAC exists but there is no specific corresponding CKM_*_KEY_GEN // mechanism. Therefore we attempt both CKM_GENERIC_SECRET_KEY_GEN and // vendor-specific mechanisms. - for _, genMech := range cipher.GenParams { + for n, genMech := range cipher.GenParams { template.AddIfNotPresent([]*pkcs11.Attribute{ pkcs11.NewAttribute(pkcs11.CKA_CLASS, pkcs11.CKO_SECRET_KEY), pkcs11.NewAttribute(pkcs11.CKA_KEY_TYPE, genMech.KeyType), @@ -308,7 +308,7 @@ func (c *Context) GenerateSecretKeyWithAttributes(template AttributeSet, bits in }) if bits > 0 { - template.Set(pkcs11.CKA_VALUE_LEN, bits/8) + _ = template.Set(pkcs11.CKA_VALUE_LEN, bits/8) // safe for an int } mech := []*pkcs11.Mechanism{pkcs11.NewMechanism(genMech.GenMech, nil)} @@ -319,8 +319,16 @@ func (c *Context) GenerateSecretKeyWithAttributes(template AttributeSet, bits in return nil } - // nShield returns this if if doesn't like the CKK/CKM combination. - if e, ok := err.(pkcs11.Error); ok && e == pkcs11.CKR_TEMPLATE_INCONSISTENT { + if n == len(cipher.GenParams)-1 { + // If we have tried all available gen params, we should return a sensible error. So we skip the + // retry logic below and return directly. + return err + } + + // nShield returns CKR_TEMPLATE_INCONSISTENT if if doesn't like the CKK/CKM combination. + // AWS CloudHSM returns CKR_ATTRIBUTE_VALUE_INVALID in the same circumstances. + if e, ok := err.(pkcs11.Error); ok && + e == pkcs11.CKR_TEMPLATE_INCONSISTENT || e == pkcs11.CKR_ATTRIBUTE_VALUE_INVALID { continue } From d992c782d9e1f9c1270ac0d59acc7c34a4a0f147 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Wed, 11 Sep 2019 10:08:48 +0100 Subject: [PATCH 17/36] Retry HMAC gen without encrypt/decrypt perms --- attributes.go | 6 ++++++ symmetric.go | 17 +++++++++++++++-- 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/attributes.go b/attributes.go index 8bbf1a8..4ff9bd2 100644 --- a/attributes.go +++ b/attributes.go @@ -221,6 +221,12 @@ func (a AttributeSet) Copy() AttributeSet { return b } +// Unset removes an attribute from the attributes set. If the set does not contain the attribute, this +// is a no-op. +func (a AttributeSet) Unset(attributeType AttributeType) { + delete(a, attributeType) +} + // NewAttributeSetWithID is a helper function that populates a new slice of Attributes with the provided ID. // This function returns an error if the ID is an empty slice. func NewAttributeSetWithID(id []byte) (AttributeSet, error) { diff --git a/symmetric.go b/symmetric.go index 485986c..53846e7 100644 --- a/symmetric.go +++ b/symmetric.go @@ -301,8 +301,8 @@ func (c *Context) GenerateSecretKeyWithAttributes(template AttributeSet, bits in pkcs11.NewAttribute(pkcs11.CKA_TOKEN, true), pkcs11.NewAttribute(pkcs11.CKA_SIGN, cipher.MAC), pkcs11.NewAttribute(pkcs11.CKA_VERIFY, cipher.MAC), - pkcs11.NewAttribute(pkcs11.CKA_ENCRYPT, cipher.Encrypt), - pkcs11.NewAttribute(pkcs11.CKA_DECRYPT, cipher.Encrypt), + pkcs11.NewAttribute(pkcs11.CKA_ENCRYPT, cipher.Encrypt), // Not supported on CloudHSM + pkcs11.NewAttribute(pkcs11.CKA_DECRYPT, cipher.Encrypt), // Not supported on CloudHSM pkcs11.NewAttribute(pkcs11.CKA_SENSITIVE, true), pkcs11.NewAttribute(pkcs11.CKA_EXTRACTABLE, false), }) @@ -319,6 +319,19 @@ func (c *Context) GenerateSecretKeyWithAttributes(template AttributeSet, bits in return nil } + // As a special case, AWS CloudHSM does not accept CKA_ENCRYPT and CKA_DECRYPT on a + // Generic Secret key. If we are in that special case, try again without those attributes. + if e, ok := err.(pkcs11.Error); ok && e == pkcs11.CKR_ARGUMENTS_BAD && genMech.GenMech == pkcs11.CKM_GENERIC_SECRET_KEY_GEN { + template.Unset(CkaEncrypt) + template.Unset(CkaDecrypt) + + privHandle, err := session.ctx.GenerateKey(session.handle, mech, template.ToSlice()) + if err == nil { + k = &SecretKey{pkcs11Object{privHandle, c}, cipher} + return nil + } + } + if n == len(cipher.GenParams)-1 { // If we have tried all available gen params, we should return a sensible error. So we skip the // retry logic below and return directly. From a1d816f146d9c2723310c796e9ee6e67be093c04 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Thu, 12 Sep 2019 12:05:43 +0100 Subject: [PATCH 18/36] Add String() function to AttributeSet --- attributes.go | 245 +++++++++++++++++++++++++++++++++++++++++++++++++- symmetric.go | 3 + 2 files changed, 247 insertions(+), 1 deletion(-) diff --git a/attributes.go b/attributes.go index 4ff9bd2..b362a59 100644 --- a/attributes.go +++ b/attributes.go @@ -3,6 +3,7 @@ package crypto11 import ( "errors" "fmt" + "strings" "github.com/miekg/pkcs11" ) @@ -13,7 +14,7 @@ type AttributeType = uint // Attribute represents a PKCS#11 CK_ATTRIBUTE type. type Attribute = pkcs11.Attribute -//noinspection GoUnusedConst +//noinspection GoUnusedConst,GoDeprecation const ( CkaClass = AttributeType(0x00000000) CkaToken = AttributeType(0x00000001) @@ -227,6 +228,14 @@ func (a AttributeSet) Unset(attributeType AttributeType) { delete(a, attributeType) } +func (a AttributeSet) String() string { + result := new(strings.Builder) + for attr, value := range a { + _, _ = fmt.Fprintf(result, "%s: %x\n", attributeTypeString(attr), value.Value) + } + return result.String() +} + // NewAttributeSetWithID is a helper function that populates a new slice of Attributes with the provided ID. // This function returns an error if the ID is an empty slice. func NewAttributeSetWithID(id []byte) (AttributeSet, error) { @@ -252,3 +261,237 @@ func NewAttributeSetWithIDAndLabel(id, label []byte) (a AttributeSet, err error) _ = a.Set(CkaLabel, label) // error not possible with []byte return a, nil } + +func attributeTypeString(a AttributeType) string { + //noinspection GoDeprecation + switch a { + case CkaClass: + return "CkaClass" + case CkaToken: + return "CkaToken" + case CkaPrivate: + return "CkaPrivate" + case CkaLabel: + return "CkaLabel" + case CkaApplication: + return "CkaApplication" + case CkaValue: + return "CkaValue" + case CkaObjectId: + return "CkaObjectId" + case CkaCertificateType: + return "CkaCertificateType" + case CkaIssuer: + return "CkaIssuer" + case CkaSerialNumber: + return "CkaSerialNumber" + case CkaAcIssuer: + return "CkaAcIssuer" + case CkaOwner: + return "CkaOwner" + case CkaAttrTypes: + return "CkaAttrTypes" + case CkaTrusted: + return "CkaTrusted" + case CkaCertificateCategory: + return "CkaCertificateCategory" + case CkaJavaMIDPSecurityDomain: + return "CkaJavaMIDPSecurityDomain" + case CkaUrl: + return "CkaUrl" + case CkaHashOfSubjectPublicKey: + return "CkaHashOfSubjectPublicKey" + case CkaHashOfIssuerPublicKey: + return "CkaHashOfIssuerPublicKey" + case CkaNameHashAlgorithm: + return "CkaNameHashAlgorithm" + case CkaCheckValue: + return "CkaCheckValue" + + case CkaKeyType: + return "CkaKeyType" + case CkaSubject: + return "CkaSubject" + case CkaId: + return "CkaId" + case CkaSensitive: + return "CkaSensitive" + case CkaEncrypt: + return "CkaEncrypt" + case CkaDecrypt: + return "CkaDecrypt" + case CkaWrap: + return "CkaWrap" + case CkaUnwrap: + return "CkaUnwrap" + case CkaSign: + return "CkaSign" + case CkaSignRecover: + return "CkaSignRecover" + case CkaVerify: + return "CkaVerify" + case CkaVerifyRecover: + return "CkaVerifyRecover" + case CkaDerive: + return "CkaDerive" + case CkaStartDate: + return "CkaStartDate" + case CkaEndDate: + return "CkaEndDate" + case CkaModulus: + return "CkaModulus" + case CkaModulusBits: + return "CkaModulusBits" + case CkaPublicExponent: + return "CkaPublicExponent" + case CkaPrivateExponent: + return "CkaPrivateExponent" + case CkaPrime1: + return "CkaPrime1" + case CkaPrime2: + return "CkaPrime2" + case CkaExponent1: + return "CkaExponent1" + case CkaExponent2: + return "CkaExponent2" + case CkaCoefficient: + return "CkaCoefficient" + case CkaPublicKeyInfo: + return "CkaPublicKeyInfo" + case CkaPrime: + return "CkaPrime" + case CkaSubprime: + return "CkaSubprime" + case CkaBase: + return "CkaBase" + + case CkaPrimeBits: + return "CkaPrimeBits" + case CkaSubprimeBits: + return "CkaSubprimeBits" + + case CkaValueBits: + return "CkaValueBits" + case CkaValueLen: + return "CkaValueLen" + case CkaExtractable: + return "CkaExtractable" + case CkaLocal: + return "CkaLocal" + case CkaNeverExtractable: + return "CkaNeverExtractable" + case CkaAlwaysSensitive: + return "CkaAlwaysSensitive" + case CkaKeyGenMechanism: + return "CkaKeyGenMechanism" + + case CkaModifiable: + return "CkaModifiable" + case CkaCopyable: + return "CkaCopyable" + + case CkaDestroyable: + return "CkaDestroyable" + + case CkaEcParams: + return "CkaEcParams" + + case CkaEcPoint: + return "CkaEcPoint" + + case CkaSecondaryAuth: + return "CkaSecondaryAuth" + case CkaAuthPinFlags: + return "CkaAuthPinFlags" + + case CkaAlwaysAuthenticate: + return "CkaAlwaysAuthenticate" + + case CkaWrapWithTrusted: + return "CkaWrapWithTrusted" + + case ckfArrayAttribute: + return "ckfArrayAttribute" + + case CkaWrapTemplate: + return "CkaWrapTemplate" + case CkaUnwrapTemplate: + return "CkaUnwrapTemplate" + + case CkaOtpFormat: + return "CkaOtpFormat" + case CkaOtpLength: + return "CkaOtpLength" + case CkaOtpTimeInterval: + return "CkaOtpTimeInterval" + case CkaOtpUserFriendlyMode: + return "CkaOtpUserFriendlyMode" + case CkaOtpChallengeRequirement: + return "CkaOtpChallengeRequirement" + case CkaOtpTimeRequirement: + return "CkaOtpTimeRequirement" + case CkaOtpCounterRequirement: + return "CkaOtpCounterRequirement" + case CkaOtpPinRequirement: + return "CkaOtpPinRequirement" + case CkaOtpCounter: + return "CkaOtpCounter" + case CkaOtpTime: + return "CkaOtpTime" + case CkaOtpUserIdentifier: + return "CkaOtpUserIdentifier" + case CkaOtpServiceIdentifier: + return "CkaOtpServiceIdentifier" + case CkaOtpServiceLogo: + return "CkaOtpServiceLogo" + case CkaOtpServiceLogoType: + return "CkaOtpServiceLogoType" + + case CkaGOSTR3410Params: + return "CkaGOSTR3410Params" + case CkaGOSTR3411Params: + return "CkaGOSTR3411Params" + case CkaGOST28147Params: + return "CkaGOST28147Params" + + case CkaHwFeatureType: + return "CkaHwFeatureType" + case CkaResetOnInit: + return "CkaResetOnInit" + case CkaHasReset: + return "CkaHasReset" + + case CkaPixelX: + return "CkaPixelX" + case CkaPixelY: + return "CkaPixelY" + case CkaResolution: + return "CkaResolution" + case CkaCharRows: + return "CkaCharRows" + case CkaCharColumns: + return "CkaCharColumns" + case CkaColor: + return "CkaColor" + case CkaBitsPerPixel: + return "CkaBitsPerPixel" + case CkaCharSets: + return "CkaCharSets" + case CkaEncodingMethods: + return "CkaEncodingMethods" + case CkaMimeTypes: + return "CkaMimeTypes" + case CkaMechanismType: + return "CkaMechanismType" + case CkaRequiredCmsAttributes: + return "CkaRequiredCmsAttributes" + case CkaDefaultCmsAttributes: + return "CkaDefaultCmsAttributes" + case CkaSupportedCmsAttributes: + return "CkaSupportedCmsAttributes" + case CkaAllowedMechanisms: + return "CkaAllowedMechanisms" + default: + return "Unknown" + } +} diff --git a/symmetric.go b/symmetric.go index 53846e7..08bcf29 100644 --- a/symmetric.go +++ b/symmetric.go @@ -23,6 +23,7 @@ package crypto11 import ( "errors" + "fmt" "github.com/miekg/pkcs11" ) @@ -325,6 +326,8 @@ func (c *Context) GenerateSecretKeyWithAttributes(template AttributeSet, bits in template.Unset(CkaEncrypt) template.Unset(CkaDecrypt) + fmt.Println("Adjusted template:\n", template) + privHandle, err := session.ctx.GenerateKey(session.handle, mech, template.ToSlice()) if err == nil { k = &SecretKey{pkcs11Object{privHandle, c}, cipher} From 42f22a067c71331f8a2872b86cca545b25ac806f Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Thu, 12 Sep 2019 12:12:16 +0100 Subject: [PATCH 19/36] Fix error handling bug --- symmetric.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/symmetric.go b/symmetric.go index 08bcf29..a840766 100644 --- a/symmetric.go +++ b/symmetric.go @@ -328,7 +328,7 @@ func (c *Context) GenerateSecretKeyWithAttributes(template AttributeSet, bits in fmt.Println("Adjusted template:\n", template) - privHandle, err := session.ctx.GenerateKey(session.handle, mech, template.ToSlice()) + privHandle, err = session.ctx.GenerateKey(session.handle, mech, template.ToSlice()) if err == nil { k = &SecretKey{pkcs11Object{privHandle, c}, cipher} return nil From 2ba5092bccd795b9006f653e821af038017683d9 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Thu, 12 Sep 2019 12:16:59 +0100 Subject: [PATCH 20/36] Don't overwrite existing template --- symmetric.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/symmetric.go b/symmetric.go index a840766..88ce461 100644 --- a/symmetric.go +++ b/symmetric.go @@ -323,12 +323,13 @@ func (c *Context) GenerateSecretKeyWithAttributes(template AttributeSet, bits in // As a special case, AWS CloudHSM does not accept CKA_ENCRYPT and CKA_DECRYPT on a // Generic Secret key. If we are in that special case, try again without those attributes. if e, ok := err.(pkcs11.Error); ok && e == pkcs11.CKR_ARGUMENTS_BAD && genMech.GenMech == pkcs11.CKM_GENERIC_SECRET_KEY_GEN { - template.Unset(CkaEncrypt) - template.Unset(CkaDecrypt) + adjustedTemplate := template.Copy() + adjustedTemplate.Unset(CkaEncrypt) + adjustedTemplate.Unset(CkaDecrypt) - fmt.Println("Adjusted template:\n", template) + fmt.Println("Adjusted template:\n", adjustedTemplate) - privHandle, err = session.ctx.GenerateKey(session.handle, mech, template.ToSlice()) + privHandle, err = session.ctx.GenerateKey(session.handle, mech, adjustedTemplate.ToSlice()) if err == nil { k = &SecretKey{pkcs11Object{privHandle, c}, cipher} return nil From e44873e420fe43207183186ef55d6480bd34987b Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Thu, 12 Sep 2019 13:33:36 +0100 Subject: [PATCH 21/36] Fix template bug that caused wrong key type --- attributes.go | 13 +++++++++++++ symmetric.go | 35 ++++++++++++++++++++--------------- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/attributes.go b/attributes.go index b362a59..17ae575 100644 --- a/attributes.go +++ b/attributes.go @@ -192,6 +192,19 @@ func (a AttributeSet) Set(attributeType AttributeType, value interface{}) error return nil } +// cloneFrom make this AttributeSet a clone of the supplied set. Values are deep copied. +func (a AttributeSet) cloneFrom(set AttributeSet) { + for key := range a { + delete(a, key) + } + + // Use Copy to do the deep cloning for us + c := set.Copy() + for k, v := range c { + a[k] = v + } +} + // AddIfNotPresent adds the attributes if the Attribute Type is not already present in the AttributeSet. func (a AttributeSet) AddIfNotPresent(additional []*Attribute) { for _, additionalAttr := range additional { diff --git a/symmetric.go b/symmetric.go index 88ce461..0c316cf 100644 --- a/symmetric.go +++ b/symmetric.go @@ -295,22 +295,24 @@ func (c *Context) GenerateSecretKeyWithAttributes(template AttributeSet, bits in // CKK_*_HMAC exists but there is no specific corresponding CKM_*_KEY_GEN // mechanism. Therefore we attempt both CKM_GENERIC_SECRET_KEY_GEN and // vendor-specific mechanisms. + + template.AddIfNotPresent([]*pkcs11.Attribute{ + pkcs11.NewAttribute(pkcs11.CKA_CLASS, pkcs11.CKO_SECRET_KEY), + pkcs11.NewAttribute(pkcs11.CKA_TOKEN, true), + pkcs11.NewAttribute(pkcs11.CKA_SIGN, cipher.MAC), + pkcs11.NewAttribute(pkcs11.CKA_VERIFY, cipher.MAC), + pkcs11.NewAttribute(pkcs11.CKA_ENCRYPT, cipher.Encrypt), // Not supported on CloudHSM + pkcs11.NewAttribute(pkcs11.CKA_DECRYPT, cipher.Encrypt), // Not supported on CloudHSM + pkcs11.NewAttribute(pkcs11.CKA_SENSITIVE, true), + pkcs11.NewAttribute(pkcs11.CKA_EXTRACTABLE, false), + }) + if bits > 0 { + _ = template.Set(pkcs11.CKA_VALUE_LEN, bits/8) // safe for an int + } + for n, genMech := range cipher.GenParams { - template.AddIfNotPresent([]*pkcs11.Attribute{ - pkcs11.NewAttribute(pkcs11.CKA_CLASS, pkcs11.CKO_SECRET_KEY), - pkcs11.NewAttribute(pkcs11.CKA_KEY_TYPE, genMech.KeyType), - pkcs11.NewAttribute(pkcs11.CKA_TOKEN, true), - pkcs11.NewAttribute(pkcs11.CKA_SIGN, cipher.MAC), - pkcs11.NewAttribute(pkcs11.CKA_VERIFY, cipher.MAC), - pkcs11.NewAttribute(pkcs11.CKA_ENCRYPT, cipher.Encrypt), // Not supported on CloudHSM - pkcs11.NewAttribute(pkcs11.CKA_DECRYPT, cipher.Encrypt), // Not supported on CloudHSM - pkcs11.NewAttribute(pkcs11.CKA_SENSITIVE, true), - pkcs11.NewAttribute(pkcs11.CKA_EXTRACTABLE, false), - }) - - if bits > 0 { - _ = template.Set(pkcs11.CKA_VALUE_LEN, bits/8) // safe for an int - } + + _ = template.Set(CkaKeyType, genMech.KeyType) mech := []*pkcs11.Mechanism{pkcs11.NewMechanism(genMech.GenMech, nil)} @@ -331,6 +333,9 @@ func (c *Context) GenerateSecretKeyWithAttributes(template AttributeSet, bits in privHandle, err = session.ctx.GenerateKey(session.handle, mech, adjustedTemplate.ToSlice()) if err == nil { + // Store the actual attributes + template.cloneFrom(adjustedTemplate) + k = &SecretKey{pkcs11Object{privHandle, c}, cipher} return nil } From aee21bdd44ba7938006dce489c6a563008b9d14c Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Thu, 12 Sep 2019 13:43:38 +0100 Subject: [PATCH 22/36] Remove println --- symmetric.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/symmetric.go b/symmetric.go index 0c316cf..4a01248 100644 --- a/symmetric.go +++ b/symmetric.go @@ -23,7 +23,6 @@ package crypto11 import ( "errors" - "fmt" "github.com/miekg/pkcs11" ) @@ -329,8 +328,6 @@ func (c *Context) GenerateSecretKeyWithAttributes(template AttributeSet, bits in adjustedTemplate.Unset(CkaEncrypt) adjustedTemplate.Unset(CkaDecrypt) - fmt.Println("Adjusted template:\n", adjustedTemplate) - privHandle, err = session.ctx.GenerateKey(session.handle, mech, adjustedTemplate.ToSlice()) if err == nil { // Store the actual attributes From cde36544026675e6bb6d27b9bf22dbf14d704f86 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Thu, 12 Sep 2019 13:51:34 +0100 Subject: [PATCH 23/36] Skip unsupported HMAC mechanisms --- hmac_test.go | 2 ++ rsa_test.go | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/hmac_test.go b/hmac_test.go index 6762c92..403baee 100644 --- a/hmac_test.go +++ b/hmac_test.go @@ -57,6 +57,8 @@ func TestHmac(t *testing.T) { func testHmac(t *testing.T, ctx *Context, keytype int, mech int, length int, xlength int, full bool) { + skipIfMechUnsupported(t, ctx, uint(mech)) + id := randomBytes() key, err := ctx.GenerateSecretKey(id, 256, Ciphers[keytype]) require.NoError(t, err) diff --git a/rsa_test.go b/rsa_test.go index 8fa7d91..c5ac06f 100644 --- a/rsa_test.go +++ b/rsa_test.go @@ -240,7 +240,7 @@ func skipIfMechUnsupported(t *testing.T, ctx *Context, wantMech uint) { return } } - t.Skipf("mechanism %v not supported", wantMech) + t.Skipf("mechanism 0x%x not supported", wantMech) } func TestRsaRequiredArgs(t *testing.T) { From 6b251482f6a70fd03c60a10f22d3e6ece6a640a7 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Thu, 12 Sep 2019 14:19:41 +0100 Subject: [PATCH 24/36] Use labels not IDs in test, to appease CloudHSM --- keys_test.go | 42 ++++++++++++++++++++---------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/keys_test.go b/keys_test.go index 243d70c..aaa0270 100644 --- a/keys_test.go +++ b/keys_test.go @@ -39,30 +39,27 @@ func TestFindKeysRequiresIdOrLabel(t *testing.T) { func TestFindingKeysWithAttributes(t *testing.T) { withContext(t, func(ctx *Context) { - id := randomBytes() - id2 := randomBytes() + label := randomBytes() + label2 := randomBytes() - key, err := ctx.GenerateSecretKey(id, 128, CipherAES) + key, err := ctx.GenerateSecretKeyWithLabel(randomBytes(), label, 128, CipherAES) require.NoError(t, err) defer func(k *SecretKey) { _ = k.Delete() }(key) - key, err = ctx.GenerateSecretKey(id2, 128, CipherAES) + key, err = ctx.GenerateSecretKeyWithLabel(randomBytes(), label2, 128, CipherAES) require.NoError(t, err) defer func(k *SecretKey) { _ = k.Delete() }(key) - key, err = ctx.GenerateSecretKey(id2, 256, CipherAES) + key, err = ctx.GenerateSecretKeyWithLabel(randomBytes(), label2, 256, CipherAES) require.NoError(t, err) defer func(k *SecretKey) { _ = k.Delete() }(key) - attrs, err := NewAttributeSetWithID(id) - require.NoError(t, err) - + attrs := NewAttributeSet() + _ = attrs.Set(CkaLabel, label) keys, err := ctx.FindKeysWithAttributes(attrs) require.Len(t, keys, 1) - attrs, err = NewAttributeSetWithID(id2) - require.NoError(t, err) - + _ = attrs.Set(CkaLabel, label2) keys, err = ctx.FindKeysWithAttributes(attrs) require.Len(t, keys, 2) @@ -84,30 +81,31 @@ func TestFindingKeysWithAttributes(t *testing.T) { func TestFindingKeyPairsWithAttributes(t *testing.T) { withContext(t, func(ctx *Context) { - id := randomBytes() - id2 := randomBytes() - key, err := ctx.GenerateRSAKeyPair(id, rsaSize) - require.NoError(t, err) - defer func(k Signer) { _ = k.Delete() }(key) + // Note: we use common labels, not IDs in this test code. AWS CloudHSM + // does not accept two keys with the same ID. + + label := randomBytes() + label2 := randomBytes() - key, err = ctx.GenerateRSAKeyPair(id2, rsaSize) + key, err := ctx.GenerateRSAKeyPairWithLabel(randomBytes(), label, rsaSize) require.NoError(t, err) defer func(k Signer) { _ = k.Delete() }(key) - key, err = ctx.GenerateRSAKeyPair(id2, rsaSize) + key, err = ctx.GenerateRSAKeyPairWithLabel(randomBytes(), label2, rsaSize) require.NoError(t, err) defer func(k Signer) { _ = k.Delete() }(key) - attrs, err := NewAttributeSetWithID(id) + key, err = ctx.GenerateRSAKeyPairWithLabel(randomBytes(), label2, rsaSize) require.NoError(t, err) + defer func(k Signer) { _ = k.Delete() }(key) + attrs := NewAttributeSet() + _ = attrs.Set(CkaLabel, label) keys, err := ctx.FindKeyPairsWithAttributes(attrs) require.Len(t, keys, 1) - attrs, err = NewAttributeSetWithID(id2) - require.NoError(t, err) - + _ = attrs.Set(CkaLabel, label2) keys, err = ctx.FindKeyPairsWithAttributes(attrs) require.Len(t, keys, 2) From 241e158155b5c7c6799a1f8b08f3fab4b92e1312 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Thu, 12 Sep 2019 14:24:33 +0100 Subject: [PATCH 25/36] Fix bug where key test relies on chosen exponent --- keys_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/keys_test.go b/keys_test.go index aaa0270..2eab7e8 100644 --- a/keys_test.go +++ b/keys_test.go @@ -5,6 +5,8 @@ import ( "crypto/rsa" "testing" + "github.com/miekg/pkcs11" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -110,7 +112,7 @@ func TestFindingKeyPairsWithAttributes(t *testing.T) { require.Len(t, keys, 2) attrs = NewAttributeSet() - err = attrs.Set(CkaPublicExponent, []byte{1, 0, 1}) + err = attrs.Set(CkaKeyType, pkcs11.CKK_RSA) require.NoError(t, err) keys, err = ctx.FindKeyPairsWithAttributes(attrs) From 5d11fa3ff256e7569a27b76826535e93a419691f Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Thu, 12 Sep 2019 14:29:50 +0100 Subject: [PATCH 26/36] Be more explicit about key finding error handling --- keys.go | 11 +++++++++-- keys_test.go | 11 ++++++++--- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/keys.go b/keys.go index 70dd822..b0976a0 100644 --- a/keys.go +++ b/keys.go @@ -337,7 +337,11 @@ func (c *Context) FindKeyPairsWithAttributes(attributes AttributeSet) (signer [] return nil }) - return keys, err + if err != nil { + return nil, err + } + + return keys, nil } // FindAllKeyPairs retrieves all existing asymmetric key pairs, or a nil slice if none can be found. @@ -474,7 +478,10 @@ func (c *Context) FindKeysWithAttributes(attributes AttributeSet) ([]*SecretKey, return nil }) - return keys, err + if err != nil { + return nil, err + } + return keys, nil } // FindAllKeyPairs retrieves all existing symmetric keys, or a nil slice if none can be found. diff --git a/keys_test.go b/keys_test.go index 2eab7e8..ca8ab6a 100644 --- a/keys_test.go +++ b/keys_test.go @@ -59,10 +59,12 @@ func TestFindingKeysWithAttributes(t *testing.T) { attrs := NewAttributeSet() _ = attrs.Set(CkaLabel, label) keys, err := ctx.FindKeysWithAttributes(attrs) + require.NoError(t, err) require.Len(t, keys, 1) _ = attrs.Set(CkaLabel, label2) keys, err = ctx.FindKeysWithAttributes(attrs) + require.NoError(t, err) require.Len(t, keys, 2) attrs = NewAttributeSet() @@ -70,6 +72,7 @@ func TestFindingKeysWithAttributes(t *testing.T) { require.NoError(t, err) keys, err = ctx.FindKeysWithAttributes(attrs) + require.NoError(t, err) require.Len(t, keys, 2) attrs = NewAttributeSet() @@ -77,6 +80,7 @@ func TestFindingKeysWithAttributes(t *testing.T) { require.NoError(t, err) keys, err = ctx.FindKeysWithAttributes(attrs) + require.NoError(t, err) require.Len(t, keys, 1) }) } @@ -105,17 +109,18 @@ func TestFindingKeyPairsWithAttributes(t *testing.T) { attrs := NewAttributeSet() _ = attrs.Set(CkaLabel, label) keys, err := ctx.FindKeyPairsWithAttributes(attrs) + require.NoError(t, err) require.Len(t, keys, 1) _ = attrs.Set(CkaLabel, label2) keys, err = ctx.FindKeyPairsWithAttributes(attrs) + require.NoError(t, err) require.Len(t, keys, 2) attrs = NewAttributeSet() - err = attrs.Set(CkaKeyType, pkcs11.CKK_RSA) - require.NoError(t, err) - + _ = attrs.Set(CkaKeyType, pkcs11.CKK_RSA) keys, err = ctx.FindKeyPairsWithAttributes(attrs) + require.NoError(t, err) require.Len(t, keys, 3) }) } From 41df502f00585d2627dd8aaf28dd305f02725e50 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Thu, 12 Sep 2019 14:36:01 +0100 Subject: [PATCH 27/36] Reduce size of random tests to suit CloudHSM --- rand_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rand_test.go b/rand_test.go index f0fea63..45124f0 100644 --- a/rand_test.go +++ b/rand_test.go @@ -39,8 +39,8 @@ func TestRandomReader(t *testing.T) { reader, err := ctx.NewRandomReader() require.NoError(t, err) - var a [32768]byte - for _, size := range []int{1, 16, 32, 256, 347, 4096, 32768} { + var a [8192]byte + for _, size := range []int{1, 16, 32, 256, 347, 4096, 8192} { n, err := reader.Read(a[:size]) require.NoError(t, err) require.Equal(t, size, n) From a148d96a50d255ef19083308b9b4f5c5a80cb4af Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Thu, 12 Sep 2019 14:42:32 +0100 Subject: [PATCH 28/36] Skip unsupported mechanisms in test --- symmetric_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/symmetric_test.go b/symmetric_test.go index bbe9f37..c0687a6 100644 --- a/symmetric_test.go +++ b/symmetric_test.go @@ -46,6 +46,9 @@ func TestHardSymmetric(t *testing.T) { } func testHardSymmetric(t *testing.T, ctx *Context, keytype int, bits int) { + for _, p := range Ciphers[keytype].GenParams { + skipIfMechUnsupported(t, ctx, p.GenMech) + } id := randomBytes() key, err := ctx.GenerateSecretKey(id, bits, Ciphers[keytype]) From 088aec4c061d40ef9389c53c38000dc477e5f3eb Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Thu, 12 Sep 2019 14:48:45 +0100 Subject: [PATCH 29/36] Handle panics in test --- symmetric_test.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/symmetric_test.go b/symmetric_test.go index c0687a6..f7c89d4 100644 --- a/symmetric_test.go +++ b/symmetric_test.go @@ -130,6 +130,14 @@ func testHardSymmetric(t *testing.T, ctx *Context, keytype int, bits int) { } func testSymmetricBlock(t *testing.T, encryptKey cipher.Block, decryptKey cipher.Block) { + // The functions in cipher.Block have no error returns, so they panic if they encounter + // a problem. We catch these panics here, so the test can fail nicely + defer func() { + if cause := recover(); cause != nil { + t.Fatalf("Caught panic: %q", cause) + } + }() + b := encryptKey.BlockSize() input := make([]byte, 3*b) middle := make([]byte, 3*b) @@ -179,6 +187,14 @@ func testSymmetricBlock(t *testing.T, encryptKey cipher.Block, decryptKey cipher } func testSymmetricMode(t *testing.T, encrypt cipher.BlockMode, decrypt cipher.BlockMode) { + // The functions in cipher.Block have no error returns, so they panic if they encounter + // a problem. We catch these panics here, so the test can fail nicely + defer func() { + if cause := recover(); cause != nil { + t.Fatalf("Caught panic: %q", cause) + } + }() + input := make([]byte, 256) middle := make([]byte, 256) output := make([]byte, 256) From 85a427cb4fc38e4e6a66dc8ccacb1cacab9e0e89 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Fri, 13 Sep 2019 10:18:01 +0100 Subject: [PATCH 30/36] Support retrieving IVs in GCM mode from token --- aead.go | 33 ++++++++++++++++++++++----------- crypto11.go | 6 ++++++ 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/aead.go b/aead.go index c0c720c..0f994e3 100644 --- a/aead.go +++ b/aead.go @@ -49,7 +49,9 @@ type genericAead struct { nonceSize int - makeMech func(nonce []byte, additionalData []byte) ([]*pkcs11.Mechanism, error) + // Note - if the GCMParams result is non-nil, the caller must call Free() on the params when + // finished. + makeMech func(nonce []byte, additionalData []byte) ([]*pkcs11.Mechanism, *pkcs11.GCMParams, error) } // NewGCM returns a given cipher wrapped in Galois Counter Mode, with the standard @@ -66,9 +68,9 @@ func (key *SecretKey) NewGCM() (cipher.AEAD, error) { key: key, overhead: 16, nonceSize: 12, - makeMech: func(nonce []byte, additionalData []byte) ([]*pkcs11.Mechanism, error) { + makeMech: func(nonce []byte, additionalData []byte) ([]*pkcs11.Mechanism, *pkcs11.GCMParams, error) { params := pkcs11.NewGCMParams(nonce, additionalData, 16*8 /*bits*/) - return []*pkcs11.Mechanism{pkcs11.NewMechanism(key.Cipher.GCMMech, params)}, nil + return []*pkcs11.Mechanism{pkcs11.NewMechanism(key.Cipher.GCMMech, params)}, params, nil }, } return g, nil @@ -96,12 +98,12 @@ func (key *SecretKey) NewCBC(paddingMode PaddingMode) (cipher.AEAD, error) { key: key, overhead: 0, nonceSize: key.BlockSize(), - makeMech: func(nonce []byte, additionalData []byte) ([]*pkcs11.Mechanism, error) { + makeMech: func(nonce []byte, additionalData []byte) ([]*pkcs11.Mechanism, *pkcs11.GCMParams, error) { if len(additionalData) > 0 { - return nil, errors.New("additional data not supported for CBC mode") + return nil, nil, errors.New("additional data not supported for CBC mode") } - return []*pkcs11.Mechanism{pkcs11.NewMechanism(pkcsMech, nonce)}, nil + return []*pkcs11.Mechanism{pkcs11.NewMechanism(pkcsMech, nonce)}, nil, nil }, } @@ -119,10 +121,12 @@ func (g genericAead) Overhead() int { func (g genericAead) Seal(dst, nonce, plaintext, additionalData []byte) []byte { var result []byte if err := g.key.context.withSession(func(session *pkcs11Session) (err error) { - var mech []*pkcs11.Mechanism - if mech, err = g.makeMech(nonce, additionalData); err != nil { - return + mech, params, err := g.makeMech(nonce, additionalData) + if err != nil { + return err } + defer params.Free() + if err = session.ctx.EncryptInit(session.handle, mech, g.key.handle); err != nil { err = fmt.Errorf("C_EncryptInit: %v", err) return @@ -131,6 +135,11 @@ func (g genericAead) Seal(dst, nonce, plaintext, additionalData []byte) []byte { err = fmt.Errorf("C_Encrypt: %v", err) return } + + if g.key.context.cfg.UseGCMIVFromHSM { + copy(nonce, params.IV()) + } + return }); err != nil { panic(err) @@ -143,10 +152,12 @@ func (g genericAead) Seal(dst, nonce, plaintext, additionalData []byte) []byte { func (g genericAead) Open(dst, nonce, ciphertext, additionalData []byte) ([]byte, error) { var result []byte if err := g.key.context.withSession(func(session *pkcs11Session) (err error) { - var mech []*pkcs11.Mechanism - if mech, err = g.makeMech(nonce, additionalData); err != nil { + mech, params, err := g.makeMech(nonce, additionalData) + if err != nil { return } + defer params.Free() + if err = session.ctx.DecryptInit(session.handle, mech, g.key.handle); err != nil { err = fmt.Errorf("C_DecryptInit: %v", err) return diff --git a/crypto11.go b/crypto11.go index d6f7b00..b37ebed 100644 --- a/crypto11.go +++ b/crypto11.go @@ -242,6 +242,12 @@ type Config struct { // LoginNotSupported should be set to true for tokens that do not support logging in. LoginNotSupported bool + + // UseGCMIVFromHSM should be set to true for tokens such as CloudHSM, which ignore the supplied IV for + // GCM mode and generate their own. In this case, the token will write the IV used into the CK_GCM_PARAMS. + // If UseGCMIVFromHSM is true, we will copy this IV and overwrite the 'nonce' slice passed to Seal and Open. It + // is therefore necessary that the nonce is the correct length (12 bytes for CloudHSM). + UseGCMIVFromHSM bool } // refCount counts the number of contexts using a particular P11 library. It must not be read or modified From 46fc365bbede05204806fe27a8476cc06b62ad8a Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Fri, 13 Sep 2019 10:35:29 +0100 Subject: [PATCH 31/36] Test we support mechanisms before using it --- symmetric_test.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/symmetric_test.go b/symmetric_test.go index f7c89d4..3ff0318 100644 --- a/symmetric_test.go +++ b/symmetric_test.go @@ -62,7 +62,10 @@ func testHardSymmetric(t *testing.T, ctx *Context, keytype int, bits int) { require.NoError(t, err) }) - t.Run("Block", func(t *testing.T) { testSymmetricBlock(t, key, key2) }) + t.Run("Block", func(t *testing.T) { + skipIfMechUnsupported(t, key.context, key.Cipher.ECBMech) + testSymmetricBlock(t, key, key2) + }) iv := make([]byte, key.BlockSize()) for i := range iv { @@ -70,11 +73,12 @@ func testHardSymmetric(t *testing.T, ctx *Context, keytype int, bits int) { } t.Run("CBC", func(t *testing.T) { + skipIfMechUnsupported(t, key2.context, key2.Cipher.CBCMech) testSymmetricMode(t, cipher.NewCBCEncrypter(key2, iv), cipher.NewCBCDecrypter(key2, iv)) }) t.Run("CBCClose", func(t *testing.T) { - + skipIfMechUnsupported(t, key2.context, key2.Cipher.CBCMech) enc, err := key2.NewCBCEncrypterCloser(iv) require.NoError(t, err) @@ -87,6 +91,7 @@ func testHardSymmetric(t *testing.T, ctx *Context, keytype int, bits int) { }) t.Run("CBCNoClose", func(t *testing.T) { + skipIfMechUnsupported(t, key2.context, key2.Cipher.CBCMech) enc, err := key2.NewCBCEncrypter(iv) require.NoError(t, err) From b53c3a02e2a83ff5dfa35deff5aac752cf5a2d2a Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Mon, 16 Sep 2019 10:04:34 +0100 Subject: [PATCH 32/36] Test for ECB mechanism on CBC test --- symmetric_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/symmetric_test.go b/symmetric_test.go index 3ff0318..265e49f 100644 --- a/symmetric_test.go +++ b/symmetric_test.go @@ -73,7 +73,8 @@ func testHardSymmetric(t *testing.T, ctx *Context, keytype int, bits int) { } t.Run("CBC", func(t *testing.T) { - skipIfMechUnsupported(t, key2.context, key2.Cipher.CBCMech) + // By using cipher.NewCBCEncrypter, this test will actually use ECB mode on the key. + skipIfMechUnsupported(t, key2.context, key2.Cipher.ECBMech) testSymmetricMode(t, cipher.NewCBCEncrypter(key2, iv), cipher.NewCBCDecrypter(key2, iv)) }) From ba139f0f9ef1b65f535ad6e5d89aa8d30742fd86 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Mon, 16 Sep 2019 11:20:26 +0100 Subject: [PATCH 33/36] Slow down the test code, so CloudHSM doesn't choke --- thread_test.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/thread_test.go b/thread_test.go index 472c2ee..77f5d4e 100644 --- a/thread_test.go +++ b/thread_test.go @@ -33,6 +33,9 @@ var threadCount = 32 var signaturesPerThread = 256 func TestThreadedRSA(t *testing.T) { + if testing.Short() { + t.Skip() + } ctx, err := ConfigureFromFile("config") require.NoError(t, err) @@ -53,6 +56,9 @@ func TestThreadedRSA(t *testing.T) { for i := 0; i < threadCount; i++ { go signingRoutine(t, key, done) + + // CloudHSM falls over if you create sessions too quickly + time.Sleep(50 * time.Millisecond) } t.Logf("Waiting for %v threads", threadCount) for i := 0; i < threadCount; i++ { @@ -69,7 +75,9 @@ func TestThreadedRSA(t *testing.T) { func signingRoutine(t *testing.T, key crypto.Signer, done chan int) { for i := 0; i < signaturesPerThread; i++ { testRsaSigningPKCS1v15(t, key, crypto.SHA1) + + // CloudHSM falls over if you create sessions too quickly + time.Sleep(50 * time.Millisecond) } done <- 1 - } From 07202e9ec6452c19b3bf6a13e5e62cf309fa3d65 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Mon, 16 Sep 2019 12:26:05 +0100 Subject: [PATCH 34/36] Ensure key is deleted --- dsa_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dsa_test.go b/dsa_test.go index c988504..ab0d73e 100644 --- a/dsa_test.go +++ b/dsa_test.go @@ -118,7 +118,7 @@ func TestHardDSA(t *testing.T) { key, err := ctx.GenerateDSAKeyPairWithLabel(id, label, params) require.NoError(t, err, "Failed for key size %s", parameterSizeToString(pSize)) - defer func() { _ = key.Delete() }() + defer func(k Signer) { _ = k.Delete() }(key) testDsaSigning(t, key, pSize, "hard1") From fb28938f2c6de53b223aa5dc5c32cd1b9fa0e082 Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Mon, 16 Sep 2019 14:57:25 +0100 Subject: [PATCH 35/36] Add additional comments about CloudHSM --- README.md | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/README.md b/README.md index 427adb8..b0a12f1 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,41 @@ list of the following options: in some crypto libraries). Needed for AWS CloudHSM. * `DSA` - disables DSA tests. Needed for AWS CloudHSM (and any other tokens not supporting DSA). +Testing with AWS CloudHSM +------------------------- + +A minimal configuration file for CloudHSM will look like this: + +```json +{ + "Path" : "/opt/cloudhsm/lib/libcloudhsm_pkcs11_standard.so", + "TokenLabel": "cavium", + "Pin" : "username:password", + "UseGCMIVFromHSM" : true, +} +``` + +To run the test suite you must skip unsupported tests: + +``` +CRYPTO11_SKIP=CERTS,OAEP_LABEL,DSA go test -v +``` + +Be sure to take note of the supported mechanisms, key types and other idiosyncrasies described at +https://docs.aws.amazon.com/cloudhsm/latest/userguide/pkcs11-library.html. Here's a collection of things we +noticed when testing with the v2.0.4 PKCS#11 library: + +- 1024-bit RSA keys don't appear to be supported, despite what `C_GetMechanismInfo` tells you. +- The `CKM_RSA_PKCS_OAEP` mechanism doesn't support source data. I.e. when constructing a `CK_RSA_PKCS_OAEP_PARAMS`, +one must set `pSourceData` to `NULL` and `ulSourceDataLen` to zero. +- CloudHSM will generate it's own IV for GCM mode. This is described in their documentation, see footnote 4 on +https://docs.aws.amazon.com/cloudhsm/latest/userguide/pkcs11-mechanisms.html. +- It appears that `CKA_ID` values must be unique, otherwise you get a `CKR_ATTRIBUTE_VALUE_INVALID` error. +- Very rapid session opening can trigger the following error: + ``` + C_OpenSession failed with error CKR_ARGUMENTS_BAD : 0x00000007 + HSM error 8c: HSM Error: Already maximum number of sessions are issued + ``` Testing with SoftHSM2 --------------------- From ad6f4b19a159a497f1aec6649673531fda979dda Mon Sep 17 00:00:00 2001 From: Duncan Jones Date: Mon, 16 Sep 2019 16:16:33 +0100 Subject: [PATCH 36/36] Panic if we cannot copy a token-generated GCM IV --- aead.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/aead.go b/aead.go index 0f994e3..1ca6899 100644 --- a/aead.go +++ b/aead.go @@ -42,6 +42,8 @@ const ( PaddingPKCS ) +var errBadGCMNonceSize = errors.New("nonce slice too small to hold IV") + type genericAead struct { key *SecretKey @@ -119,6 +121,7 @@ func (g genericAead) Overhead() int { } func (g genericAead) Seal(dst, nonce, plaintext, additionalData []byte) []byte { + var result []byte if err := g.key.context.withSession(func(session *pkcs11Session) (err error) { mech, params, err := g.makeMech(nonce, additionalData) @@ -137,6 +140,10 @@ func (g genericAead) Seal(dst, nonce, plaintext, additionalData []byte) []byte { } if g.key.context.cfg.UseGCMIVFromHSM { + if len(nonce) != len(params.IV()) { + return errBadGCMNonceSize + } + copy(nonce, params.IV()) }