FR: Allow clients to set extension policies for x.509 verification

[nbastin]

Given that the default policy seems to generally be to verify against the CA/B ruleset (a reasonable default), it would be nice for non-internet PKI users if we could pass our own extension policy rules into the verifier.

[woodruffw]

For xrefs: #10276 (comment) and #11042 are similar requests.

[deivse]

Hello, just want to bump this, since this is something we also require where I work.
I was hoping the 43.0.0 ClientVerifier would solve my issues, and it does to an extent by removing the subject specification requirement, but unfortunately only some of our certificates have the EKU extension with correct values (those actually used for client/server authentication in the traditional sense.)

I haven't contributed to cryptography before, but I think I may be able to contribute this functionality with some direction from more experienced contributors. From what I read, the main issue most people have with this is the EKU extension being required and validated. A make_generic_verifier as was suggested in some other discussion on this topic would solve this. A potential extension of this functionality would then be to allow users to define custom "extension policies".
If this is something that is desired, I can discuss contributing with my manager.

[alex]

We are interested in expanding these APIs, but it's a very large design space, so before you dive into implementing, I think it'd be helpful for you to sketch out the API that you think makes sense.

…

On Sun, Jul 21, 2024 at 5:25 AM Ivan Desiatov ***@***.***> wrote: Hello, just want to bump this, since this is something we also require at my company. I was hoping the 43.0.0 ClientVerifier would solve my issues, and it does to an extent by removing the subject specification requirement, but unfortunately only some of our certificates have the EKU extension with correct values (those actually used for client/server authentication in the traditional sense.) I haven't contributed to cryptography before, but I think I may be able to contribute this functionality with some direction from more experienced contributors. The minimal feature set resolving the issue most people have with this is the EKU extension being required and validated. A make_generic_verifier as was suggested in some other discussion on this topic would solve this. A potential extension of this functionality would then be to allow users to define custom "extension policies". If this is something that is desired, I can discuss contributing with my manager. — Reply to this email directly, view it on GitHub <#11165 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAAGBAWMQDNRI6BIHFXSDLZNN47XAVCNFSM6AAAAABJ42EE3GVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENBRGU2DCOBUGU> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

-- All that is necessary for evil to succeed is for good people to do nothing.

[nbastin]

At a high level, at least, what I was imagining was a way to instantiate ExtensionValidator objects on the python side, and construct a map of those objects (as on the rust side) to pass into optional ca_extension_policy and ee_extension_policy methods/properties on a PolicyBuilder object before you call .build_server_verfier() or .build_client_verifier() (if you don't supply them those methods provide the default verifier objects you get now). Alternatively it could be that you have to use a .build_custom_verifier() method to make clear that you're in a thar-be-dragons-here world (which might make more sense anyhow, since abusing server/client verification is not right for some of these use cases where you would not otherwise claim that kind of certificate).

I think that matches the pattern being used now, and it makes sense to me, but if you think some other entry point makes more sense that's also perfectly fine. I'm more interested in having the functionality than caring about whether it is my preferred style.

I'm working up a usage example that I'll hopefully post in a bit.

[woodruffw]

Alternatively it could be that you have to use a .build_custom_verifier() method to make clear that you're in a thar-be-dragons-here world (which might make more sense anyhow, since abusing server/client verification is not right for some of these use cases where you would not otherwise claim that kind of certificate).

My preference would definitely be for this, in terms of misuse-resistance: ClientVerifier and ServerVerifier are explicitly mean to have client and server auth in the context of the Web PKI, where the set of extensions is strongly constrained by the CA/B guidelines.

[nbastin]

Something like this:

from cryptography import x509
from cryptography.x509.verification import Criticality, ExtensionValidator, PolicyBuilder


def my_ku_validator (policy_builder, cert, extension):
    # Trying to map the callback in rust
    ...
    # Some way to raise ValidationError consistent with core cryptography
    return

ca_policy = { x509.KeyUsage : ExtensionValidator.present(Criticality.Critical, my_ku_validator),
              x509.BasicConstraints: ExtensionValidator.present(Criticality.Agnostic, None),
              x509.SubjectAlternativeName : ExtensionValidator.maybe_present(Criticality.Agnostic, None),
              ...
            }


pb = verification.PolicyBuilder().max_chain_depth(4).ca_extension_policy(ca_policy)
vf = pb.build_custom_verifier()

vf.verify(my_cert, [i1, i2, r1])

Obviously the namespaces for new objects are fungible. The same pattern would follow for ee_extension_policy (or maybe a less loaded name, although that is the one in the internals).

[prauscher]

Additionally to a general "here-be-dragons" build_custom_verifier, an build_smime_verifier would be great, where EKU for emailProtection etc could be checked. Especially as SMIME Encryption is about to be added, a typical usecase could be:

1. Read certificate specified by a user
2. Validate the certificate against the CA
3. Send an encrypted mail to the subject specified in the Certificate

[deivse]

I like the python API that @nbastin suggests, but I think since we are trying to allow customizing verification to this extent, it shouldn't be limited to extension policies, and we should allow customization of most values in the rust Policy struct. Without thinking about the rust side too much (I'm sure it's possible to come up with a clean implementation), I suggest adding a new CustomPolicyBuilder class that will have the same methods as the current PolicyBuilder but also additional methods to set the EKU, minimum RSA modulus, permitted algorithms, as well as the CA and EE extension policies. It is also worth considering renaming the current PolicyBuilder to something like WebPKIPolicyBuilder, so it is more explicit that it's to be used in that specific context. In the future, other policy builders may be added, eg. an SMIMEPolicyBuilder.

I also think that the CA/B extension policies are a good default even in the case of custom verification, so they can be used unless supplied explicitly (or a different default can be applied, but I wouldn't be able to come up with one):

CustomPolicyBuilder().store().eku(x509.ExtendedKeyUsageOID.CODE_SIGNING) # this still uses the current default extension policies

Extension policies on the python side could either be implemented as ExtensionPolicy objects with fixed supported extensions (currently how it's done on the Rust side), or a python dict that would map arbitrary x509 extension OIDs to ExtensionValidator instances. The latter is preferable to allow working with custom extensions.
Partially modifying a default extension policy also seems useful:

ee_policy = ExtensionPolicy.ee_default()
# Could accept both cryptography extension classes and oids
ee_policy[x509.KeyUsage.oid] = ExtensionValidator.present(Criticality.Critical, my_ku_validator)

Regarding custom validator callbacks, I think they shouldn't accept the policy as a parameter to avoid exposing the actual rust Policy implementation to python. Any required values can be captured by the python function.

# The signature is simplified compared to nbastin's initial suggestion, doesn't expose the policy object.
def my_ku_validator (cert, extension):
    ...
    if not_validated:
        # input on what exception type(s) should be allowed here appreciated.
        raise ValueError("...") 

Finally, here's all of it together:

from cryptography import x509
from cryptography.x509.verification import Criticality, ExtensionValidator, PolicyBuilder, ExtensionPolicy

def key_usage_validator (cert, extension):
    ...

ee_extension_policy = ExtensionPolicy.ee_default()
ee_extension_policy[x509.KeyUsage] = ExtensionValidator.present(Criticality.Critical, key_usage_validator)

pb = CustomPolicyBuilder() \
    .store(store) \
    .max_chain_depth(4) \
    .ee_extension_policy(ee_extension_policy) \
    .eku(x509.ExtendedKeyUsageOID.CODE_SIGNING) 

vf = pb.build_client_verifier()  # Current API with client and server verifier applicable depending on if subject name verification is desired.

I still haven't given much thought to how this could be implemented on the Rust side, so feel free to point out any issues that you think might arise since this will be my first time using pyo3.

@alex, sorry for the delayed response. What do you think about this?

[deivse]

I'm not sure how to handle the built-int extension validators for the default policies on the python side, but I think they can be a different breed of ExtensionValidator with the validator function somehow opaque to the user. (Still a bit hazy on the pyo3 stuff)

[nbastin]

Regarding custom validator callbacks, I think they shouldn't accept the policy as a parameter to avoid exposing the actual rust Policy implementation to python. Any required values can be captured by the python function.

# The signature is simplified compared to nbastin's initial suggestion, doesn't expose the policy object.
def my_ku_validator (cert, extension):
    ...
    if not_validated:
        # input on what exception type(s) should be allowed here appreciated.
        raise ValueError("...") 

Extension validation is not context free - validators must have access to the entire policy object or you can't do meaningful validation.

[woodruffw]

For folks looking into this: keep in mind that the core validation handling in PyCA Cryptography is pure Rust, i.e. isn't aware of Python objects (even via PyO3). That means that any callback-style approach for custom extension handling needs to ensure that the core validator can handle callbacks without needing to obtain the GIL or access Python objects.

[deivse]

Extension validation is not context free - validators must have access to the entire policy object or you can't do meaningful validation.

Yes, I agree. I just thought of a different approach where any required context would be taken from the scope where the python custom validator is defined. I now realise that that's not ideal for multiple reasons (e.g. accessing policy properties that have not been explicitly set by the user)

I am currently playing around with implementation, not as a final version, just to get accustomed to working with cryptography internals. Does anyone have thoughts on the proposed API?

By the way, I would be fine limiting the scope of the proposal to custom extension policies for now, without allowing to change the algorithms permitted by the policy. However, I think a separate CustomPolicyBuilder would be better even in that case, since it would enable complete separation of custom policies and allow to put that api into hazmat, which might be a good idea.

[deivse]

I currently have a working prototype of this feature with the API as outlined in the code snippet below. I would appreciate advice on what the next steps should be for gradually getting this reviewed and merged. Should I open a PR that contains only the .pyi interface stubs so the external python API can be finalised first?

def validator_cb(policy, cert, ext):
    # Any exception from the python validator is treated as failure.
    raise ValueError("...")

ee_ext_policy = ExtensionPolicy.web_pki_defaults_ee()

# I ended up mirroring the rust ExtensionPolicy struct here instead of using a dict
# since only a predefined set of extensions is supported by the underlying 
# cryptography_x509_verification implementation, and a dict 
# would evoke the impression that arbitrary extension types are supported.
ee_ext_policy.basic_constraints = ExtensionValidator.maybe_present(
    Criticality.AGNOSTIC, validator_cb
)

policy_builder = CustomPolicyBuilder().store(store)\
    .max_chain_depth(10)\
    .ee_extension_policy(ee_ext_policy)\
    .ca_extension_policy(ca_ext_policy)\
    # Setting the eku is optional. 
    # If the eku is not set, the default extension policies accept any eku.
    # but otherwise keep the original requirements (criticality, present/non-present)
    .eku(x509.ExtendedKeyUsageOID.CLIENT_AUTH)

verifier = policy_builder.build_client_verifier()
# A custom policy may not require a SANs extesnion so this may be None.
verifier.sans
# This is an always present x509.Name instance, although this may be ommitted,
# since accessing the subject of the leaf certificate does not require as 
# much extra code from the user compared to getting the SANs
verifier.subject 

[alex]

I think for the policy, it'd be better to have a builder. And I think (?) we do want to support arbitrary extensions (@woodruffw I believe you have a use case)?

Doing a .pyi for the API sounds like a reasonable place for the extension bits. Pieces like max_chain_depth and EKU are probably straightforward enough that they could go directly to PR now.

Thanks for looking at this.

[deivse]

I think for the policy, it'd be better to have a builder. And I think (?) we do want to support arbitrary extensions (@woodruffw I believe you have a use case)?

Do you mean that you prefer the API with a separate CustomPolicyBuilder or that it would be better to have a builder style API for the extension policy?

I will include the stubs, and a cut down version of the CustomPolicyBuilder (if you meant the former) in the first PR then.

Regarding arbitrary extension support, it doesn't seem like it would be hard to implement, and might actually improve some of the code in cryptography-x509-verification by reducing boilerplate match and if statements. So I'm fine with implementing it if we want that functionality.

[vEpiphyte]

If there is a branch available for testing, I'd be happy to test it out to compare it against what we can do currently with the pyopenssl X509Store ( see #10393 ).

[deivse]

I have a branch in my fork of cryptography (disclaimer: wip, lackluster tests and no docs).

I haven't worked with pyopenssl before, but it seems that the only way to adjust how extensions are handled is with flags, namely X509StoreFlags.IGNORE_CRITICAL and the policy related-flags. The functionality I have implemented allows to replicate the IGNORE_CRITICAL behaviour (it even goes further by allowing you to define arbitrary extension policies), but the Certificate Policies extension and related are not currently processed at all. Not sure if that's in the books at all.

[woodruffw]

Do you mean that you prefer the API with a separate CustomPolicyBuilder or that it would be better to have a builder style API for the extension policy?

He can correct me if I misunderstood, but I think @alex meant something like:

ca_exts_policy = ExtensionPolicyBuilder().subject_alternative_name(...).basic_constraints(...).build()

This would also enable classmethods to extend/augment reasonable baseline policies, e.g.:

ca_exts_policy = ExtensionPolicyBuilder.ca_webpki_policies().whatever(...).build()

[deivse]

I like this API, as long as we don't allow to specify validators for arbitrary extension types, which @alex mentioned you might have a use case for, and might be useful in general. If we do support arbitrary extension types I think the initial proposition, where the extension policy is just a dict mapping extension OIDs to validators might make more sense. Although, doing something like ExtensionPolicyBuilder.custom_extension(oid, validator) to enable that use case is definitely a way forward as well. I don't have a strong opinion on the API for this.

@woodruffw, I'd also appreciate your input on adding a CustomPolicyBuilder vs just extending the PolicyBuilder API with further features. I initially chose the CustomPolicyBuilder route to follow cryptography's philosophy of "making it hard to do insecure things", but it can be argued that having good defaults on the PolicyBuilder and requiring explicit calls to relax the requirements is enough (and avoids some code duplication).

[woodruffw]

Although, doing something like ExtensionPolicyBuilder.custom_extension(oid, validator) to enable that use case is definitely a way forward as well. I don't have a strong opinion on the API for this

Yeah, I think this would be my preference -- I think custom extension support would ideally be other_extension(oid, presence, criticality, callable_validator) to mirror the expectation that presence and criticality are declared rather than checked implicitly via the validation callable.

(We can model that as an OID -> validator dictionary under the hood if doing so makes sense! But IMO the builder API is maximally consistent with PyCA's other X.509 APIs.)

I'd also appreciate your input on adding a CustomPolicyBuilder vs just extending the PolicyBuilder API with further features.

I think I prefer CustomPolicyBuilder personally -- IMO it makes it easier to visually scan (and write lints) for non-default things.

(My actual use case is validating custom extensions that are only present in the Sigstore PKI.)

[deivse]

@woodruffw I was thinking about the details of the ExtensionPolicyBuilder API and there's a slight problem with having a presence parameter for the builder methods. Since a hypothetical ExtensionPresence.NOT_PRESENT doesn't require any extra arguments unlike the two other cases where criticality must be specified (and optionally a validator cb).

While it's possible, I don't like the idea of having a "dynamic function signature" based on the first parameter. By this I mean having both criticality and validator be optional in the function signature, and throwing if presence is not NotPresent and criticality is not provided.

Instead I suggest the following interface:

ext_policy_builder.not_present(oid)
ext_policy_builder.maybe_present(oid, criticality, validator)
ext_policy_builder.must_be_present(oid, criticality, validator)

I think this has many positives as it reduces the number of boilerplate rust methods (no new method for each common extension), and provides a rigid API that would also make the resulting code quite readable in my opinion.

PS: builder.present(oid, criticality, validator) instead of builder.must_be_present(oid, criticality, validator) is also an option, but I prefer the longer but more explicit version.