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

Introduce less ambiguous option for disabling certificate verification? #732

Open
DerGuteMoritz opened this issue Jun 18, 2024 · 6 comments

Comments

@DerGuteMoritz
Copy link
Collaborator

The insecure? option in Aleph is somewhat ambiguous: It specifically refers to disabling TLS certificate verification but it may also be interpreted to mean to disable TLS. Perhaps it would make sense to deprecated it and introduce a more explicit new option like disable-certificate-verification? to resolve this. Then again, it's a pretty established name, e.g. clj-http uses it, too, and so does curl by way of its --insecure flag.

Prompted by #730 (comment)

@arnaudgeiser
Copy link
Collaborator

Then again, it's a pretty established name, e.g. clj-http uses it, too, and so does curl by way of its --insecure flag.

Although it's not super clear, it's pretty standard and I don't think we need to go into it here.

@KingMob
Copy link
Collaborator

KingMob commented Jun 19, 2024

I think we should introduce a new flag, and in the docstring, mention that --insecure is an alias for it. Anyone searching for that exact string can still find it, and anyone searching more broadly for what it represents, will hopefully find the new flag more easily.

@bitti
Copy link

bitti commented Jun 20, 2024

Shouldn't that issue be moot when moving this option into the :ssl-context map? (Configuring this implies TLS/SSL). Furthermore, this can already be configured by using an appropriate SslContext instance for the :ssl-context (you even provide the helper fn aleph.netty/insecure-ssl-client-context for this). So having this extra option outside of :ssl-context leaves two potentially conflicting ways to configure this.

@KingMob
Copy link
Collaborator

KingMob commented Jun 21, 2024

@bitti For backwards compatibility, we can't move/remove the existing :insecure? flag. We can hide its docs, but regardless of where the new flag goes, we'll still have to handle the same functionality being set in multiple places.

@bitti
Copy link

bitti commented Jun 25, 2024

If you're just searching for a different name: Python's requests lib has verify (which can be set to False or None, besides to an explicit CA chain, or just True for the system chains. Ruby has verify_mode which supports various OpenSSL constant (so it's more than just a flag). The curl insecure nomenclature probably comes from the fact that disabling verification makes using TLS insignificant as a security measure, even though technically it's 'just' the verification which is disabled.

@KingMob
Copy link
Collaborator

KingMob commented Jun 25, 2024

Hmm. verify is less misleading, but still too short; it leaves out relevant info. verify_mode is better if there's actual modes, though cert_verification_mode or verify_cert_mode are more clear still. I personally like verify-certificate?. We don't have to concern ourselves as much with being succinct, since it's way less likely people will type this at the command-line that often.

The curl insecure nomenclature probably comes from the fact that disabling verification makes using TLS insignificant as a security measure, even though technically it's 'just' the verification which is disabled.

That's still a misleading name, though, because it masks the fact that users still ARE encrypting with SSL. When the first S in SSL means "secure", an option called insecure suggests you're not using SSL/TLS at all.

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

No branches or pull requests

4 participants