JackOfMostTrades / aws-kms-pkcs11

PKCS#11 Provider Using AWS KMS
MIT License
39 stars 17 forks source link

Misc fixes/improvements and ability to retrieve certs from acm-pca #23

Closed ozbenh closed 1 year ago

ozbenh commented 1 year ago

This improves the config file parsing a bit, fixes a crashing bug when hitting the "no config file" fallback, and adds the ability to retrieve certificates directly from acm-pca by ARN

JackOfMostTrades commented 1 year ago

Can you update the slots section of the README with the new ca_arn and certificate_arn options?

JackOfMostTrades commented 1 year ago

We'll probably also need an update on this line to have CI build the acm-pca SDK library.

ozbenh commented 1 year ago

Ok thanks, I'll update later today or tomorrow

ozbenh commented 1 year ago

Updated. Additionally, you can now ommit ca_arn as it's usually a substring of the certificate_arn so I added code to just extract it if unspecified

JackOfMostTrades commented 1 year ago

Thanks, changes look good to me! Merging!

JackOfMostTrades commented 1 year ago

@ozbenh Hmm, openssl doesn't seem to care for the *X509 parameters being marked as const. Did you hit this issue at all? Wondering if I should bump the version of openssl or if we need to drop those const qualifiers.

#!/bin/bash -eo pipefail
AWS_SDK_PATH=$HOME/aws-sdk-cpp make
Using C++ SDK static libraries
Using C SDK static libraries
g++ -shared -fPIC -Wall -I/home/circleci/aws-sdk-cpp/include -I/usr/include/p11-kit-1/p11-kit -I/usr/include/json-c -fno-exceptions -std=c++17 attributes.cpp aws_kms_pkcs11.cpp certificates.cpp unsupported.cpp debug.cpp aws_kms_slot.cpp \
    -o aws_kms_pkcs11.so  /home/circleci/aws-sdk-cpp/lib/libaws-cpp-sdk-kms.a /home/circleci/aws-sdk-cpp/lib/libaws-cpp-sdk-acm-pca.a /home/circleci/aws-sdk-cpp/lib/libaws-cpp-sdk-core.a /home/circleci/aws-sdk-cpp/lib/libaws-crt-cpp.a -Wl,--start-group /home/circleci/aws-sdk-cpp/lib/libaws-checksums.a /home/circleci/aws-sdk-cpp/lib/libaws-c-common.a /home/circleci/aws-sdk-cpp/lib/libaws-c-event-stream.a /home/circleci/aws-sdk-cpp/lib/libaws-c-auth.a /home/circleci/aws-sdk-cpp/lib/libaws-c-http.a /home/circleci/aws-sdk-cpp/lib/libaws-c-io.a /home/circleci/aws-sdk-cpp/lib/libaws-c-mqtt.a /home/circleci/aws-sdk-cpp/lib/libaws-c-cal.a /home/circleci/aws-sdk-cpp/lib/libaws-c-compression.a /home/circleci/aws-sdk-cpp/lib/libaws-c-s3.a /home/circleci/aws-sdk-cpp/lib/libaws-c-sdkutils.a /home/circleci/aws-sdk-cpp/lib/libs2n.a -Wl,--end-group  -lcrypto -ljson-c -lcurl
attributes.cpp: In function ‘CK_RV do_get_raw_cert(const X509*, CK_VOID_PTR, CK_ULONG_PTR)’:
attributes.cpp:221:29: error: invalid conversion from ‘const X509*’ {aka ‘const x509_st*’} to ‘X509*’ {aka ‘x509_st*’} [-fpermissive]
  221 |     CK_ULONG len = i2d_X509(cert, &buffer);
      |                             ^~~~
      |                             |
      |                             const X509* {aka const x509_st*}
In file included from /usr/include/openssl/ec.h:17,
                 from attributes.cpp:1:
/usr/include/openssl/x509.h:551:1: note:   initializing argument 1 of ‘int i2d_X509(X509*, unsigned char**)’
  551 | DECLARE_ASN1_FUNCTIONS(X509)
      | ^
attributes.cpp: In function ‘CK_RV do_get_raw_name(const X509_NAME*, CK_VOID_PTR, CK_ULONG_PTR)’:
attributes.cpp:231:34: error: invalid conversion from ‘const X509_NAME*’ {aka ‘const X509_name_st*’} to ‘X509_NAME*’ {aka ‘X509_name_st*’} [-fpermissive]
  231 |     CK_ULONG len = i2d_X509_NAME(name, &buffer);
      |                                  ^~~~
      |                                  |
      |                                  const X509_NAME* {aka const X509_name_st*}
In file included from /usr/include/openssl/ec.h:17,
                 from attributes.cpp:1:
/usr/include/openssl/x509.h:545:1: note:   initializing argument 1 of ‘int i2d_X509_NAME(X509_NAME*, unsigned char**)’
  545 | DECLARE_ASN1_FUNCTIONS(X509_NAME)
      | ^
attributes.cpp: In function ‘CK_RV do_get_raw_integer(const ASN1_INTEGER*, CK_VOID_PTR, CK_ULONG_PTR)’:
attributes.cpp:241:37: error: invalid conversion from ‘const ASN1_INTEGER*’ {aka ‘const asn1_string_st*’} to ‘ASN1_INTEGER*’ {aka ‘asn1_string_st*’} [-fpermissive]
  241 |     CK_ULONG len = i2d_ASN1_INTEGER(serial, &buffer);
      |                                     ^~~~~~
      |                                     |
      |                                     const ASN1_INTEGER* {aka const asn1_string_st*}
In file included from /usr/include/openssl/ec.h:17,
                 from attributes.cpp:1:
/usr/include/openssl/asn1.h:570:1: note:   initializing argument 1 of ‘int i2d_ASN1_INTEGER(ASN1_INTEGER*, unsigned char**)’
  570 | DECLARE_ASN1_FUNCTIONS(ASN1_INTEGER)
      | ^
make: *** [Makefile:172: aws_kms_pkcs11.so] Error 1

Exited with code exit status 2
CircleCI received exit code 2
ozbenh commented 1 year ago

On Wed, 2023-02-01 at 18:07 -0800, Ian Haken wrote:

@ozbenh Hmm, openssl doesn't seem to care for the *X509 parameters being marked as const. Did you hit this issue at all? Wondering if I should bump the version of openssl or if we need to drop those const qualifiers. Ah, I have worked around this for OpenSSL 1.0.2 in my amzn2 branch, we can move that workaround to master and 1.1.1, I'll look into this when I'm back home tonight or tomorrow. It's fine with OpenSSL 3.0

Cheers, Ben.

ozbenh commented 1 year ago

Fixes in https://github.com/JackOfMostTrades/aws-kms-pkcs11/pull/24

JackOfMostTrades commented 1 year ago

Tagged a release v0.0.10 with these changes.

ozbenh commented 1 year ago

Thanks. Note that it gets quite slow with a few keys in there. In my case, it looks like the plugin is opened/closed often and getting to load all the certs from acm-pca is taking a while. I'll look at adding lazy loading of them. I'm also thinking of caching the SDK client objects when the region isn't changed from one call to the next.