Add possibility to establish SSL connection with SSL_VERIFY_NONE#72
Add possibility to establish SSL connection with SSL_VERIFY_NONE#72mtuleika-appcast wants to merge 5 commits intoredis:masterfrom appcast-inc:CLIC-22047-with-ssl-support
Conversation
|
I think first PR #69 can be merged and then the ssl support added. |
|
Well PR #69 has been merged... |
|
hey @ssinghi and team, would love for this to be merged if/when possible ! |
|
|
||
| $LDFLAGS << " #{hiredis_dir}/libhiredis.a" | ||
| $LDFLAGS << " #{hiredis_dir}/libhiredis_ssl.a" | ||
| $LDFLAGS << " -L/usr/local/opt/openssl/lib" |
There was a problem hiding this comment.
I don't think we can assume that OpenSSL is here. We may want to use pkg_config to determine the LDFLAGS and CFLAGS.
|
|
||
| redisInitOpenSSL(); | ||
| ssl = redisCreateSSLContext(NULL, NULL, NULL, NULL, StringValuePtr(arg_host), &ssl_error); | ||
| SSL_CTX_set_verify(ssl->ssl_ctx, SSL_VERIFY_NONE, NULL); |
There was a problem hiding this comment.
I don't think SSL_VERIFY_NONE should be the default. I'd say that the default should be verify is enabled, but you may pass a verify option.
|
@stanhu I also saw https://github.com/michael-grunder/hiredis-rb/commits/ssl-support which is also an attempt to add SSL support. I saw it has some code to conditionally link OpenSSL: michael-grunder@a9897ec#diff-573a6b1ffeebb872ab4255105affc087786e9e64b583ebcf28bca619c0a1227aR48-R49 |
| #include <errno.h> | ||
| #include "hiredis_ext.h" | ||
|
|
||
| struct redisSSLContext { |
There was a problem hiding this comment.
This is a bit of an ugly hack since it's reaching inside the hiredis internals to get this information.
I think we're better off modifying hiredis to support the setting of verify mode: redis/hiredis#1085
|
@stanhu @ssinghi |
This PR adds possibility to establish SSL connection to Redis with SSL_VERIFY_NONE(in my case to ElastiCache).
What was done: