[RFC,OPNESSL-ODP,2/2] Adding Async ODP engine for AES cipher

---
 engine/eng_odp.c | 375 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 375 insertions(+)
 create mode 100644 engine/eng_odp.c

On Fri, Jun 3, 2016 at 12:59 PM, Nikhil Agarwal <nikhil.agarwal@linaro.org>
wrote:

> ---
>  engine/eng_odp.c | 375
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 375 insertions(+)
>  create mode 100644 engine/eng_odp.c
>
> diff --git a/engine/eng_odp.c b/engine/eng_odp.c
> new file mode 100644
> index 0000000..3340649
> --- /dev/null
> +++ b/engine/eng_odp.c
> @@ -0,0 +1,375 @@
> +
> +#include <stdio.h>
> +#include <string.h>
> +
> +#include <openssl/engine.h>
> +#include <openssl/aes.h>
> +#include <openssl/evp.h>
> +#include <openssl/async.h>
> +#include <openssl/bn.h>
> +#include <openssl/crypto.h>
> +#include <openssl/ssl.h>
> +#include <openssl/modes.h>
> +#include <odp.h>
> +#include <pthread.h>
> +#include <unistd.h>
> +
> +#define DUMMY_CHAR 'X'
> +odp_pool_t pool;
> +int num_queue = 8;
> +odp_queue_t out_queue[8];
> +OSSL_ASYNC_FD pipefds[2] = {0, 0};
> +
> +/* Engine Id and Name */
> +static const char *engine_odp_id = "libsslodp";
> +static const char *engine_odp_name = "ODP based engine";
> +
> +/* Engine Lifetime functions */
> +static int ossl_odp_destroy(ENGINE *e);
> +static int ossl_odp_init(ENGINE *e);
> +static int ossl_odp_finish(ENGINE *e);
> +
> +/* Set up digests. Just SHA1 for now */
> +static int ossl_odp_digests(ENGINE *e, const EVP_MD **digest,
> +               const int **nids, int nid);
> +
> +/*
> + * Holds the EVP_MD object for sha1 in this engine. Set up once only
> during
> + * engine bind and can then be reused many times.
> + */
> +static void destroy_digests(void)
> +{
> +       /*Nothing for now*/
> +}
> +
> +static int ossl_odp_digest_nids(const int **nids)
> +{
> +       int digest_nids[2] = { 0, 0 };
> +
> +       *nids = digest_nids;
> +       return 1;
> +}
> +
> +/* AES */
> +
> +static int ossl_odp_aes128_init_key(EVP_CIPHER_CTX *ctx,
> +               const unsigned char *key, const unsigned char *iv, int
> enc);
> +static int ossl_odp_aes128_cbc_cipher(EVP_CIPHER_CTX *ctx, unsigned char
> *out,
> +               const unsigned char *in, size_t inl);
> +static int ossl_odp_aes128_cbc_cleanup(EVP_CIPHER_CTX *ctx);
> +
> +struct ossl_odp_ctx {
> +       odp_crypto_session_t session;
> +};
> +
> +/*
> + * Holds the EVP_CIPHER object for aes_128_cbc in this engine. Set up
> once only
> + * during engine bind and can then be reused many times.
> + */
> +static EVP_CIPHER *_hidden_aes_128_cbc;
> +static const EVP_CIPHER *ossl_odp_aes_128_cbc(void)
> +{
> +       return _hidden_aes_128_cbc;
> +}
> +
> +static void destroy_ciphers(void)
> +{
> +       EVP_CIPHER_meth_free(_hidden_aes_128_cbc);
> +       _hidden_aes_128_cbc = NULL;
> +}
> +
> +static int ossl_odp_ciphers(ENGINE *e, const EVP_CIPHER **cipher,
> +               const int **nids, int nid);
> +
> +static int ossl_odp_cipher_nids[] = {
> +       NID_aes_128_cbc,
> +       0
> +};
> +
> +static int bind_odp(ENGINE *e)
> +{
> +       if (!ENGINE_set_id(e, engine_odp_id)
> +                       || !ENGINE_set_name(e, engine_odp_name)
> +                       || !ENGINE_set_ciphers(e, ossl_odp_ciphers)
> +                       || !ENGINE_set_destroy_function(e,
> ossl_odp_destroy)
> +                       || !ENGINE_set_init_function(e, ossl_odp_init)
> +                       || !ENGINE_set_finish_function(e,
> ossl_odp_finish)) {
> +               return 0;
> +       }
> +
> +       _hidden_aes_128_cbc = EVP_CIPHER_meth_new(NID_aes_128_cbc,
> +                       16 /* block size */,
> +                       16 /* key len */);
> +       if (_hidden_aes_128_cbc == NULL
> +               || !EVP_CIPHER_meth_set_iv_length(_hidden_aes_128_cbc, 16)
> +               || !EVP_CIPHER_meth_set_flags(_hidden_aes_128_cbc,
> +                       EVP_CIPH_FLAG_DEFAULT_ASN1
> +                       | EVP_CIPH_CBC_MODE)
> +               || !EVP_CIPHER_meth_set_init(_hidden_aes_128_cbc,
> +                       ossl_odp_aes128_init_key)
> +               || !EVP_CIPHER_meth_set_do_cipher(_hidden_aes_128_cbc,
> +                       ossl_odp_aes128_cbc_cipher)
> +               || !EVP_CIPHER_meth_set_cleanup(_hidden_aes_128_cbc,
> +                       ossl_odp_aes128_cbc_cleanup)
> +               || !EVP_CIPHER_meth_set_impl_ctx_size(_hidden_aes_128_cbc,
> +                               sizeof(struct ossl_odp_ctx))) {
> +               EVP_CIPHER_meth_free(_hidden_aes_128_cbc);
> +               _hidden_aes_128_cbc = NULL;
> +       }
> +       return 1;
> +}
> +
> +static int bind_helper(ENGINE *e, const char *id)
> +{
> +       if (id && (strcmp(id, engine_odp_id) != 0))
> +               return 0;
> +       if (!bind_odp(e))
> +               return 0;
> +       return 1;
> +}
> +
> +IMPLEMENT_DYNAMIC_CHECK_FN()
> +IMPLEMENT_DYNAMIC_BIND_FN(bind_helper)
> +
> +static int ossl_odp_init(ENGINE *e)
> +{
> +       odp_queue_param_t qparam;
> +       odp_pool_param_t params;
> +       int i;
> +       char buf = DUMMY_CHAR;
> +
> +       if (0 != odp_init_global(NULL, NULL)) {
>

odp_init_global() now takes an instance output variable as it's first
argument. You need to save that for use in both odp_init_local() as well as
odp_term_global()


> +               printf("error: odp_init_global() failed.\n");
>

Is direct use of printf() customary in OpenSSL extensions or does OpenSSL
have its own logging function?


> +               return -1;
> +       }
>

This raises an interesting question of how this works with an application
that is already using ODP but has portions of itself that use OpenSSL.
Does the OpenSSL stuff operate as a separate process and thus have its own
ODP instance? This seems to intersect with a lot of the questions
Christophe has been raising that we've been discussing.  Do we need an
odp_init_info() API that would allow code to query whether an ODP instance
is already active?


> +       if (0 != odp_init_local(ODP_THREAD_WORKER)) {
>

odp_init_local() now takes an instance ID as its first argument.


> +               printf("error: odp_init_local() failed.\n");
> +               return -1;
> +       }
> +
> +       memset(&params, 0, sizeof(params));
>

This should be odp_pool_param_init(&params);


> +       params.pkt.seg_len = 20480;
> +       params.pkt.len = 20480;
> +       params.pkt.num = 4096;
> +       params.type = ODP_POOL_PACKET;
> +
> +       pool = odp_pool_create("packet_pool", &params);
>

Since this is an ODP OpenSSL service thread, perhaps a better prefix for
the pool name would be appropriate like "openssl-odp packet pool" or some
such?


> +
> +       if (ODP_POOL_INVALID == pool) {
> +               printf("Packet pool creation failed.\n");
> +               return -1;
>

You should clean up by issuing odp_term_local() and odp_term_global() calls
here rather than leave the ODP environment dangling.


> +       }
> +       odp_queue_param_init(&qparam);
> +       qparam.sched.prio = ODP_SCHED_PRIO_HIGHEST;
> +       qparam.sched.sync = ODP_SCHED_SYNC_ATOMIC;
> +       qparam.sched.group = ODP_SCHED_GROUP_ALL;
> +       for (i = 0; i < num_queue; i++) {
> +               out_queue[i] = odp_queue_create("crypto-out",
> +                               ODP_QUEUE_TYPE_SCHED, &qparam);
>

If you're going to assign names to these queues, they should be unique. So
you can either leave the name NULL to create anonymous queues, or should
sprintf a numerical suffix to the names to they're unique.  Also, same
comment about using some sort of openssl-odp prefix on these names as for
the pool create.


> +               if (ODP_QUEUE_INVALID == out_queue[i]) {
> +                       printf("Crypto outq creation failed.\n");
> +                       return -1;
>

No, you have to unwind by doing odp_queue_destroy() calls for any queues
you might have previously created, destroy the pool, and properly terminate
ODP before returning here.  A general cleanup_exit handler seems called for
here to consolidate this process in one place.


> +               }
> +       }
> +       if (pipe(pipefds) != 0)
> +               return;
>

Again, need a cleanup_exit here.  Also this won't compile since a return
value is expected here.  I assume this is another return -1?

+
> +       write(pipefds[1], &buf, 1);
> +       return 1;
> +}
> +
> +
> +static int ossl_odp_finish(ENGINE *e)
> +{
> +       int i;
> +
> +       for (i = 0; i < num_queue; i++)
> +               odp_queue_destroy(out_queue[i]);
> +
> +       odp_pool_destroy(pool);
> +       odp_term_local();
> +       odp_term_global();
>

odp_term_global() requires an instance ID as an argument here.


> +       return 1;
> +}
> +
> +
> +static int ossl_odp_destroy(ENGINE *e)
> +{
> +       destroy_ciphers();
> +       return 1;
> +}
> +
> +static int ossl_odp_ciphers(ENGINE *e, const EVP_CIPHER **cipher,
> +               const int **nids, int nid)
> +{
> +       int ok = 1;
> +
> +       if (cipher == NULL) {
> +               /* We are returning a list of supported nids */
> +               *nids = ossl_odp_cipher_nids;
> +               return (sizeof(ossl_odp_cipher_nids) -
> +                               1) / sizeof(ossl_odp_cipher_nids[0]);
> +       }
> +       /* We are being asked for a specific cipher */
> +       switch (nid) {
> +       case NID_aes_128_cbc:
> +               *cipher = ossl_odp_aes_128_cbc();
> +               break;
> +       default:
> +               ok = 0;
> +               *cipher = NULL;
> +               break;
> +       }
> +       return ok;
> +}
> +
> +static void wait_cleanup(ASYNC_WAIT_CTX *ctx, const void *key,
> +               OSSL_ASYNC_FD readfd, void *pvwritefd)
> +{
> +       /*Nothing todo for now*/
> +}
> +
> +/* Cipher helper functions */
> +static int ossl_odp_cipher_init_key_helper(EVP_CIPHER_CTX *ctx,
> +               const unsigned char *key,
> +               const unsigned char *iv, int enc,
> +               const EVP_CIPHER *cipher)
> +{
> +       int ret;
> +       enum odp_crypto_ses_create_err status;
> +       odp_crypto_session_params_t ses_params;
> +       odp_crypto_key_t cipher_key = { .data = NULL, .length = 0 },
> +                        auth_key = { .data = NULL, .length = 0 };
> +       odp_crypto_iv_t ses_iv;
> +       static int next;
> +
> +       if (next == num_queue)
> +               next = 0;
> +
> +
> +       struct ossl_odp_ctx *odp_ctx =
> +               (struct ossl_odp_ctx *)EVP_CIPHER_CTX_get_cipher_data(ctx);
> +
> +       /* Create a crypto session */
> +       memset(&ses_params, 0, sizeof(ses_params));
>

We should make a note that for Tiger Moth we should add an
odp_crypto_session_param_init() API for consistency with other parameter
initializations.


> +       ses_params.op = (enc == 1) ?
> ODP_CRYPTO_OP_ENCODE:ODP_CRYPTO_OP_DECODE;
> +       ses_params.auth_cipher_text = false;
> +       ses_params.pref_mode = ODP_CRYPTO_ASYNC;
> +       ses_params.cipher_alg = ODP_CIPHER_ALG_AES128_CBC;
> +       ses_params.auth_alg = ODP_AUTH_ALG_NULL;
> +       cipher_key.data = (unsigned char *)key;
> +       cipher_key.length = 16;
> +       ses_iv.data = (unsigned char *)iv;
> +       ses_iv.length = 16;
> +       ses_params.cipher_key = cipher_key;
> +       ses_params.iv = ses_iv;
> +       ses_params.auth_key = auth_key;
> +       ses_params.compl_queue = out_queue[next];
> +       ses_params.output_pool = pool;
> +
> +       ret = odp_crypto_session_create(&ses_params,
> +                       &(odp_ctx->session), &status);
> +
> +       next++;
> +       if (ODP_CRYPTO_SES_CREATE_ERR_NONE != status)
> +               return 0;
> +       return ret;
>

This doesn't make sense. If the status from the create request is ERR_NONE
than odp_crypto_session_create() is going to return 0, no?  So this looks
like you'll return 0 no matter what the result of the create() call is.


> +}
> +
> +static int ossl_odp_cipher_helper(EVP_CIPHER_CTX *ctx, unsigned char *out,
> +               const unsigned char *in, size_t inl,
> +               const EVP_CIPHER *cipher)
> +{
> +       int ret = 1;
> +       odp_crypto_op_params_t params;
> +       odp_packet_t pkt;
> +       odp_bool_t posted;
> +       odp_event_t event;
> +       odp_crypto_compl_t compl_event;
> +       ASYNC_WAIT_CTX *waitctx;
> +       OSSL_ASYNC_FD *fd;
> +       char completed = 0;
> +       odp_crypto_op_result_t result;
> +       ASYNC_JOB *job = ASYNC_get_current_job();
> +       struct ossl_odp_ctx *odp_ctx =
> +               (struct ossl_odp_ctx *)EVP_CIPHER_CTX_get_cipher_data(ctx);
> +
> +       if (job == NULL) {
> +               printf("\nSYNC operation not yet supported\n");
> +               return;
> +       }
> +
> +       waitctx = ASYNC_get_wait_ctx(job);
> +
> +       if (!ASYNC_WAIT_CTX_get_fd(waitctx, engine_odp_id, &pipefds[0],
> +                               (void **)&fd)) {
> +               ASYNC_WAIT_CTX_set_wait_fd(waitctx, engine_odp_id,
> pipefds[0],
> +                               NULL, wait_cleanup);
> +       }
> +       pkt = odp_packet_alloc(pool, inl);
>

You need to check to be sure this alloc() request succeeded before
referencing it.


> +       memcpy(odp_packet_data(pkt), in, inl);
>

This is dangerous. Use odp_packet_copy_from_mem() to copy raw data into an
ODP packet. That way any segmentation is handled for you automatically.
And you need to check the return code from this to ensure it was successful
before proceeding.


> +       memset(&params, 0, sizeof(params));
>

Again, for Tiger Moth we should add an odp_crypto_op_param_init() API for
consistency here.


> +       params.ctx = NULL;
> +       params.session = odp_ctx->session;
> +       params.pkt = pkt;
> +       params.ctx = &completed;
> +       params.out_pkt = pkt;
> +       params.cipher_range.offset = 0;
> +       params.cipher_range.length = inl;
> +       params.override_iv_ptr = NULL;
> +       if (odp_crypto_operation(&params,
> +                               &posted,
> +                               &result)) {
> +               abort();
> +       }
> +       if (posted) {
> +               while (!completed) {
>

How does this loop ever exit?  completed is a local variable set to 0 and I
don't see where it is changed within the scope of this loop.


> +                       ASYNC_pause_job();
> +                       /* Poll completion queue for results */
> +                       event = odp_schedule(NULL, ODP_SCHED_NO_WAIT);
> +
> +                       while (event != ODP_EVENT_INVALID) {
> +                               compl_event =
> +                                       odp_crypto_compl_from_event(event);
> +                               odp_crypto_compl_result(compl_event,
> &result);
> +                               odp_crypto_compl_free(compl_event);
> +                               *((char *)result.ctx) = 1;
> +
> +                               if (!result.ok)
> +                                       return;
>

This function wants an int return value so this won't compile. Not clear
what the intent is here.


> +                               event = odp_schedule(NULL,
> ODP_SCHED_NO_WAIT);
> +                       }
> +               }
> +       }
> +       memcpy(out, odp_packet_data(pkt), inl);
>

Use odp_packet_copy_to_mem() to copy data from an ODP packet back into your
own buffer memory.


> +       odp_packet_free(pkt);
> +       return ret;
> +}
> +
> +/*
> + * AES128 CBC Implementation
> + */
> +
> +static int ossl_odp_aes128_init_key(EVP_CIPHER_CTX *ctx,
> +               const unsigned char *key, const unsigned char *iv, int enc)
> +{
> +       return ossl_odp_cipher_init_key_helper(ctx, key, iv,
> +                                               enc, EVP_aes_128_cbc());
> +}
> +
> +static int ossl_odp_aes128_cbc_cipher(EVP_CIPHER_CTX *ctx, unsigned char
> *out,
> +               const unsigned char *in, size_t inl)
> +{
> +       return ossl_odp_cipher_helper(ctx, out, in, inl,
> EVP_aes_128_cbc());
> +}
> +
> +static int ossl_odp_aes128_cbc_cleanup(EVP_CIPHER_CTX *ctx)
> +{
> +       struct ossl_odp_ctx *odp_ctx =
> +               (struct ossl_odp_ctx *)EVP_CIPHER_CTX_get_cipher_data(ctx);
> +
> +       odp_crypto_session_destroy(odp_ctx->session);
> +
> +       return 1;
> +}
> +
> --
> 2.8.2
>
> _______________________________________________
> lng-odp mailing list
> lng-odp@lists.linaro.org
> https://lists.linaro.org/mailman/listinfo/lng-odp
>