C client SSL implementation: request for initial review

bookshelfdave commented 10 years ago

added a comment for BIO_free() in riak_connection_free, it will remain commented out
once this is cleaned up a bit more, I'll submit an official PR
you'll need the autoconf-archive package installed. If you are on OSX, I believe brew install autoconf-archive does the trick.

(cd test/ssl_tests && ./build_test_node.sh)
./autogen.sh
./configure -with-openssl=/usr/local/Cellar/openssl/1.0.1f
make
./riak_c_example --port 8087 -S -U foo -P bar -C ./test/ssl_tests/certs/cacert.pem --put --bucket TestBucket --key TestKey --value FooBar123
./riak_c_example --port 8087 -S -U foo -P bar -C ./test/ssl_tests/certs/cacert.pem --get --bucket TestBucket --key TestKey

Only tested on OSX at this point, Linux testing is next.

cc @javajolt @broach @bkerley

bookshelfdave commented 10 years ago

cc @Vagabond

hazen commented 10 years ago

Lots of build warnings on OSX 10.9.2

src/riak_connection.c:82:5: warning: 'BIO_write' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
    BIO_write(bio, &length, 4);
    ^
/usr/include/openssl/bio.h:597:5: note: 'BIO_write' declared here
int     BIO_write(BIO *b, const void *data, int len) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:83:5: warning: 'BIO_write' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
    BIO_write(bio, &messageCode, 1);
    ^
/usr/include/openssl/bio.h:597:5: note: 'BIO_write' declared here
int     BIO_write(BIO *b, const void *data, int len) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:87:17: warning: 'BIO_read' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
        int p = BIO_read(bio, b+total, 5);
                ^
/usr/include/openssl/bio.h:595:5: note: 'BIO_read' declared here
int     BIO_read(BIO *b, void *data, int len) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:113:5: warning: 'SSL_library_init' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
    SSL_library_init();
    ^
/usr/include/openssl/ssl.h:1553:5: note: 'SSL_library_init' declared here
int SSL_library_init(void ) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
    ^
src/riak_connection.c:116:32: warning: passing 'const SSL_METHOD *' (aka 'const struct ssl_method_st *') to parameter of
      type 'SSL_METHOD *' (aka 'struct ssl_method_st *') discards qualifiers
      [-Wincompatible-pointer-types-discards-qualifiers]
    SSL_CTX* ctx = SSL_CTX_new(sc->ssl_method);
                               ^~~~~~~~~~~~~~
/usr/include/openssl/ssl.h:1346:34: note: passing argument to parameter 'meth' here
SSL_CTX *SSL_CTX_new(SSL_METHOD *meth) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
                                 ^
src/riak_connection.c:116:20: warning: 'SSL_CTX_new' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
    SSL_CTX* ctx = SSL_CTX_new(sc->ssl_method);
                   ^
/usr/include/openssl/ssl.h:1346:10: note: 'SSL_CTX_new' declared here
SSL_CTX *SSL_CTX_new(SSL_METHOD *meth) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
         ^
src/riak_connection.c:120:9: warning: 'SSL_CTX_free' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
        SSL_CTX_free(ctx);
        ^
/usr/include/openssl/ssl.h:1347:6: note: 'SSL_CTX_free' declared here
void    SSL_CTX_free(SSL_CTX *) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:126:9: warning: 'SSL_CTX_free' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
        SSL_CTX_free(ctx);
        ^
