Skip to content

Unable to create pkcs12 truststore using pkcs12.serialize_key_and_certificates #7065

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

Closed
rbuffat opened this issue Apr 13, 2022 · 16 comments · Fixed by #12393
Closed

Unable to create pkcs12 truststore using pkcs12.serialize_key_and_certificates #7065

rbuffat opened this issue Apr 13, 2022 · 16 comments · Fixed by #12393

Comments

@rbuffat
Copy link

rbuffat commented Apr 13, 2022

I try to implement the following keytool command with cryptography to create a truststore using a self created CA certificate:

keytool -keystore truststore.p12 -alias CARoot -import -file /path/to/ca_cert.pem -storepass the_password -noprompt -storetype PKCS12

However, when writing the CA certificate using pkcs12.serialize_key_and_certificates the resulting truststore is different and seems to be unusable.

Code to reproduce:

import datetime
import os
import typing
import cryptography

import cryptography.x509 as x509
from cryptography import x509
from cryptography.hazmat.primitives import hashes, serialization
from cryptography.hazmat.primitives.asymmetric import rsa
from cryptography.hazmat.primitives.serialization import pkcs12
from cryptography.x509.oid import NameOID

COUNTRY_NAME = "CH"
STATE_OR_PROVINCE_NAME = "Zurich"
LOCALITY_NAME = "Zurich"
ORGANIZATION_NAME = "MaybeBug"

CA_CERTIFICATE_PATH = "cacert.pem"


def create_ca() -> typing.Tuple[rsa.RSAPrivateKey, x509.Certificate]:

    private_key: rsa.RSAPrivateKey = rsa.generate_private_key(
        public_exponent=65537,
        key_size=4096,
    )

    ca_cert = (
        x509.CertificateBuilder()
        .subject_name(
            x509.Name(
                [
                    # Provide various details about who we are.
                    x509.NameAttribute(NameOID.COUNTRY_NAME, COUNTRY_NAME),
                    x509.NameAttribute(
                        NameOID.STATE_OR_PROVINCE_NAME, STATE_OR_PROVINCE_NAME
                    ),
                    x509.NameAttribute(NameOID.LOCALITY_NAME, LOCALITY_NAME),
                    x509.NameAttribute(NameOID.ORGANIZATION_NAME, ORGANIZATION_NAME),
                    x509.NameAttribute(NameOID.COMMON_NAME, "CA"),
                ]
            )
        )
        .issuer_name(
            x509.Name(
                [
                    # Provide various details about who we are.
                    x509.NameAttribute(NameOID.COUNTRY_NAME, COUNTRY_NAME),
                    x509.NameAttribute(
                        NameOID.STATE_OR_PROVINCE_NAME, STATE_OR_PROVINCE_NAME
                    ),
                    x509.NameAttribute(NameOID.LOCALITY_NAME, LOCALITY_NAME),
                    x509.NameAttribute(NameOID.ORGANIZATION_NAME, ORGANIZATION_NAME),
                    x509.NameAttribute(NameOID.COMMON_NAME, "CA"),
                ]
            )
        )
        .public_key(private_key.public_key())
        .serial_number(1)
        .not_valid_before(datetime.datetime.utcnow())
        .not_valid_after(
            # Our certificate will be valid for 10 days
            datetime.datetime.utcnow()
            + datetime.timedelta(days=3650)
        )
        .add_extension(x509.BasicConstraints(ca=True, path_length=None), critical=True)
        .add_extension(
            x509.KeyUsage(
                digital_signature=False,
                content_commitment=False,
                key_encipherment=False,
                data_encipherment=False,
                key_agreement=False,
                key_cert_sign=True,
                crl_sign=True,
                encipher_only=False,
                decipher_only=False,
            ),
            critical=False,
        )
        .add_extension(
            x509.AuthorityKeyIdentifier.from_issuer_public_key(
                private_key.public_key()
            ),
            critical=False,
        )
        .sign(private_key, hashes.SHA256())
    )

    with open(CA_CERTIFICATE_PATH, "wb") as f:
        f.write(ca_cert.public_bytes(serialization.Encoding.PEM))

    return private_key, ca_cert


def create_truststore_keytool(truststore_path: str, truststore_password: str):
    command = f'keytool -keystore {truststore_path} -alias CARoot -import -file {CA_CERTIFICATE_PATH} -storepass "{truststore_password}" -noprompt -storetype PKCS12'
    os.system(command)


def create_truststore_cryptography(
    ca_certificate: x509.Certificate, truststore_path: str, truststore_password: str
):

    pcks12_ca_certificate = pkcs12.PKCS12Certificate(ca_certificate, "CAroot".encode())

    p12 = pkcs12.serialize_key_and_certificates(
        name="CARoot".encode(),
        key=None,
        cert=None,
        cas=[pcks12_ca_certificate],
        encryption_algorithm=serialization.BestAvailableEncryption(
            truststore_password.encode()
        ),
    )
    with open(truststore_path, "wb") as f:
        f.write(p12)


