On 6/24/2017 11:35 AM, Emmanuel Deloget wrote:> Hello Douglas, > > On Fri, Jun 23, 2017 at 9:16 PM, Douglas E Engert <deengert at gmail.com <mailto:deengert at gmail.com>> wrote: > > OpenSC has taken a different approach to OpenSSL-1.1. Rather then writing > > a shim for OpenSSL-1.1, the OpenSC code has been converted to > > the OpenSSL-1.1 API and a sc-ossl-compat.h" file consisting of defines and > > macros was written to support older versions of OpenSSL and Libressl. > > > > https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/sc-ossl-compat.h > > > > The nice part of this approach is when using OpenSSL-1.1 sc-ossl-compat.h > > does not do anything. It sole purpose to provide calls to the older APIs > > that are not going to change and eventually the sc-ossl-compat.h could be > > removed. > > > > Only the OpenSSL routines used by OpenSC have added to sc-ossl-compat.h > > but others defines and macro could be added.There are a few utilities that > > use still use a few #ifdef's during initialization. > > This might be because I'm kind of a failure when I try to speak English but this is what I assumed to be a shim :) Of course, I might be wrong again :)Yes it is a shim. https://en.wikipedia.org/wiki/Shim_(computing) "Shims can be used to support an old API in a newer environment, or a new API in an older environment." My reading of the discussion was to keep the older API to avoid changing the OpenSSH code so a shim would be to support an "old API in a newer environment(OpenSSL-1.1)". What we did was to convert OpenSC to the OpenSSL-1.1 API the write the shim for "new API in an older environment". Looking at your openssl_compat.h it looks like you have done something similar. With OpenSC we only added the routines and defines we needed in OpenSC.> > This is very similar to the approach taken by Kurt in his patch and to the work I did for OpenVPN > ? [1]? > . And I also think it's the way to go since it will allow to diss older versions of openssl and/or libressl with minimal change in the Code That Matters while not being terribly difficult to maintain. > According to Kurt's patch, such a compat file for openssh would clock at roughly 500 lines of nearly trivial code (and I insist on this fact: code is quite trivial. The most complicated function would > be this one : > > ?----8<-----? > static int EVP_MD_CTX_reset(EVP_MD_CTX *ctx) > { > ?? > if (ctx == NULL) > ?? > return 1; > ?? > /* > ?? > * Don't assume ctx->md_data was cleaned in EVP_Digest_Final, because > ?? > * sometimes only copies of the context are ever finalised. > ?? > */ > ?? > if (ctx->digest && ctx->digest->cleanup > ?? > && !EVP_MD_CTX_test_flags(ctx, EVP_MD_CTX_FLAG_CLEANED)) > ?? > ctx->digest->cleanup(ctx); > ?? > if (ctx->digest && ctx->digest->ctx_size && ctx->md_data > ?? > && !EVP_MD_CTX_test_flags(ctx, EVP_MD_CTX_FLAG_REUSE)) { > ?? > OPENSSL_clear_free(ctx->md_data, ctx->digest->ctx_size); > ?? > } > ?? > EVP_PKEY_CTX_free(ctx->pctx); > #ifndef OPENSSL_NO_ENGINE > ?? > ENGINE_finish(ctx->engine); > #endif > ?? > OPENSSL_cleanse(ctx, sizeof(*ctx)); > > ?? > return 1; > } > ?---->8----- > > > Other functions are getter and setters such as : > > ?----8<-----? > void DH_get0_key(const DH *dh, const BIGNUM **pub_key, const BIGNUM **priv_key) > { > if (pub_key != NULL) > *pub_key = dh->pub_key; > if (priv_key != NULL) > *priv_key = dh->priv_key; > } > > int DH_set0_key(DH *dh, BIGNUM *pub_key, BIGNUM *priv_key) > { > /* If the field pub_key in dh is NULL, the corresponding input > * parameters MUST be non-NULL. The priv_key field may > * be left NULL. > */ > if (dh->pub_key == NULL && pub_key == NULL) > return 0; > > if (pub_key != NULL) { > BN_free(dh->pub_key); > dh->pub_key = pub_key; > } > if (priv_key != NULL) { > BN_free(dh->priv_key); > dh->priv_key = priv_key; > } > > return 1; > } > > ?int RSA_bits(const RSA *r) > { > return (BN_num_bits(r->n)); > } > ?---->8----- > > > There are some simple security?-related functions such as OPENSSL_zalloc() or OPENSSL_clean_free() but these functions are quite simple too (and the reasoning behind them is, I assume, well known). > > > BR, > > -- Emmanuel Deloget > > ?[1] https://github.com/OpenVPN/openvpn/blob/master/src/openvpn/openssl_compat.h > ?-- Douglas E. Engert <DEEngert at gmail.com>
On 2017-06-24, Douglas E Engert <deengert at gmail.com> wrote:> My reading of the discussion was to keep the older API to avoid changing the > OpenSSH code so a shim would be to support an "old API in a newer environment(OpenSSL-1.1)".Exactly the other way round. Since it seems 1.0.x is going to be around for some years yet until RHEL7 is end-of-life, there's clearly a need for downstream projects to work with multiple OpenSSL versions. Many projects have been handling the conversion with #ifdef'd compatibility code, which is hard to read and hard to test. (As a packager for an OS using LibreSSL this is very obvious as these almost all check OPENSSL_VERSION_NUMBER and need changes for LibreSSL). It's obvious why people have been taking this approach - it has long been the usual approach to deal with new functions being introduced in the OpenSSL API. It's not quite so messy when using it to disable certain features not yet supported, but using it for whole parallel codepaths gets a lot more confusing (and diffs get rather hard to review). Additionally it seems like some of this code (especially for smaller projects) has come from users not especially familiar with either the downstream code *or* OpenSSL, whose main motivation is to unbreak build for OS that are using 1.1, and are less interested in 1.0.x.> What we did was to convert OpenSC to the OpenSSL-1.1 API the write the shim > for "new API in an older environment". > > Looking at your openssl_compat.h it looks like you have done something similar. > With OpenSC we only added the routines and defines we needed in OpenSC.Using shims like this and writing to the new API seems exactly the right approach, and is similar to what OpenSSH-portable has done successfully for years. But isn't it silly that each project wanting to support both old+new OpenSSL has to maintain their own shim? This would all be a lot simpler for downstream projects if there was a single trusted source for such a shim, coming from people familiar with the OpenSSL code. Then those downstream projects (who know their *own* code well) can treat it as a simple API change and not have to worry about compatibility.
On 6/25/2017 4:47 AM, Stuart Henderson wrote:> On 2017-06-24, Douglas E Engert <deengert at gmail.com> wrote: >> My reading of the discussion was to keep the older API to avoid changing the >> OpenSSH code so a shim would be to support an "old API in a newer environment(OpenSSL-1.1)". > > Exactly the other way round.Upon further review, I stand corrected. OpenSC's approach [1] is similar to the examples in this thread.> > Since it seems 1.0.x is going to be around for some years yet until > RHEL7 is end-of-life, there's clearly a need for downstream projects > to work with multiple OpenSSL versions. > > Many projects have been handling the conversion with #ifdef'd > compatibility code, which is hard to read and hard to test. (As a > packager for an OS using LibreSSL this is very obvious as these almost > all check OPENSSL_VERSION_NUMBER and need changes for LibreSSL). >> It's obvious why people have been taking this approach - it has long > been the usual approach to deal with new functions being introduced > in the OpenSSL API. It's not quite so messy when using it to disable > certain features not yet supported, but using it for whole parallel > codepaths gets a lot more confusing (and diffs get rather hard to > review). > > Additionally it seems like some of this code (especially for smaller > projects) has come from users not especially familiar with either the > downstream code *or* OpenSSL, whose main motivation is to unbreak build > for OS that are using 1.1, and are less interested in 1.0.x. > >> What we did was to convert OpenSC to the OpenSSL-1.1 API the write the shim >> for "new API in an older environment". >> >> Looking at your openssl_compat.h it looks like you have done something similar. >> With OpenSC we only added the routines and defines we needed in OpenSC.Yes, supports multiple OpenSSL and LibreSSL.> > Using shims like this and writing to the new API seems exactly the right > approach, and is similar to what OpenSSH-portable has done successfully > for years. But isn't it silly that each project wanting to support both > old+new OpenSSL has to maintain their own shim?Yes, but to OpenSSL's credit, they have been moving in this direction, in previous releases, with adding macros or routines to do move away from direct access to what are now hidden structures. But they have now forced the use of these new routines and additional hidden structures in 1.1.> > This would all be a lot simpler for downstream projects if there was a > single trusted source for such a shim, coming from people familiar with > the OpenSSL code. Then those downstream projects (who know their *own* > code well) can treat it as a simple API change and not have to worry > about compatibility. > > > _______________________________________________ > openssh-unix-dev mailing list > openssh-unix-dev at mindrot.org > https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev >[1] https://github.com/OpenSC/OpenSC/blob/master/src/libopensc/sc-ossl-compat.h -- Douglas E. Engert <DEEngert at gmail.com>