Commit aa01b82c authored by Richard Levitte's avatar Richard Levitte
Browse files

If an engine comes up explicitely, it must also come down explicitely 



In apps/apps.c, one can set up an engine with setup_engine().
However, we freed the structural reference immediately, which means
that for engines that don't already have a structural reference
somewhere else (because it has registered at least one cipher or digest
algorithm method, and therefore gets a functional reference through the
ENGINE_set_default() call), we end up returning an invalid reference.

Instead, the function release_engine() is added, and called at the end
of the routines that call setup_engine().

Originally, the ENGINE API wasn't designed for this to happen, an
engine had to register at least one algorithm method, and was
especially expected to register the algorithms corresponding to the
key types that could be stored and hidden in hardware.  However, it
turns out that some engines will not register those algorithms with
the ENGINE_set_{algo}, ENGINE_set_cipher or ENGINE_set_digest
functions, as they only want the methods to be used for keys, not as
general crypto accelerator methods.  That may cause ENGINE_set_default()
to do nothing, and no functional reference is therefore made, leading
to a premature deallocation of the engine and it thereby becoming
unavailable when trying to fetch a key.

Reviewed-by: default avatarRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/1644)
parent 10e60f26

apps/apps.c

+6 −2
Original line number Diff line number Diff line
@@ -1564,12 +1564,16 @@ ENGINE *setup_engine(BIO *err, const char *engine, int debug) 
        }



        BIO_printf(err, "engine \"%s\" set.\n", ENGINE_get_id(e));

    }

    return e;

}



void release_engine(ENGINE *e)

{

    if (e != NULL)

        /* Free our "structural" reference. */

        ENGINE_free(e);

}

    return e;

}

#endif



int load_config(BIO *err, CONF *cnf)

apps/apps.h

+1 −0
Original line number Diff line number Diff line
@@ -261,6 +261,7 @@ STACK_OF(X509_CRL) *load_crls(BIO *err, const char *file, int format, 
X509_STORE *setup_verify(BIO *bp, char *CAfile, char *CApath);

# ifndef OPENSSL_NO_ENGINE

ENGINE *setup_engine(BIO *err, const char *engine, int debug);

void release_engine(ENGINE *e);

# endif



# ifndef OPENSSL_NO_OCSP

apps/ca.c

+4 −0
Original line number Diff line number Diff line
@@ -1485,6 +1485,10 @@ int MAIN(int argc, char **argv) 
    X509_CRL_free(crl);

    NCONF_free(conf);

    NCONF_free(extconf);

#ifndef OPENSSL_NO_ENGINE

    if (e != NULL)

        release_engine(e);

#endif

    OBJ_cleanup();

    apps_shutdown();

    OPENSSL_EXIT(ret);

apps/cms.c

+4 −0
Original line number Diff line number Diff line
@@ -1170,6 +1170,10 @@ int MAIN(int argc, char **argv) 
    EVP_PKEY_free(key);

    CMS_ContentInfo_free(cms);

    CMS_ContentInfo_free(rcms);

#ifndef OPENSSL_NO_ENGINE

    if (e != NULL)

        release_engine(e);

#endif

    BIO_free(rctin);

    BIO_free(in);

    BIO_free(indata);

apps/dgst.c

+4 −0
Original line number Diff line number Diff line
@@ -537,6 +537,10 @@ int MAIN(int argc, char **argv) 
        OPENSSL_free(sigbuf);

    if (bmd != NULL)

        BIO_free(bmd);

#ifndef OPENSSL_NO_ENGINE

    if (e != NULL)

        release_engine(e);

#endif

    apps_shutdown();

    OPENSSL_EXIT(err);

}