`permitted_public_key_algorithms` and `permitted_signature_algorithms` are not exposed when verifying a x509 certificate

[twilleke]

I am trying to verify a certificate that has a ED25519 key and the certificate authority also has a ED25519 key.
But even when the certificate is valid I get the error:

cryptography.hazmat.bindings._rust.x509.VerificationError: validation failed: candidates exhausted: Forbidden public key algorithm: AlgorithmIdentifier { oid: DefinedByMarker(PhantomData<asn1::object_identifier::ObjectIdentifier>), params: Ed25519 }

This seems to occur because when the Policy is create, the WebPKI defaults are selected and the WebPKI does not allow ED25519 keys.

cryptography/src/rust/cryptography-x509-verification/src/policy/mod.rs

Lines 257 to 258 in 1a952fc

	permitted_public_key_algorithms: Arc::clone(&*WEBPKI_PERMITTED_SPKI_ALGORITHMS),
	permitted_signature_algorithms: Arc::clone(&*WEBPKI_PERMITTED_SIGNATURE_ALGORITHMS),

I have found no way to set these parameters on the python side. Is this an oversight or is it meant that way ?

[alex]

That's correct that there's currently no API for setting allowed signature or public key algorithms.

[alex]

I think in principle we'd be ok with an API for this, if someone was interested in figuring out how it should work and proposing something.

[alex]

For the API here, we'll need 2 new methods on PolicyBuilder for setting the key and signature algorithms.

Potential values they can take:

• A new enum with different key/signature types
• Raw OIDs, except this will have to be granular (AlgorithmIdentifier is used for EC, PSS, etc. to indicate subfields)
• Some sort of re-use of existing types: PSS(), ec.SECP256R1(), etc.
• something else

Opinions?

[ralphje]

I've run into this issue while trying to migrate away from certvalidator. In my case, I'm trying to verify Authenticode signatures, so I need to build different policies than WebPKI. RsaWithSha1 is perfectly acceptable in (especially older) binaries.

Perhaps it's best to ensure that existing types work, but currently, the X.509 class seems to rely on using the OIDs and parameters (see e.g. signature_algorithm_oid and signature_algorithm_parameters). That would need to be adjusted/fixed as well.

I've also noticed that minimum_rsa_modulus and extended_key_usage are also not exposed by the PolicyBuilder.

[alex]

I don't see why signature_algorithm_oid or signature_algorithm_parameters would need to chane here. RSA key sizes and EKUs are distinct issues from this.

[ralphje]

I'm sorry, I wasn't making clear what I meant, probably caused by typing stuff in a hurry.

I think that whatever is decided to support here, e.g. a new enum, for consistency, we should make sure that there exists an attribute on the Certificate class that returns the same value. In that respect, using OIDs with parameters makes some sense, since we already return those OIDs from the Certificate class.

However, like you said, OIDs are not granular enough, and in my opinion, perhaps a bit verbose. In that latter sense, it does make sense to re-use the existing classes to allow restricting public key algorithms and signature algorithms. But I'm not particularly well-versed in this package to know how you'd approach this.

A naive approach could be to expose the AlgorithmIdentifier subclasses and logic to Python, re-using the existing classes from hazmat where possible. This would be a bit similar to how ExtensionType wraps a single OID for an extension.

Another option would be to allow a callable akin to MaybeExtensionValidatorCallback to handle validation. This would allow for a bit more flexibility, although that does not solve the problem of how to define these types.

Regarding minimum_rsa_modulus and extended_key_usage, I do agree that those are a different topic than permitted_public_key_algorithms and permitted_signature_algorithms. However, given that these parameters are lacking in the Python interface as well, I merely added this as a notion of "there's no feature-parity between the Rust parameters and the exposed parameters". This may or may not have been a consious decision, but given that it seems related in that regard, I just wanted to put this out here.

Given my lack of knowledge on the inner workings of this module, please feel free to disregard my comments entirely :).