mirleft / ocaml-x509

X509 (RFC5280) handling in OCaml
BSD 2-Clause "Simplified" License
52 stars 34 forks source link

Option allow_ca_cert for buggy server certificates where CA=true #160

Closed mbacarella closed 1 year ago

mbacarella commented 1 year ago

See the PR at https://github.com/mirleft/ocaml-tls/pull/466 for more information.

hannesm commented 1 year ago

Dear @mbacarella, thanks for your PR. But to be honest, I'd really prefer to figure out which functions need to be exposed so you can in your application / library write such an authenticator yourself. Adding more and more arguments to validate_chain_of_trust makes everything complex and error-prone -- which as well means this library will be harder to maintain and reason about.

I know back in the days, there was Authenticator.null(and I'm glad we removed it in https://github.com/mirleft/ocaml-x509/pull/131/commits/0bf0574fee409fc6b0eaf79e6d3392ab5f2407ff).

Reading a bit more on other TLS implementations, they usually provide a verify_callback. I guess from ocaml-x509 we could provide the building blocks for such a verification (of course, providing chain_of_trust and fingerprint authenticator as currently done).

Would that work for you?

mbacarella commented 1 year ago

I can appreciate your concern.

So, I took a crack at writing an authenticator by copy/pasting code from ocaml-tls and ocaml-x509 and it just started to feel like the wrong approach.

This is as far as I got: https://gist.github.com/mbacarella/27df6edbc67a8244f695aabd33fc2d6a

Basically, I was a little anxious about having that much code duplicated with no easy way of being looped in to future improvements to upstream.

But also it doesn't even work since not enough is exposed. The Certificate needs to expose version and signature_data and a few other things. The entire Algorithm module needs exposing, as well. Stuff like that. To cut down on duplication we could expose a lot more utility functions from Validation but this starts exposing some pretty low-level bits (like functions with "hack" in the name).

Shifting gears, what do you have in mind with a verify_callback approach?

hannesm commented 1 year ago

Shifting gears, what do you have in mind with a verify_callback approach?

To provide some useful functions/building blocks to do the certificate authentication yourself. This requires some design for having a composable set of things of what is needed / useful in the real world -- otherwise it'll be rather clunky.

hannesm commented 1 year ago

Looking back to your original issue -- AFAICT the situation is that the default configuration of that service generates a private key and self-signed CA certificate. The latter is use for the TLS endpoint by default.

Now, for validation we use the very same certificate as trust anchor and verify the server-provided certificate -- which is the identical one. Since the very special case is that it is a self-signed certificate, I'm thinking that we could allow it, independent of the BasicConstraints CA=true extension.

Would you mind to try out whether #161 suits your needs?

mbacarella commented 1 year ago

Wow, yes, #161 solves it nicely! Thank you.

Sorry for not being competent enough in TLS certificate stuff to understand what the underlying problem was all along.

hannesm commented 1 year ago

@mbacarella please don't be sorry. I'm glad that solves the issue for you, and thanks for working on patches to make it work for you :)

hannesm commented 1 year ago

superseeded by #161