I wonder, how does that change benefit the openssl command?

@levitte, basically all openssl apps that (now or in the future) use more than one private key and thus potentially more than one set of crypto methods or storage types can benefit. This is the case for s_server and the proposed cmp app. Also the ocsp app could benefit as soon as engine support is enabled for it, because it also has the options to use two keys: -signkey and -rkey.

I could easily have missed something after a long and hectic day, but I don't see how this helps. The OpenSSL engine framework doesn't distinguish between different engines that support the same algorithm or between an engine and the built in implementation -- if I remember the code sequences correctly, it will choose the default then the first engine that says: "yes, I do that." To avoid this, the calling application needs to specify which engine it wants to use. I don't think there is a way to say to use the built in software implementation. That is, changing the default isn't going to provide the flexibility you want by itself.

Would some manner of per engine capability list be more appropriate? That is, each engine specifies more precisely what it does and the application can request exactly what it wants.

I think this highlights a limitation in the command line app and engine. If you specify an engine on the command line an attempt is then made to use it for all crypto algorithms it supports. Sometimes you don't want that and just want to use the ENGINE to load a private key. You might want to specify different keys in different engines. An engine might implement a crypto algorithm that is only for use with its keys and not general purpose crypto: setting ENGINE_FLAGS_NO_REGISTER_ALL in the engine partly helps but is has limitations.

Hmmm, if an engine is used solely for loading keys, said engines should be set to add no default algos... so ok, that's a point.

Stephen has given a very good summary of the main problem: once the methods of an engine have been set as the default, the superseded built-in methods cannot be used any more for key material not related to that engine.
Moreover, one cannot use multiple engines (unless they happen to provide the same set of methods, i.e., their only distinction could be the key storage location/mechanism they provide).

While the idea given by Pauli might to provide even more flexibility, the very simple generalization I propose would already be a major gain. In particular, it allows to load and use keys (simultaneously in the same application run) from any number of different engines and from software/file storage. It also allows to specify exactly which methods, if any, of which engine are taken as the default.

I still don't understand how this fixes the problem. I don't see why not having a default engine is the sole problem. What am I missing?

When there are two (or more) engines that implement the same algorithm, the code that decides if an engine should be used or not will always pick the same engine. It only knows about the algorithm wanted and cannot distinguish based on the data being processed. Likewise, if any engine implements an algorithm, it will always be chosen in preference to the built in implementation. Setting the default forces the result but it doesn't change the fact that a predetermined engine will be used.

That's not the whole story. Some calls have an optional engine argument. If this is specified, then yes you can choose which specific engine you want. The internal software implementation is unavailable. Additionally, some operations carry the engine used with them (EVP_PKEY) and this is chosen if possible, overriding the engine argument if specified. Again, the internal implementation cannot be used.

How does disabling the default engine help? An application needs to specify the engine used for each operation if it wants to use more than one and if this is done the default is ignored.

I think that in the end, what we're talking about here is a conflict between two quite separate engine usage:

1. the engine as an accelerator of calculations.
2. the engine as a store of keys.

The trouble is that when both usages are activated, the same functions are used for them, since there is no distinction at a higher level between, say, RSA functions for keys with all parameters exposed and for keys with private parameters hidden. In the end, it requires that the different engines must be able to distinguish between "normal" keys and keys hidden in a device and act accordingly.

By turning off the installation of defaults, we essentially lose acceleration. It can, however, be argued that this particular usage has lost its importance these days, compared to when the ENGINE API was first designed, back in around 2000. These days, it seems that the more important usages are to store keys and to bring new algorithms that aren't build into libcrypto...

Thanks for the explanation, I understand why you're trying to do this.

@levitte I'm not convinced that the engine as an accelerator is gone. Small embedded CPUs often have cryptographic accelerations. Intel CPUs have the RDRAND engine which is an accelerator but only tangentially related to this.

Some of the code paths upon seeing a NULL engine reference try to locate a suitable engine. Other's don't. EVP_CipherInit_ex will always go looking for a suitable engine, default or not. EVP_PKEY_CTX_new will go looking if no engine is already defined or passed in.

Using EVP_PKEY_CTX_dup after first creating a context with the correct engine might be a viable approach. I've not tracked down if copying a NULL engine field will later get replaced or not. Gaining access to the default crypto algorithms when an engine implements them seems like the major underlying concern.

It seems to me that a special internal engine setting that forces the libcrypto implementations to be used might be a more comprehensive solution.

There are also two additional uses of defaults which are not associated with crypto acceleration.

1. Implementation is only supplied by an ENGINE and not OpenSSL itself. This applies to GOST currently.
2. Policy dictates that a certain "approved" implementation is used. This would apply to FIPS validated algorithms in FIPS mode for example. We don't currently do this but it's a mechanism I'm planning to use for the next validation.

@snhenson is the first related to the default engine? Won't it be discovered regardless as the only engine that implements GOST?

The second is more interesting, it needs to not only be the default but also mandatory -- i.e. no other engine is permitted to supply the same algorithms even if one can.

I keep circling back to capability lists for engines which can be specified by the using application. I should pull my finger out and implement something to see what folks think.

Are there any new thoughts on this topic?
While more fundamental issues with the engine mechanism touched here may take more time to be solved, can we pragmatically do the pretty small change that I proposed, which would already make the engine setup function more generally useful?

I think that the only point where I can see the clash is if you load two engines that both implement the same default algo FOO (and assuming that those engines also implement other desirable stuff that differ from each other or there would be no point loading them both). Am I correct?

Note that the openssl app doesn't really support loading more than one engine, with a few exceptions where you can have a separate engine for key storage (i.e. that shouldn't be made to provide any default algos).

No, as I tried to explain above the issue also arises when using just one engine, namely when loading and using private keys from both the engine and from other source(s), such as a from a file or from memory.

Well, this PR doesn't really change that much, it only moves the exact same flags to another spot and gives the possibility to change them if that would be desired...

I'm curious how you want to move forward... how do you see those flags being changed?

Right, the code change I proposed does not change anything for use with the current apps.
Yet the CMP client app of the CMP extension we are currently preparing will make use of it, and also any other apps that use more than one private key can benefit.

For instance, both x509 and s_server can use more than one private key. I just extended them to take advantage of the given generalization to setup_engine().
Doing so, I rebased the branch, added two checks for potential error conditions that were missing in setup_engine(), and fixed a minor bugs in s_server.c (engine debugging was on unconditionally). I also made sure that all their key format options may include ENGINE and corrected/extended their documentation accordingly.

Thanks a lot @levitte for having a look again at this PR.
I've just rebased it and did the requested fix.

Included two further small fixups, also of the documentation.

@levitte, I've meanwhile fixed all the minor build issues reported by the CI.

Can this now be approved, such that I can merge this into #11470?

Rebased and mostly squashed.
@levitte, is there anything else to do here?

@levitte, hope you can approve this now?
A week ago I fulfilled the small change request you gave.

Pinging again @levitte - this should be a low-hanging fruit.

I've also rebased this PR and made use of the new/extended setup_engine_methods() function in the meanwhile merged app/cmp.c.

Please check the CI status before merging.

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

Merged - thanks!

Wow - note the date of the first commit of this PR:

Mon Aug 28 19:14:47 2017 +0200

@DDvO #11818