/usr/include/openssl/ssl.h:1347:6: note: 'SSL_CTX_free' declared here
void    SSL_CTX_free(SSL_CTX *) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:129:9: warning: 'SSL_CTX_load_verify_locations' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
    if(!SSL_CTX_load_verify_locations(ctx, sc->cacertfile, NULL)) {
        ^
/usr/include/openssl/ssl.h:1571:5: note: 'SSL_CTX_load_verify_locations' declared here
int SSL_CTX_load_verify_locations(SSL_CTX *ctx, const char *CAfile,
    ^
src/riak_connection.c:131:9: warning: 'SSL_CTX_free' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
        SSL_CTX_free(ctx);
        ^
/usr/include/openssl/ssl.h:1347:6: note: 'SSL_CTX_free' declared here
void    SSL_CTX_free(SSL_CTX *) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:135:11: warning: 'SSL_new' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
    ssl = SSL_new(ctx);
          ^
/usr/include/openssl/ssl.h:1481:7: note: 'SSL_new' declared here
SSL *   SSL_new(SSL_CTX *ctx) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:136:5: warning: 'SSL_set_bio' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
    SSL_set_bio(ssl,bio,bio);
    ^
/usr/include/openssl/ssl.h:1375:6: note: 'SSL_set_bio' declared here
void    SSL_set_bio(SSL *s, BIO *rbio,BIO *wbio) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:137:5: warning: 'SSL_set_connect_state' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
    SSL_set_connect_state(ssl);
    ^
/usr/include/openssl/ssl.h:1548:6: note: 'SSL_set_connect_state' declared here
void SSL_set_connect_state(SSL *s) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
     ^
src/riak_connection.c:139:30: warning: 'SSL_do_handshake' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
    int handshake_response = SSL_do_handshake(ssl);
                             ^
/usr/include/openssl/ssl.h:1529:5: note: 'SSL_do_handshake' declared here
int SSL_do_handshake(SSL *s) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
    ^
src/riak_connection.c:141:7: warning: initializing 'char *' with an expression of type 'const char *' discards
      qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
char *errstr = ERR_reason_error_string(SSL_get_error(ssl, handshake_response));
      ^        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
src/riak_connection.c:141:16: warning: 'ERR_reason_error_string' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
char *errstr = ERR_reason_error_string(SSL_get_error(ssl, handshake_response));
               ^
/usr/include/openssl/err.h:283:13: note: 'ERR_reason_error_string' declared here
const char *ERR_reason_error_string(unsigned long e) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
            ^
src/riak_connection.c:141:40: warning: 'SSL_get_error' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
char *errstr = ERR_reason_error_string(SSL_get_error(ssl, handshake_response));
                                       ^
/usr/include/openssl/ssl.h:1501:5: note: 'SSL_get_error' declared here
int     SSL_get_error(const SSL *s,int ret_code) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:143:9: warning: 'SSL_CTX_free' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
        SSL_CTX_free(ctx);
        ^
/usr/include/openssl/ssl.h:1347:6: note: 'SSL_CTX_free' declared here
void    SSL_CTX_free(SSL_CTX *) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:144:9: warning: 'SSL_free' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
        SSL_free(ssl);
        ^
/usr/include/openssl/ssl.h:1490:6: note: 'SSL_free' declared here
void    SSL_free(SSL *ssl) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:148:10: warning: 'SSL_get_peer_certificate' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
    if(!(SSL_get_peer_certificate(ssl) != NULL && SSL_get_verify_result(ssl) == X509_V_OK)) {
         ^
/usr/include/openssl/ssl.h:1450:8: note: 'SSL_get_peer_certificate' declared here
X509 *  SSL_get_peer_certificate(const SSL *s) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:148:51: warning: 'SSL_get_verify_result' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
    if(!(SSL_get_peer_certificate(ssl) != NULL && SSL_get_verify_result(ssl) == X509_V_OK)) {
                                                  ^
/usr/include/openssl/ssl.h:1584:6: note: 'SSL_get_verify_result' declared here
long SSL_get_verify_result(const SSL *ssl) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
     ^
src/riak_connection.c:149:18: warning: 'SSL_get_verify_result' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
        long l = SSL_get_verify_result(ssl);
                 ^
/usr/include/openssl/ssl.h:1584:6: note: 'SSL_get_verify_result' declared here
long SSL_get_verify_result(const SSL *ssl) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
     ^
src/riak_connection.c:153:34: warning: 'X509_verify_cert_error_string' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
                                 X509_verify_cert_error_string(l));
                                 ^
./src/include/riak_log.h:88:59: note: expanded from macro 'riak_log_critical_config'
        __func__, sizeof(__func__)-1, __LINE__, (format), __VA_ARGS__)
                                                          ^
/usr/include/openssl/x509.h:752:13: note: 'X509_verify_cert_error_string' declared here
const char *X509_verify_cert_error_string(long n) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
            ^
src/riak_connection.c:154:9: warning: 'SSL_CTX_free' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
        SSL_CTX_free(ctx);
        ^
/usr/include/openssl/ssl.h:1347:6: note: 'SSL_CTX_free' declared here
void    SSL_CTX_free(SSL_CTX *) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:155:9: warning: 'SSL_free' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
        SSL_free(ssl);
        ^
/usr/include/openssl/ssl.h:1490:6: note: 'SSL_free' declared here
void    SSL_free(SSL *ssl) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:228:9: warning: 'BIO_new' is deprecated: first deprecated in OS X 10.7 [-Wdeprecated-declarations]
    bio=BIO_new(BIO_s_socket());
        ^
/usr/include/openssl/bio.h:591:7: note: 'BIO_new' declared here
BIO *   BIO_new(BIO_METHOD *type) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:228:17: warning: 'BIO_s_socket' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
    bio=BIO_new(BIO_s_socket());
                ^
/usr/include/openssl/bio.h:628:13: note: 'BIO_s_socket' declared here
BIO_METHOD *BIO_s_socket(void) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
            ^
src/riak_connection.c:229:5: warning: 'BIO_int_ctrl' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
    BIO_set_fd(bio, cxn->fd, BIO_NOCLOSE);
    ^
/usr/include/openssl/bio.h:460:28: note: expanded from macro 'BIO_set_fd'
#define BIO_set_fd(b,fd,c)      BIO_int_ctrl(b,BIO_C_SET_FD,c,fd)
                                ^
/usr/include/openssl/bio.h:603:6: note: 'BIO_int_ctrl' declared here
long    BIO_int_ctrl(BIO *bp,int cmd,long larg,int iarg) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:238:7: warning: 'BIO_free_all' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
      BIO_free_all(bio);
      ^
/usr/include/openssl/bio.h:606:6: note: 'BIO_free_all' declared here
void    BIO_free_all(BIO *a) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:246:7: warning: 'BIO_free_all' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
      BIO_free_all(bio);
      ^
/usr/include/openssl/bio.h:606:6: note: 'BIO_free_all' declared here
void    BIO_free_all(BIO *a) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:258:7: warning: 'BIO_free_all' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
      BIO_free_all(bio);
      ^
/usr/include/openssl/bio.h:606:6: note: 'BIO_free_all' declared here
void    BIO_free_all(BIO *a) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:295:7: warning: 'SSL_free' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
      SSL_free(cxn->ssl);
      ^
/usr/include/openssl/ssl.h:1490:6: note: 'SSL_free' declared here
void    SSL_free(SSL *ssl) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:309:7: warning: 'SSL_CTX_free' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
      SSL_CTX_free(cxn->ssl_context);
      ^
/usr/include/openssl/ssl.h:1347:6: note: 'SSL_CTX_free' declared here
void    SSL_CTX_free(SSL_CTX *) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;
        ^
src/riak_connection.c:331:25: warning: 'SSLv23_client_method' is deprecated: first deprecated in OS X 10.7
      [-Wdeprecated-declarations]
    creds->ssl_method = SSLv23_client_method();
                        ^
/usr/include/openssl/ssl.h:1517:13: note: 'SSLv23_client_method' declared here
SSL_METHOD *SSLv23_client_method(void) DEPRECATED_IN_MAC_OS_X_VERSION_10_7_AND_LATER;   /* SSLv3 but can rollbac...
            ^
34 warnings generated.

bookshelfdave commented 10 years ago

@javajolt I don't have Mavericks (and I won't install it!).

I receive a ton of compile errors if I don't specify the OpenSSL version directly w/ configure: ./configure -with-openssl=/usr/local/Cellar/openssl/1.0.1f

hazen commented 10 years ago

Yes. Found out I was using the wrong library. Commented about the config line.

hazen commented 10 years ago

Hey! What do you know? If you use the correct OpenSSL library it works!

./riak_c_example -U foo -P bar -C test/ssl_tests/certs/cacert.pem --ping --port 8087
option -U with value `foo'
option -P with value `bar'
option -C with value `test/ssl_tests/certs/cacert.pem'
option -p with value `8087'
PONG

bookshelfdave commented 10 years ago

added a -S flag to the example to indicate that you'd like to make a secure connection

bookshelfdave commented 10 years ago

DP TODO: check that BIO_free_all() doesn't crash in ssl_handshake() when Riak is down

bookshelfdave commented 10 years ago

rebased + squashed in https://github.com/basho/riak-c-client/pull/58

basho-labs / riak-c-client

C client SSL implementation: request for initial review #57