Skip to content
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

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

Closed
wants to merge 1 commit into from

Conversation

mbacarella
Copy link

@mbacarella mbacarella commented Feb 2, 2023

See the PR at mirleft/ocaml-tls#466 for more information.

@hannesm
Copy link
Member

hannesm commented Feb 3, 2023

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 0bf0574).

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
Copy link
Author

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
Copy link
Member

hannesm commented Feb 3, 2023

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
Copy link
Member

hannesm commented Feb 4, 2023

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
Copy link
Author

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
Copy link
Member

hannesm commented Feb 4, 2023

@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
Copy link
Member

hannesm commented Feb 4, 2023

superseeded by #161

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants