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

Make it possible to set SSL verify mode #1085

Merged
merged 1 commit into from
Aug 16, 2022

Conversation

stanhu
Copy link
Contributor

@stanhu stanhu commented Aug 10, 2022

If no SSL certificates are provided, many Redis clients default to
disabling SSL peer verification. Previously it was a bit cumbersome to
configure this because the client would either have to reimplement
redisCreateSSLContext() or reach into the internals to set the
OpenSSL verify mode.

We can improve the SSL API by introducing a
redisCreateSSLContextWithOptions() call that takes into structured
parameters for SSL initialization. This structure contains a verify
mode that is used to set the OpenSSL verify mode.

Relates to #646

@michael-grunder
Copy link
Collaborator

I'm not opposed to this in principle, as it does provide quite a bit of convenience.

Presently there's quite a bit of boilerplate involved when you're just testing a TLS connection and you don't want/have peer verification.

That said, if we do add a simple options struct, perhaps we should also incorporate other simple options, such as one that would call SSL_CTX_set_default_verify_paths?

@yossigo let me know if you have any thoughts.

@yossigo
Copy link
Member

yossigo commented Aug 14, 2022

@michael-grunder I'm not against that either, but slightly concerned about not ending up wrapping too much of OpenSSL. Ideally, we should be able to draw a line and define what capabilities we provide for convenience and what is out of scope and requires users to handle their OpenSSL context.

@stanhu
Copy link
Contributor Author

stanhu commented Aug 14, 2022

Another alternative would be to access the SSL_CTX pointer by exporting the redisSSLContext structure.

@yossigo
Copy link
Member

yossigo commented Aug 15, 2022

@stanhu There's already redisInitiateSSL() for those folks who are willing to use OpenSSL directly, what would be the benefit of also exposing SSL_CTX from redisSSLContext?

@stanhu
Copy link
Contributor Author

stanhu commented Aug 15, 2022

@yossigo redisCreateSSLContext() does a fair amount of work to set up the SSL paths. Callers would essentially have to duplicate that code if they want to use redisInitializeSSL().

ssl.c Outdated Show resolved Hide resolved
@michael-grunder
Copy link
Collaborator

If you don't mind making the aesthetic change, I will get this merged and we can determine later if we want to extend the options at all.

If no SSL certificates are provided, many Redis clients default to
disabling SSL peer verification. Previously it was a bit cumbersome to
configure this because the client would either have to reimplement
`redisCreateSSLContext()` or reach into the internals to set the
OpenSSL verify mode.

We can improve the SSL API by introducing a
`redisCreateSSLContextWithOptions()` call that takes into structured
parameters for SSL initialization. This structure contains a verify
mode that is used to set the OpenSSL verify mode.

Relates to redis#646
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

Successfully merging this pull request may close these issues.

None yet

3 participants