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.

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

PKCS#11 Integration #44

Merged
merged 41 commits into from
Sep 20, 2023
Merged

PKCS#11 Integration #44

merged 41 commits into from
Sep 20, 2023

Conversation

13ajay
Copy link
Contributor

@13ajay 13ajay commented Aug 1, 2023

Description of changes:

Creating another PR that adds the PKCS#11 integration changes back in since the previous one was automatically closed by GitHub.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Setting the CKA_ALWAYS_AUTHENTICATE attribute on an object still hasn't been tested yet

Refactor PIN prompting specifically for PKCS#11
 * Sometimes, there are successive operations that a user may want to
   perform with a PKCS11Signer, each of which require an open session.
   In this case, it would be better to leave the session open until each
   of those operations have completed and only then clean things up.

 * By default, save session and handle information. Introduce
   CloseSession() method to aws_signing_helper.Signer interface if
   partial-cleanup has to be done after successive operations.

 * Don't save token information within CertObjInfo. The information
   isn't specific to individual certificate objects in PKCS#11.
 * Check that signing operations can be done after signer.CloseSession()

 * Clean up some unnecessary code/comments in signer_test.go and the
   Makefile
 * Add some more debug logging to pkcs11_signer.go.

 * Make debug logging output to stdout.
 * PKCS#11 integration and certificate store integrations don't allow
   the user to specify intermediates.

 * For now, if the user uses PKCS#12 files, intermediates also can't be
   specified, but that should be changed.
 * Sometimes, it's possible for the certificate and private key to be
   found in different slots. We want to search for the rest of the
   certificate chain in the same slot that the certificate was found,
   but in this case, we only store the session that the key was found
   in. To get around this, reopen (and close once done) the session that
   the certificate was found in, in order to do searching.
 * Also format go files using 'go fmt'
 * Fetch and save resources (that may not be strictly required for the
   signer) like the certificate chain when creating the PKCS11Signer.

 * Update certificate and private key URIs based on where they are first
   found when creating the PKCS11Signer.

 * Fix certificate chain lookup - requires testing.
 * If a certificate is provided (even if it's just the path to the file
   that was provided as opposed to a URI), make sure that the private
   key matches it when creating a PKCS11Signer.

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

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

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

 * Add tests to make sure that certificate and private key do match when
   creating the PKCS11Signer.
 * Use different device files for console input and output on Windows
   (CONIN$ and CONOUT$, respectively).

 * Note that these changes haven't been completely tested yet.
 * Terminate for-loop when PEM blocks can no longer be parsed
   (pem.Decode returns nil).

 * Add test case for parsing PEM file with comments.
 * There has to be an end-entity certificate in the PKCS#12 container.
   Otherwise, a Signer won't be created. The first certificate found in
   the container that doesn't issue other certificates in the container
   will be used.

 * Every certificate other than the end-entity certificate gets included
   in the CreateSession request as intermediates (even if they don't
   chain up to the end-entity certificate that was found).

 * Add negative testing using PKCS#12 containers with self-signed
   certificates.
attribute is set.

 * Add force-prompt flag for credentialing commands and sign-string.

 * Add test for when there are multiple matching private key objects
   when creating PKCS11Signer.
 * Make the "try user PIN as the context-specific PIN" behavior opt-in,
   reducing the risk of lockout.

 * Modify unit tests to set the reuse PIN option, so that prompting
   doesn't occur.
 * Refactor PKCS#11 section of README and add "Other Notes" as a
   sub-section.

 * Add documentation on the "reuse-pin" flag.
 * Restructure getMatchingCerts() so that more detailed error messages
   are returned when matching certificate(s) can't be found.

 * Remove some unnecessary code relating to saving context-specific
   PINs.

 * Fix typo in comment.
@rlalcorn rlalcorn requested review from dwmw2 and removed request for dwmw2 September 20, 2023 00:55
will register itself with p11-kit and will be automatically visible
through the `p11-kit-proxy.{dylib, dll, so}` provider which is used by default.

If you have a poorly packaged provider module from a vendor, then
Copy link
Contributor

Choose a reason for hiding this comment

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

Define "poorly" in this context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll do this in a follow-up PR. Merging this for now.

@13ajay 13ajay merged commit 3e1345b into main Sep 20, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants