[openssl-commits] [openssl] master update

Richard Levitte levitte at openssl.org
Fri Nov 2 19:22:34 UTC 2018


The branch master has been updated
       via  681e8cacdbdc44ac00af29b6656fc52745a9baa2 (commit)
       via  458c7dad9e3f59490fb0908c1a27ca39e4cf38dd (commit)
       via  28ac1bd9a97d9725273956e26d89ccfa5b4de67b (commit)
       via  36af124bfb209b49cb92a5fb9fab627d9cd4a44b (commit)
      from  2b201ce9417cc6e617b7ca2db0a99cc87bbc343b (commit)


- Log -----------------------------------------------------------------
commit 681e8cacdbdc44ac00af29b6656fc52745a9baa2
Author: Richard Levitte <levitte at openssl.org>
Date:   Fri Nov 2 10:24:24 2018 +0100

    crypto/engine/eng_devcrypto.c: ensure we don't leak resources
    
    If engine building fails for some reason, we must make sure to close
    the /dev/crypto handle.
    
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    (Merged from https://github.com/openssl/openssl/pull/7506)

commit 458c7dad9e3f59490fb0908c1a27ca39e4cf38dd
Author: Richard Levitte <levitte at openssl.org>
Date:   Wed Oct 31 19:23:44 2018 +0100

    crypto/engine/eng_devcrypto.c: open /dev/crypto only once
    
    We opened /dev/crypto once for each session, which is quite unnecessary.
    With this change, we open /dev/crypto once at engine init, and close
    it on unload.
    
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    (Merged from https://github.com/openssl/openssl/pull/7506)

commit 28ac1bd9a97d9725273956e26d89ccfa5b4de67b
Author: Richard Levitte <levitte at openssl.org>
Date:   Sat Oct 27 09:26:22 2018 +0200

    crypto/engine/eng_devcrypto.c: new compilers are strict on prototypes
    
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    (Merged from https://github.com/openssl/openssl/pull/7506)

commit 36af124bfb209b49cb92a5fb9fab627d9cd4a44b
Author: Richard Levitte <levitte at openssl.org>
Date:   Sat Oct 27 09:15:04 2018 +0200

    crypto/engine/eng_devcrypto.c: add digest copy
    
    Copying an EVP_MD_CTX, including the implementation local bits, is a
    necessary operation.  In this case, though, it's the same as
    initializing the local bits to be "copied to".
    
    Fixes #7495
    
    Reviewed-by: Bernd Edlinger <bernd.edlinger at hotmail.de>
    (Merged from https://github.com/openssl/openssl/pull/7506)

-----------------------------------------------------------------------

Summary of changes:
 crypto/engine/eng_devcrypto.c | 126 +++++++++++++++++++++++-------------------
 1 file changed, 68 insertions(+), 58 deletions(-)

diff --git a/crypto/engine/eng_devcrypto.c b/crypto/engine/eng_devcrypto.c
index 51105ec..4a0ba09 100644
--- a/crypto/engine/eng_devcrypto.c
+++ b/crypto/engine/eng_devcrypto.c
@@ -28,6 +28,13 @@
 # define CHECK_BSD_STYLE_MACROS
 #endif
 
+/*
+ * ONE global file descriptor for all sessions.  This allows operations
+ * such as digest session data copying (see digest_copy()), but is also
+ * saner...  why re-open /dev/crypto for every session?
+ */
+static int cfd;
+
 /******************************************************************************
  *
  * Ciphers
@@ -39,7 +46,6 @@
  *****/
 
 struct cipher_ctx {
-    int cfd;
     struct session_op sess;
 
     /* to pass from init to do_cipher */
@@ -135,19 +141,13 @@ static int cipher_init(EVP_CIPHER_CTX *ctx, const unsigned char *key,
     const struct cipher_data_st *cipher_d =
         get_cipher_data(EVP_CIPHER_CTX_nid(ctx));
 
-    if ((cipher_ctx->cfd = open("/dev/crypto", O_RDWR, 0)) < 0) {
-        SYSerr(SYS_F_OPEN, errno);
-        return 0;
-    }
-
     memset(&cipher_ctx->sess, 0, sizeof(cipher_ctx->sess));
     cipher_ctx->sess.cipher = cipher_d->devcryptoid;
     cipher_ctx->sess.keylen = cipher_d->keylen;
     cipher_ctx->sess.key = (void *)key;
     cipher_ctx->op = enc ? COP_ENCRYPT : COP_DECRYPT;
-    if (ioctl(cipher_ctx->cfd, CIOCGSESSION, &cipher_ctx->sess) < 0) {
+    if (ioctl(cfd, CIOCGSESSION, &cipher_ctx->sess) < 0) {
         SYSerr(SYS_F_IOCTL, errno);
-        close(cipher_ctx->cfd);
         return 0;
     }
 
@@ -186,7 +186,7 @@ static int cipher_do_cipher(EVP_CIPHER_CTX *ctx, unsigned char *out,
     cryp.flags = COP_FLAG_WRITE_IV;
 #endif
 
-    if (ioctl(cipher_ctx->cfd, CIOCCRYPT, &cryp) < 0) {
+    if (ioctl(cfd, CIOCCRYPT, &cryp) < 0) {
         SYSerr(SYS_F_IOCTL, errno);
         return 0;
     }
@@ -212,14 +212,10 @@ static int cipher_cleanup(EVP_CIPHER_CTX *ctx)
     struct cipher_ctx *cipher_ctx =
         (struct cipher_ctx *)EVP_CIPHER_CTX_get_cipher_data(ctx);
 
-    if (ioctl(cipher_ctx->cfd, CIOCFSESSION, &cipher_ctx->sess.ses) < 0) {
+    if (ioctl(cfd, CIOCFSESSION, &cipher_ctx->sess.ses) < 0) {
         SYSerr(SYS_F_IOCTL, errno);
         return 0;
     }
-    if (close(cipher_ctx->cfd) < 0) {
-        SYSerr(SYS_F_CLOSE, errno);
-        return 0;
-    }
 
     return 1;
 }
@@ -233,14 +229,10 @@ static int known_cipher_nids[OSSL_NELEM(cipher_data)];
 static int known_cipher_nids_amount = -1; /* -1 indicates not yet initialised */
 static EVP_CIPHER *known_cipher_methods[OSSL_NELEM(cipher_data)] = { NULL, };
 
-static void prepare_cipher_methods()
+static void prepare_cipher_methods(void)
 {
     size_t i;
     struct session_op sess;
-    int cfd;
-
-    if ((cfd = open("/dev/crypto", O_RDWR, 0)) < 0)
-        return;
 
     memset(&sess, 0, sizeof(sess));
     sess.key = (void *)"01234567890123456789012345678901234567890123456789";
@@ -281,8 +273,6 @@ static void prepare_cipher_methods()
                 cipher_data[i].nid;
         }
     }
-
-    close(cfd);
 }
 
 static const EVP_CIPHER *get_cipher_method(int nid)
@@ -308,7 +298,7 @@ static void destroy_cipher_method(int nid)
     known_cipher_methods[i] = NULL;
 }
 
-static void destroy_all_cipher_methods()
+static void destroy_all_cipher_methods(void)
 {
     size_t i;
 
@@ -329,11 +319,12 @@ static int devcrypto_ciphers(ENGINE *e, const EVP_CIPHER **cipher,
 
 /*
  * We only support digests if the cryptodev implementation supports multiple
- * data updates.  Otherwise, we would be forced to maintain a cache, which is
- * perilous if there's a lot of data coming in (if someone wants to checksum
- * an OpenSSL tarball, for example).
+ * data updates and session copying.  Otherwise, we would be forced to maintain
+ * a cache, which is perilous if there's a lot of data coming in (if someone
+ * wants to checksum an OpenSSL tarball, for example).
  */
-#if defined(COP_FLAG_UPDATE) && defined(COP_FLAG_FINAL)
+#if defined(CIOCCPHASH) && defined(COP_FLAG_UPDATE) && defined(COP_FLAG_FINAL)
+#define IMPLEMENT_DIGEST
 
 /******************************************************************************
  *
@@ -346,7 +337,6 @@ static int devcrypto_ciphers(ENGINE *e, const EVP_CIPHER **cipher,
  *****/
 
 struct digest_ctx {
-    int cfd;
     struct session_op sess;
     int init;
 };
@@ -413,19 +403,12 @@ static int digest_init(EVP_MD_CTX *ctx)
     const struct digest_data_st *digest_d =
         get_digest_data(EVP_MD_CTX_type(ctx));
 
-    if (digest_ctx->init == 0
-        && (digest_ctx->cfd = open("/dev/crypto", O_RDWR, 0)) < 0) {
-        SYSerr(SYS_F_OPEN, errno);
-        return 0;
-    }
-
     digest_ctx->init = 1;
 
     memset(&digest_ctx->sess, 0, sizeof(digest_ctx->sess));
     digest_ctx->sess.mac = digest_d->devcryptoid;
-    if (ioctl(digest_ctx->cfd, CIOCGSESSION, &digest_ctx->sess) < 0) {
+    if (ioctl(cfd, CIOCGSESSION, &digest_ctx->sess) < 0) {
         SYSerr(SYS_F_IOCTL, errno);
-        close(digest_ctx->cfd);
         return 0;
     }
 
@@ -444,7 +427,7 @@ static int digest_op(struct digest_ctx *ctx, const void *src, size_t srclen,
     cryp.dst = NULL;
     cryp.mac = res;
     cryp.flags = flags;
-    return ioctl(ctx->cfd, CIOCCRYPT, &cryp);
+    return ioctl(cfd, CIOCCRYPT, &cryp);
 }
 
 static int digest_update(EVP_MD_CTX *ctx, const void *data, size_t count)
@@ -472,7 +455,7 @@ static int digest_final(EVP_MD_CTX *ctx, unsigned char *md)
         SYSerr(SYS_F_IOCTL, errno);
         return 0;
     }
-    if (ioctl(digest_ctx->cfd, CIOCFSESSION, &digest_ctx->sess.ses) < 0) {
+    if (ioctl(cfd, CIOCFSESSION, &digest_ctx->sess.ses) < 0) {
         SYSerr(SYS_F_IOCTL, errno);
         return 0;
     }
@@ -480,16 +463,38 @@ static int digest_final(EVP_MD_CTX *ctx, unsigned char *md)
     return 1;
 }
 
-static int digest_cleanup(EVP_MD_CTX *ctx)
+static int digest_copy(EVP_MD_CTX *to, const EVP_MD_CTX *from)
 {
-    struct digest_ctx *digest_ctx =
-        (struct digest_ctx *)EVP_MD_CTX_md_data(ctx);
+    struct digest_ctx *digest_from =
+        (struct digest_ctx *)EVP_MD_CTX_md_data(from);
+    struct digest_ctx *digest_to =
+        (struct digest_ctx *)EVP_MD_CTX_md_data(to);
+    struct cphash_op cphash;
+
+    if (digest_from == NULL)
+        return 1;
 
-    if (close(digest_ctx->cfd) < 0) {
-        SYSerr(SYS_F_CLOSE, errno);
+    if (digest_from->init != 1) {
+        SYSerr(SYS_F_IOCTL, EINVAL);
         return 0;
     }
 
+    if (!digest_init(to)) {
+        SYSerr(SYS_F_IOCTL, errno);
+        return 0;
+    }
+
+    cphash.src_ses = digest_from->sess.ses;
+    cphash.dst_ses = digest_to->sess.ses;
+    if (ioctl(cfd, CIOCCPHASH, &cphash) < 0) {
+        SYSerr(SYS_F_IOCTL, errno);
+        return 0;
+    }
+    return 1;
+}
+
+static int digest_cleanup(EVP_MD_CTX *ctx)
+{
     return 1;
 }
 
@@ -502,14 +507,10 @@ static int known_digest_nids[OSSL_NELEM(digest_data)];
 static int known_digest_nids_amount = -1; /* -1 indicates not yet initialised */
 static EVP_MD *known_digest_methods[OSSL_NELEM(digest_data)] = { NULL, };
 
-static void prepare_digest_methods()
+static void prepare_digest_methods(void)
 {
     size_t i;
     struct session_op sess;
-    int cfd;
-
-    if ((cfd = open("/dev/crypto", O_RDWR, 0)) < 0)
-        return;
 
     memset(&sess, 0, sizeof(sess));
 
@@ -532,6 +533,7 @@ static void prepare_digest_methods()
             || !EVP_MD_meth_set_init(known_digest_methods[i], digest_init)
             || !EVP_MD_meth_set_update(known_digest_methods[i], digest_update)
             || !EVP_MD_meth_set_final(known_digest_methods[i], digest_final)
+            || !EVP_MD_meth_set_copy(known_digest_methods[i], digest_copy)
             || !EVP_MD_meth_set_cleanup(known_digest_methods[i], digest_cleanup)
             || !EVP_MD_meth_set_app_datasize(known_digest_methods[i],
                                              sizeof(struct digest_ctx))) {
@@ -541,8 +543,6 @@ static void prepare_digest_methods()
             known_digest_nids[known_digest_nids_amount++] = digest_data[i].nid;
         }
     }
-
-    close(cfd);
 }
 
 static const EVP_MD *get_digest_method(int nid)
@@ -568,7 +568,7 @@ static void destroy_digest_method(int nid)
     known_digest_methods[i] = NULL;
 }
 
-static void destroy_all_digest_methods()
+static void destroy_all_digest_methods(void)
 {
     size_t i;
 
@@ -598,9 +598,12 @@ static int devcrypto_digests(ENGINE *e, const EVP_MD **digest,
 static int devcrypto_unload(ENGINE *e)
 {
     destroy_all_cipher_methods();
-#if defined(COP_FLAG_UPDATE) && defined(COP_FLAG_FINAL)
+#ifdef IMPLEMENT_DIGEST
     destroy_all_digest_methods();
 #endif
+
+    close(cfd);
+
     return 1;
 }
 /*
@@ -611,23 +614,30 @@ void engine_load_devcrypto_int()
 {
     ENGINE *e = NULL;
 
-    if (access("/dev/crypto", R_OK | W_OK) < 0) {
-        fprintf(stderr,
-                "/dev/crypto not present, not enabling devcrypto engine\n");
+    if ((cfd = open("/dev/crypto", O_RDWR, 0)) < 0) {
+        fprintf(stderr, "Could not open /dev/crypto: %s\n", strerror(errno));
         return;
     }
 
     prepare_cipher_methods();
-#if defined(COP_FLAG_UPDATE) && defined(COP_FLAG_FINAL)
+#ifdef IMPLEMENT_DIGEST
     prepare_digest_methods();
 #endif
 
-    if ((e = ENGINE_new()) == NULL)
+    if ((e = ENGINE_new()) == NULL
+        || !ENGINE_set_destroy_function(e, devcrypto_unload)) {
+        ENGINE_free(e);
+        /*
+         * We know that devcrypto_unload() won't be called when one of the
+         * above two calls have failed, so we close cfd explicitly here to
+         * avoid leaking resources.
+         */
+        close(cfd);
         return;
+    }
 
     if (!ENGINE_set_id(e, "devcrypto")
         || !ENGINE_set_name(e, "/dev/crypto engine")
-        || !ENGINE_set_destroy_function(e, devcrypto_unload)
 
 /*
  * Asymmetric ciphers aren't well supported with /dev/crypto.  Among the BSD
@@ -664,7 +674,7 @@ void engine_load_devcrypto_int()
 # endif
 #endif
         || !ENGINE_set_ciphers(e, devcrypto_ciphers)
-#if defined(COP_FLAG_UPDATE) && defined(COP_FLAG_FINAL)
+#ifdef IMPLEMENT_DIGEST
         || !ENGINE_set_digests(e, devcrypto_digests)
 #endif
         ) {


More information about the openssl-commits mailing list