Skip to content

Add possibility to establish SSL connection with SSL_VERIFY_NONE#72

Closed
mtuleika-appcast wants to merge 5 commits intoredis:masterfrom
appcast-inc:CLIC-22047-with-ssl-support
Closed

Add possibility to establish SSL connection with SSL_VERIFY_NONE#72
mtuleika-appcast wants to merge 5 commits intoredis:masterfrom
appcast-inc:CLIC-22047-with-ssl-support

Conversation

@mtuleika-appcast
Copy link
Copy Markdown

This PR adds possibility to establish SSL connection to Redis with SSL_VERIFY_NONE(in my case to ElastiCache).

What was done:

  • hiredis submodule was bumped to version 1.0.0
  • C extension in hiredis-rb was extended to support SSL connection without certification
require 'hiredis'

conn = Hiredis::Connection.new
conn.connect_ssl('<host>', 6379)

conn.write %w[AUTH <password>]
puts conn.read
conn.write %w[SET speed awesome]
puts conn.read
conn.write %w[GET speed]
puts conn.read

@ssinghi
Copy link
Copy Markdown

ssinghi commented Sep 26, 2021

I think first PR #69 can be merged and then the ssl support added.

@milieu
Copy link
Copy Markdown

milieu commented Feb 16, 2022

Well PR #69 has been merged...

@ayoul3
Copy link
Copy Markdown

ayoul3 commented Mar 23, 2022

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"
Copy link
Copy Markdown
Contributor

@stanhu stanhu Aug 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@engwan
Copy link
Copy Markdown

engwan commented Aug 6, 2022

@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 {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@YegorZdanovich
Copy link
Copy Markdown

@stanhu @ssinghi
@mtuleika-appcast doesn't work on ruby projects anymore, maybe someone can continue this PR

@appcast-inc appcast-inc closed this by deleting the head repository Dec 26, 2023
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.

8 participants