def verify_truststore(truststore_path, truststore_password: str):
    command = (
        f"keytool -list -keystore {truststore_path} -storepass '{truststore_password}'"
    )
    os.system(command)

    with open(truststore_path, "rb") as f:
        data = f.read()
        (
            truststore_private_key,
            truststore_certificate,
            truststore_additional_certificates,
        ) = pkcs12.load_key_and_certificates(
            data, password=truststore_password.encode()
        )

        print("truststore_private_key", truststore_private_key)
        print("truststore_certificate", truststore_certificate)
        print("truststore_additional_certificates", truststore_additional_certificates)


password = "a_very_secure_password"
ca_key, ca_certificate = create_ca()

print("Cryptography version:", cryptography.__version__)
print("Create truststore using keytool:")
create_truststore_keytool("keytool.truststore.p12", password)
verify_truststore("keytool.truststore.p12", password)
print()
print("Create truststore using cryptography:")
create_truststore_cryptography(ca_certificate, "cryptography.truststore.p12", password)
verify_truststore("cryptography.truststore.p12", password)

Output:

Cryptography version: 37.0.0.dev1
Create truststore using keytool:
Certificate was added to keystore
Keystore type: PKCS12
Keystore provider: SUN

Your keystore contains 1 entry

caroot, Apr 13, 2022, trustedCertEntry, 
Certificate fingerprint (SHA-256): BF:E5:79:AB:98:AF:DA:BF:5B:A1:75:A7:27:2E:BE:BC:9E:16:8E:A7:DD:D3:10:3F:D2:41:07:AE:27:5A:2B:2A
truststore_private_key None
truststore_certificate None
truststore_additional_certificates [<Certificate(subject=<Name(C=CH,ST=Zurich,L=Zurich,O=MaybeBug,CN=CA)>, ...)>]

Create truststore using cryptography:
Keystore type: PKCS12
Keystore provider: SUN

Your keystore contains 0 entries

truststore_private_key None
truststore_certificate None
truststore_additional_certificates [<Certificate(subject=<Name(C=CH,ST=Zurich,L=Zurich,O=MaybeBug,CN=CA)>, ...)>]


System: Arch Linux, cryptography installed using pip, Version cryptography==36.0.2

@reaperhulk
Copy link
Member

I believe #6910 should provide what you need for this to work. It will be in the next release, but if you have a chance to test it before release that would be helpful, thanks!

@rbuffat
Copy link
Author

rbuffat commented Apr 13, 2022

@reaperhulk Unfortunately not. I updated the script and output in the original post that wraps the ca certificate in a pkcs12.PKCS12Certificate before exporting.

When listing the certificates of the truststore with keytool -list -keystore cryptography.truststore.p12 -storepass 'a_very_secure_password'
for the truststore it still lists:

Your keystore contains 0 entries

For the keytool.truststore.p12 it lists:

Your keystore contains 1 entry

caroot, Apr 13, 2022, trustedCertEntry, 
Certificate fingerprint (SHA-256): BF:E5:79:AB:98:AF:DA:BF:5B:A1:75:A7:27:2E:BE:BC:9E:16:8E:A7:DD:D3:10:3F:D2:41:07:AE:27:5A:2B:2A

when exporting it with cryptography. Maybe there is something wrong with how I serialize the certificate.

@reaperhulk
Copy link
Member

I can reproduce this, but I don't understand what structure keytool is actually creating here so it's unclear how to make this work as expected.

@alex alex added this to the Thirty Seventh Release milestone Apr 15, 2022
@reaperhulk reaperhulk removed this from the Thirty Seventh Release milestone Apr 18, 2022
@rbuffat
Copy link
Author

rbuffat commented Apr 21, 2022

@reaperhulk Thanks for looking into it.

When querying the generated trust stores using openssl the following output is shown:

openssl pkcs12 -info -in keytool.truststore.p12:
MAC: sha1, Iteration 100000
MAC length: 20, salt length: 20
PKCS7 Encrypted data: pbeWithSHA1And40BitRC2-CBC, Iteration 50000
Certificate bag
Bag Attributes
    friendlyName: CARoot
    2.16.840.1.113894.746875.1.1: <Unsupported tag 6>
openssl pkcs12 -info -in cryptography.truststore.p12:
MAC: sha1, Iteration 1
MAC length: 20, salt length: 8
PKCS7 Encrypted data: pbeWithSHA1And3-KeyTripleDES-CBC, Iteration 20000
Certificate bag
Bag Attributes
    friendlyName: CAroot

Looking for 2.16.840.1.113894.746875.1.1 I found the following stackoverflow issue: https://stackoverflow.com/questions/52524948/created-java-truststore-p12-using-only-openssl

Per: previous discussion, the PKCS12 truststore bag has to have the required attribute '2.16.840.1.113894.746875.1.1' in order for the JRE to use it as an accepted truststore.

@reaperhulk
Copy link
Member

If OpenSSL doesn't support the addition of that OID then we won't be able to add support for this right now unfortunately. At some point we may choose to implement PKCS12 structures in our own ASN.1 code (much as we did for x509), but that's quite a bit of non-trivial work.

@rbuffat
Copy link
Author

rbuffat commented Apr 21, 2022

The OpenSSL command utility seems not to export this functionality. I'm not sure yet for the openssl API, but it is the first time I'm looking at it.

PKCS12_create contains a call to PKCS12_add_localkeyid. But I'm not yet sure if this would be useful to create the required attribute.

https://github.com/openssl/openssl/blob/1c0eede9827b0962f1d752fa4ab5d436fa039da4/doc/man3/PKCS12_add_localkeyid.pod
https://github.com/openssl/openssl/blob/1c0eede9827b0962f1d752fa4ab5d436fa039da4/crypto/pkcs12/p12_crt.c#L69

@reaperhulk
Copy link
Member

The OpenSSL API doesn't appear to allow adding it either.

@rbuffat
Copy link
Author

rbuffat commented Apr 26, 2022

@reaperhulk Much appreciated that you checked! Should we close this issue or leave it open until eventually OpenSSL API supports this?

@ChristianCiach
Copy link

Are you sure that the OpenSSL API does not support this? According to openssl/openssl#6684 (comment) it looks like it does.

@reaperhulk
Copy link
Member

Interesting, so we may be able to do this with OpenSSL 3. I'll reopen this and investigate a bit when I get a chance.

@crbednarz
Copy link
Contributor

Hopefully I'm understanding the issue here correctly, but it looks like OpenSSL now directly supports creating a Java compatible truststore. In the CLI it's a new jdktrust flag (docs). As an example:

openssl pkcs12 -export -in ca.crt -nokeys -out ca.p12 -password pass:password -caname ca.crt -jdktrust anyExtendedKeyUsage

Looking behind the scenes this appears to be implemented with a new PKCS12_create_ex2 (docs), which adds a callback during the creation of PKCS12 structures to allow for modifying the safebag.
Here's the relevant snippet:
https://github.com/openssl/openssl/blob/aa52ec9b0ae5c32bb4759b71117737002cbd1263/apps/pkcs12.c#L918-L941

@reaperhulk
Copy link
Member

Yes, that’s how they implemented it. However, we are increasingly taking over parsing rather than using OpenSSL (for both safety and performance reasons) so our solution will be implemented within this project.

@alex
Copy link
Member

alex commented Jan 30, 2025 via email

@crbednarz
Copy link
Contributor

Understood. I'd love to help contribute to this, if possible- though I'm no cryptography expert.

Reading a bit further onto the subject, it seems that to be considered a valid truststore the pkcs12 structure must contain exclusively certificates with the attribute OID of 2.16.840.1.113894.746875.1.1.

Not sure if there's a specific implementation in mind, but I've been toying around with some ideas. Seems to me that given the constraints and unique attributes of truststores, you'd want them to have their own serialization function on the Python side. i.e.

# Existing serialize function
def serialize_key_and_certificates(
    name: bytes | None,
    key: PKCS12PrivateKeyTypes | None,
    cert: x509.Certificate | None,
    cas: Iterable[_PKCS12CATypes] | None,
    encryption_algorithm: serialization.KeySerializationEncryption,
) -> bytes:
  # ...

def serialize_java_truststore(
    certs: Iterable[PKCS12Certificate],
    encryption_algorithm: serialization.KeySerializationEncryption,
) -> bytes:
  # ...

This way the user cannot generate an invalid truststore by including keys or having to manually track/adjust safebag attributes.

On the Rust side, I would think serialize_key_and_certificates could be refactored to add a new serialize_key_and_cert_bags method which handles the bulk of the work. That way both serialize_key_and_certificates and a new serialize_java_truststore would simply be responsible for converting inputs to the appropriate safebags and invoking serialize_key_and_cert_bags.

Assuming that approach sounds reasonable, I can put together an implementation (as time permits).

@alex
Copy link
Member

alex commented Feb 1, 2025

@crbednarz That sounds basically reasonable to me! Thanks for taking a look at this! We'd be thrilled to review a PR with that.

@crbednarz
Copy link
Contributor

Alright, PR submitted. #12393

@alex alex linked a pull request Feb 4, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

5 participants