Subject: [libssh2] Avoid calling EVP directly

[libssh2] Avoid calling EVP directly

From: Simon Josefsson <jas_at_extundo.com>
Date: Thu, 16 Nov 2006 21:02:11 +0100

This removes the LIBSSH2_CRYPT_METHOD_FLAG_EVP flag (and the entire
flag variable, although that part is not essential) and the flag's
semantic.

This moves some EVP_* calls into crypt.c, which is the first step
towards isolating all crypto functions and then replacing them.

As you can see, this moves some duplicated code into crypt.c, and
makes the core part (kex.c, packet.c, session.c) somewhat more
readable.

My indentation style may not be what libssh2 is using now... anyone
knows emacs setting to get the libssh2 style? Or parameters to
indent.

Comments or suggestions appreciated!

/Simon

Index: include/libssh2_priv.h
===================================================================
RCS file: /cvsroot/libssh2/libssh2/include/libssh2_priv.h,v
retrieving revision 1.30
diff -u -p -r1.30 libssh2_priv.h
--- include/libssh2_priv.h 22 Jun 2006 18:45:29 -0000 1.30
+++ include/libssh2_priv.h 16 Nov 2006 19:39:15 -0000
@@ -286,12 +286,6 @@ struct _LIBSSH2_HOSTKEY_METHOD {
         int (*dtor)(LIBSSH2_SESSION *session, void **abstract);
 };
 
-/* When FLAG_EVP is set, crypt contains a pointer to an EVP_CIPHER generator and init and dtor are ignored
- * Yes, I know it's a hack.
- */
-
-#define LIBSSH2_CRYPT_METHOD_FLAG_EVP 0x0001
-
 struct _LIBSSH2_CRYPT_METHOD {
         char *name;
 
@@ -301,8 +295,6 @@ struct _LIBSSH2_CRYPT_METHOD {
         int iv_len;
         int secret_len;
 
- long flags;
-
         int (*init)(LIBSSH2_SESSION *session, unsigned char *iv, int *free_iv, unsigned char *secret, int *free_secret, int encrypt, void **abstract);
         int (*crypt)(LIBSSH2_SESSION *session, unsigned char *block, void **abstract);
         int (*dtor)(LIBSSH2_SESSION *session, void **abstract);
Index: src/crypt.c
===================================================================
RCS file: /cvsroot/libssh2/libssh2/src/crypt.c,v
retrieving revision 1.4
diff -u -p -r1.4 crypt.c
--- src/crypt.c 2 Mar 2006 01:10:53 -0000 1.4
+++ src/crypt.c 16 Nov 2006 19:39:15 -0000
@@ -54,22 +54,71 @@ static LIBSSH2_CRYPT_METHOD libssh2_cryp
         8, /* blocksize (SSH2 defines minimum blocksize as 8) */
         0, /* iv_len */
         0, /* secret_len */
- 0, /* flags */
         NULL,
         libssh2_crypt_none_crypt,
         NULL
 };
 #endif
 
+#define MAKE_INIT(name, cipher) \
+ static int name (LIBSSH2_SESSION *session, \
+ unsigned char *iv, int *free_iv, \
+ unsigned char *secret, int *free_secret, \
+ int encrypt, void **abstract) \
+ { \
+ EVP_CIPHER_CTX *ctx = LIBSSH2_ALLOC(session, sizeof(EVP_CIPHER_CTX)); \
+ if (!ctx) \
+ return -1; \
+ EVP_CIPHER_CTX_init(ctx); \
+ EVP_CipherInit(ctx, cipher, secret, iv, encrypt); \
+ *abstract = ctx; \
+ *free_iv = 1; \
+ *free_secret = 1; \
+ return 0; \
+ }
+
+MAKE_INIT(aes256_init, EVP_aes_256_cbc())
+MAKE_INIT(aes192_init, EVP_aes_192_cbc())
+MAKE_INIT(aes128_init, EVP_aes_128_cbc())
+MAKE_INIT(blowfish_init, EVP_bf_cbc())
+MAKE_INIT(arcfour_init, EVP_rc4())
+MAKE_INIT(cast128_init, EVP_cast5_cbc())
+MAKE_INIT(des3_init, EVP_des_ede3_cbc())
+
+int crypt(LIBSSH2_SESSION *session, unsigned char *block, void **abstract)
+{
+ EVP_CIPHER_CTX *ctx = *(EVP_CIPHER_CTX **)abstract;
+ int blocksize = ctx->cipher->block_size;
+ unsigned char buf[EVP_MAX_BLOCK_LENGTH];
+ int ret;
+ if (blocksize == 1) /* Hack for arcfour. */
+ blocksize = 8;
+ ret = EVP_Cipher(ctx, buf, block, blocksize);
+ if (ret == 1)
+ memcpy(block, buf, blocksize);
+ return ret == 1 ? 0 : 1;
+}
+
+int dtor(LIBSSH2_SESSION *session, void **abstract)
+{
+ EVP_CIPHER_CTX **ctx = (EVP_CIPHER_CTX **)abstract;
+ if (ctx && *ctx)
+ {
+ EVP_CIPHER_CTX_cleanup(*ctx);
+ LIBSSH2_FREE(session, *ctx);
+ *abstract = NULL;
+ }
+ return 0;
+}
+
 static LIBSSH2_CRYPT_METHOD libssh2_crypt_method_3des_cbc = {
         "3des-cbc",
         8, /* blocksize */
         8, /* initial value length */
         24, /* secret length */
- LIBSSH2_CRYPT_METHOD_FLAG_EVP,
- NULL,
- (void*)EVP_des_ede3_cbc,
- NULL,
+ &des3_init,
+ &crypt,
+ &dtor
 };
 
 #if OPENSSL_VERSION_NUMBER >= 0x00907000L && !defined(OPENSSL_NO_AES)
@@ -78,10 +127,9 @@ static LIBSSH2_CRYPT_METHOD libssh2_cryp
         16, /* blocksize */
         16, /* initial value length */
         16, /* secret length -- 16*8 == 128bit */
- LIBSSH2_CRYPT_METHOD_FLAG_EVP,
- NULL,
- (void*)EVP_aes_128_cbc,
- NULL,
+ &aes128_init,
+ &crypt,
+ &dtor
 };
 
 static LIBSSH2_CRYPT_METHOD libssh2_crypt_method_aes192_cbc = {
@@ -89,10 +137,9 @@ static LIBSSH2_CRYPT_METHOD libssh2_cryp
         16, /* blocksize */
         16, /* initial value length */
         24, /* secret length -- 24*8 == 192bit */
- LIBSSH2_CRYPT_METHOD_FLAG_EVP,
- NULL,
- (void*)EVP_aes_192_cbc,
- NULL,
+ &aes192_init,
+ &crypt,
+ &dtor
 };
 
 static LIBSSH2_CRYPT_METHOD libssh2_crypt_method_aes256_cbc = {
@@ -100,10 +147,9 @@ static LIBSSH2_CRYPT_METHOD libssh2_cryp
         16, /* blocksize */
         16, /* initial value length */
         32, /* secret length -- 32*8 == 256bit */
- LIBSSH2_CRYPT_METHOD_FLAG_EVP,
- NULL,
- (void*)EVP_aes_256_cbc,
- NULL,
+ &aes256_init,
+ &crypt,
+ &dtor
 };
 
 /* rijndael-cbc_at_lysator.liu.se == aes256-cbc */
@@ -112,10 +158,9 @@ static LIBSSH2_CRYPT_METHOD libssh2_cryp
         16, /* blocksize */
         16, /* initial value length */
         32, /* secret length -- 32*8 == 256bit */
- LIBSSH2_CRYPT_METHOD_FLAG_EVP,
- NULL,
- (void*)EVP_aes_256_cbc,
- NULL,
+ &aes256_init,
+ &crypt,
+ &dtor
 };
 #endif /* OPENSSL_VERSION_NUMBER >= 0x00907000L && !defined(OPENSSL_NO_AES)*/
 
@@ -125,10 +170,9 @@ static LIBSSH2_CRYPT_METHOD libssh2_cryp
         8, /* blocksize */
         8, /* initial value length */
         16, /* secret length */
- LIBSSH2_CRYPT_METHOD_FLAG_EVP,
- NULL,
- (void*)EVP_bf_cbc,
- NULL,
+ &blowfish_init,
+ &crypt,
+ &dtor
 };
 #endif /* ! OPENSSL_NO_BLOWFISH */
 
@@ -138,10 +182,9 @@ static LIBSSH2_CRYPT_METHOD libssh2_cryp
         8, /* blocksize */
         8, /* initial value length */
         16, /* secret length */
- LIBSSH2_CRYPT_METHOD_FLAG_EVP,
- NULL,
- (void*)EVP_cast5_cbc,
- NULL,
+ &cast128_init,
+ &crypt,
+ &dtor
 };
 #endif /* ! OPENSSL_NO_CAST */
 
@@ -151,10 +194,9 @@ static LIBSSH2_CRYPT_METHOD libssh2_cryp
         8, /* blocksize */
         8, /* initial value length */
         16, /* secret length */
- LIBSSH2_CRYPT_METHOD_FLAG_EVP,
- NULL,
- (void*)EVP_rc4,
- NULL,
+ &arcfour_init,
+ &crypt,
+ &dtor
 };
 #endif /* ! OPENSSL_NO_RC4 */
 
Index: src/kex.c
===================================================================
RCS file: /cvsroot/libssh2/libssh2/src/kex.c,v
retrieving revision 1.15
diff -u -p -r1.15 kex.c
--- src/kex.c 22 Jun 2006 18:45:29 -0000 1.15
+++ src/kex.c 16 Nov 2006 19:39:15 -0000
@@ -38,7 +38,6 @@
 #include "libssh2_priv.h"
 #include <openssl/bn.h>
 #include <openssl/sha.h>
-#include <openssl/evp.h>
 #include <openssl/rand.h>
 
 /* TODO: Switch this to an inline and handle alloc() failures */
@@ -331,45 +330,23 @@ static int libssh2_kex_method_diffie_hel
 #endif
         }
 
- /* Calculate IV/Secret/Key for each direction */
- if (session->local.crypt->flags & LIBSSH2_CRYPT_METHOD_FLAG_EVP) {
- if (session->local.crypt_abstract) {
- EVP_CIPHER_CTX_cleanup(session->local.crypt_abstract);
- LIBSSH2_FREE(session, session->local.crypt_abstract);
- session->local.crypt_abstract = NULL;
- }
- } else {
- if (session->local.crypt->dtor) {
- /* Cleanup any existing cipher */
- session->local.crypt->dtor(session, &session->local.crypt_abstract);
- }
+ /* Cleanup any existing cipher */
+ if (session->local.crypt->dtor) {
+ session->local.crypt->dtor(session, &session->local.crypt_abstract);
         }
 
- if (session->local.crypt->init || (session->local.crypt->flags & LIBSSH2_CRYPT_METHOD_FLAG_EVP)) {
+ /* Calculate IV/Secret/Key for each direction */
+ if (session->local.crypt->init) {
                 unsigned char *iv = NULL, *secret = NULL;
                 int free_iv = 0, free_secret = 0;
 
                 LIBSSH2_KEX_METHOD_DIFFIE_HELLMAN_SHA1_HASH(iv, session->local.crypt->iv_len, "A");
                 LIBSSH2_KEX_METHOD_DIFFIE_HELLMAN_SHA1_HASH(secret, session->local.crypt->secret_len, "C");
- if (session->local.crypt->flags & LIBSSH2_CRYPT_METHOD_FLAG_EVP) {
- EVP_CIPHER *(*get_cipher)(void) = (void*)session->local.crypt->crypt;
- EVP_CIPHER *cipher = get_cipher();
- EVP_CIPHER_CTX *ctx;
-
- ctx = LIBSSH2_ALLOC(session, sizeof(EVP_CIPHER_CTX));
- if (!ctx) {
- LIBSSH2_FREE(session, iv);
- LIBSSH2_FREE(session, secret);
- ret = -1;
- goto clean_exit;
- }
- EVP_CIPHER_CTX_init(ctx);
- EVP_CipherInit(ctx, cipher, secret, iv, 1);
- session->local.crypt_abstract = ctx;
- free_iv = 1;
- free_secret = 1;
- } else {
- session->local.crypt->init(session, iv, &free_iv, secret, &free_secret, 1, &session->local.crypt_abstract);
+ if (session->local.crypt->init(session, iv, &free_iv, secret, &free_secret, 1, &session->local.crypt_abstract)) {
+ LIBSSH2_FREE(session, iv);
+ LIBSSH2_FREE(session, secret);
+ ret = -1;
+ goto clean_exit;
                 }
 
                 if (free_iv) {
@@ -386,44 +363,22 @@ static int libssh2_kex_method_diffie_hel
         _libssh2_debug(session, LIBSSH2_DBG_KEX, "Client to Server IV and Key calculated");
 #endif
 
- if (session->remote.crypt->flags & LIBSSH2_CRYPT_METHOD_FLAG_EVP) {
- if (session->remote.crypt_abstract) {
- EVP_CIPHER_CTX_cleanup(session->remote.crypt_abstract);
- LIBSSH2_FREE(session, session->remote.crypt_abstract);
- session->remote.crypt_abstract = NULL;
- }
- } else {
- if (session->remote.crypt->dtor) {
- /* Cleanup any existing cipher */
- session->remote.crypt->dtor(session, &session->remote.crypt_abstract);
- }
+ if (session->remote.crypt->dtor) {
+ /* Cleanup any existing cipher */
+ session->remote.crypt->dtor(session, &session->remote.crypt_abstract);
         }
 
- if (session->remote.crypt->init || (session->remote.crypt->flags & LIBSSH2_CRYPT_METHOD_FLAG_EVP)) {
+ if (session->remote.crypt->init) {
                 unsigned char *iv = NULL, *secret = NULL;
                 int free_iv = 0, free_secret = 0;
 
                 LIBSSH2_KEX_METHOD_DIFFIE_HELLMAN_SHA1_HASH(iv, session->remote.crypt->iv_len, "B");
                 LIBSSH2_KEX_METHOD_DIFFIE_HELLMAN_SHA1_HASH(secret, session->remote.crypt->secret_len, "D");
- if (session->remote.crypt->flags & LIBSSH2_CRYPT_METHOD_FLAG_EVP) {
- EVP_CIPHER *(*get_cipher)(void) = (void*)session->remote.crypt->crypt;
- EVP_CIPHER *cipher = get_cipher();
- EVP_CIPHER_CTX *ctx;
-
- ctx = LIBSSH2_ALLOC(session, sizeof(EVP_CIPHER_CTX));
- if (!ctx) {
- LIBSSH2_FREE(session, iv);
- LIBSSH2_FREE(session, secret);
- ret = -1;
- goto clean_exit;
- }
- EVP_CIPHER_CTX_init(ctx);
- EVP_CipherInit(ctx, cipher, secret, iv, 0);
- session->remote.crypt_abstract = ctx;
- free_iv = 1;
- free_secret = 1;
- } else {
- session->remote.crypt->init(session, iv, &free_iv, secret, &free_secret, 0, &session->remote.crypt_abstract);
+ if (session->remote.crypt->init(session, iv, &free_iv, secret, &free_secret, 0, &session->remote.crypt_abstract)) {
+ LIBSSH2_FREE(session, iv);
+ LIBSSH2_FREE(session, secret);
+ ret = -1;
+ goto clean_exit;
                 }
 
                 if (free_iv) {
Index: src/packet.c
===================================================================
RCS file: /cvsroot/libssh2/libssh2/src/packet.c,v
retrieving revision 1.37
diff -u -p -r1.37 packet.c
--- src/packet.c 14 Nov 2006 01:30:39 -0000 1.37
+++ src/packet.c 16 Nov 2006 19:39:15 -0000
@@ -41,7 +41,6 @@
 #ifndef WIN32
 #include <unistd.h>
 #endif
-#include <openssl/evp.h>
 #include <openssl/rand.h>
 
 /* Needed for struct iovec on some platforms */
@@ -753,9 +752,6 @@ int libssh2_packet_read(LIBSSH2_SESSION
                 int macstate;
                 int free_payload = 1;
 
- /* Safely ignored in CUSTOM cipher mode */
- EVP_CIPHER_CTX *ctx = (EVP_CIPHER_CTX *)session->remote.crypt_abstract;
-
                 /* Note: If we add any cipher with a blocksize less than 6 we'll need to get more creative with this
                  * For now, all blocksize sizes are 8+
                  */
@@ -779,14 +775,9 @@ int libssh2_packet_read(LIBSSH2_SESSION
                         return (session->socket_state == LIBSSH2_SOCKET_DISCONNECTED) ? 0 : -1;
                 }
 
- if (session->remote.crypt->flags & LIBSSH2_CRYPT_METHOD_FLAG_EVP) {
- EVP_Cipher(ctx, block + blocksize, block, blocksize);
- memcpy(block, block + blocksize, blocksize);
- } else {
- if (session->remote.crypt->crypt(session, block, &session->remote.crypt_abstract)) {
- libssh2_error(session, LIBSSH2_ERROR_DECRYPT, "Error decrypting packet preamble", 0);
- return -1;
- }
+ if (session->remote.crypt->crypt(session, block, &session->remote.crypt_abstract)) {
+ libssh2_error(session, LIBSSH2_ERROR_DECRYPT, "Error decrypting packet preamble", 0);
+ return -1;
                 }
 
                 packet_len = libssh2_ntohu32(block);
@@ -828,17 +819,12 @@ int libssh2_packet_read(LIBSSH2_SESSION
                                 LIBSSH2_FREE(session, payload);
                                 return -1;
                         }
- if (session->remote.crypt->flags & LIBSSH2_CRYPT_METHOD_FLAG_EVP) {
- EVP_Cipher(ctx, block + blocksize, block, blocksize);
- memcpy(s, block + blocksize, blocksize);
- } else {
- if (session->remote.crypt->crypt(session, block, &session->remote.crypt_abstract)) {
- libssh2_error(session, LIBSSH2_ERROR_DECRYPT, "Error decrypting packet preamble", 0);
- LIBSSH2_FREE(session, payload);
- return -1;
- }
- memcpy(s, block, blocksize);
+ if (session->remote.crypt->crypt(session, block, &session->remote.crypt_abstract)) {
+ libssh2_error(session, LIBSSH2_ERROR_DECRYPT, "Error decrypting packet preamble", 0);
+ LIBSSH2_FREE(session, payload);
+ return -1;
                         }
+ memcpy(s, block, blocksize);
 
                         s += blocksize;
                 }
@@ -1214,9 +1200,6 @@ int libssh2_packet_write(LIBSSH2_SESSION
                 unsigned char *encbuf, *s;
                 int ret;
 
- /* Safely ignored in CUSTOM cipher mode */
- EVP_CIPHER_CTX *ctx = (EVP_CIPHER_CTX *)session->local.crypt_abstract;
-
                 /* include packet_length(4) itself and room for the hash at the end */
                 encbuf = LIBSSH2_ALLOC(session, 4 + packet_length + session->local.mac->mac_len);
                 if (!encbuf) {
@@ -1240,12 +1223,7 @@ int libssh2_packet_write(LIBSSH2_SESSION
 
                 /* Encrypt data */
                 for(s = encbuf; (s - encbuf) < (4 + packet_length) ; s += session->local.crypt->blocksize) {
- if (session->local.crypt->flags & LIBSSH2_CRYPT_METHOD_FLAG_EVP) {
- EVP_Cipher(ctx, buf, s, session->local.crypt->blocksize);
- memcpy(s, buf, session->local.crypt->blocksize);
- } else {
- session->local.crypt->crypt(session, s, &session->local.crypt_abstract);
- }
+ session->local.crypt->crypt(session, s, &session->local.crypt_abstract);
                 }
 
                 session->local.seqno++;
Index: src/session.c
===================================================================
RCS file: /cvsroot/libssh2/libssh2/src/session.c,v
retrieving revision 1.27
diff -u -p -r1.27 session.c
--- src/session.c 17 Apr 2006 02:49:44 -0000 1.27
+++ src/session.c 16 Nov 2006 19:39:15 -0000
@@ -410,16 +410,8 @@ LIBSSH2_API void libssh2_session_free(LI
 
                 /* Client to Server */
                 /* crypt */
- if (session->local.crypt) {
- if (session->local.crypt->flags & LIBSSH2_CRYPT_METHOD_FLAG_EVP) {
- if (session->local.crypt_abstract) {
- EVP_CIPHER_CTX_cleanup(session->local.crypt_abstract);
- LIBSSH2_FREE(session, session->local.crypt_abstract);
- session->local.crypt_abstract = NULL;
- }
- } else if (session->local.crypt->dtor) {
- session->local.crypt->dtor(session, &session->local.crypt_abstract);
- }
+ if (session->local.crypt && session->local.crypt->dtor) {
+ session->local.crypt->dtor(session, &session->local.crypt_abstract);
                 }
                 /* comp */
                 if (session->local.comp && session->local.comp->dtor) {
@@ -432,16 +424,8 @@ LIBSSH2_API void libssh2_session_free(LI
 
                 /* Server to Client */
                 /* crypt */
- if (session->remote.crypt) {
- if (session->remote.crypt->flags & LIBSSH2_CRYPT_METHOD_FLAG_EVP) {
- if (session->remote.crypt_abstract) {
- EVP_CIPHER_CTX_cleanup(session->remote.crypt_abstract);
- LIBSSH2_FREE(session, session->remote.crypt_abstract);
- session->remote.crypt_abstract = NULL;
- }
- } else if (session->remote.crypt->dtor) {
- session->remote.crypt->dtor(session, &session->remote.crypt_abstract);
- }
+ if (session->remote.crypt && session->remote.crypt->dtor) {
+ session->remote.crypt->dtor(session, &session->remote.crypt_abstract);
                 }
                 /* comp */
                 if (session->remote.comp && session->remote.comp->dtor) {

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
libssh2-devel mailing list
libssh2-devel_at_lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/libssh2-devel
Received on 2006-11-16