diff mbox series

package: openssl: Enable built engines per default

Message ID 20210422065429.971789-1-daniel@dd-wrt.com
State Rejected
Headers show
Series package: openssl: Enable built engines per default | expand

Commit Message

Daniel Danzberger April 22, 2021, 6:54 a.m. UTC
Automatically enable an engine in the openssl.cnf if it has been build.
Before this change, /etc/openssl.cnf had to be edited manually on the
system to enable the engine.

Signed-off-by: Daniel Danzberger <daniel@dd-wrt.com>
---
 package/libs/openssl/Makefile | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Eneas U de Queiroz April 22, 2021, 11:36 p.m. UTC | #1
On Thu, Apr 22, 2021 at 3:55 AM Daniel Danzberger <daniel@dd-wrt.com> wrote:
>
> Automatically enable an engine in the openssl.cnf if it has been build.
> Before this change, /etc/openssl.cnf had to be edited manually on the
> system to enable the engine.
>

> +define Package/libopenssl-conf/enable
> +       $(if $(CONFIG_PACKAGE_libopenssl-$(2)),sed -i s/^\#*$(2)=$(2)/$(2)=$(2)/ $(1)/etc/ssl/openssl.cnf)
> +endef

>  define Package/libopenssl-conf/install
>         $(INSTALL_DIR) $(1)/etc/ssl
>         $(CP) $(PKG_INSTALL_DIR)/etc/ssl/openssl.cnf $(1)/etc/ssl/
> +       $(call Package/libopenssl-conf/enable,$(1),devcrypto)
> +       $(call Package/libopenssl-conf/enable,$(1),afalg)
> +       $(call Package/libopenssl-conf/enable,$(1),padlock)

Hi Daniel

The problem with this is that it will enable the config for all
engines in the bots configuration (all packages =m).  OpenSSL will
stop loading the engines past the point where one of them fails.  It
may do it silently, or it may show an error.  If you run the `openssl
engine` command (no flags or with -c), it will show the error; if you
add the `-t` flag, the error message is gone.  In either case, the
engines configured after the first failed one will not load.  Suppose
that you install the afalg engine, but not devcrypto.  When it loads
the config file, devcrypto comes first, and openssl will fail to find
it; then the afalg engine will not be loaded.

I do like the idea, though. My first thought was to add an install
script to the engine packages.  The problem is that the config file
may have been changed in a way that sed may produce unwanted results.
It can be mitigated by configuring engines in a separate file, so only
that file needs to be changed.  It will have a nice effect, that a
feed-installed engine can configure itself without needing a config
section added to the openssl-conf package.

Another option, which may be the easiest and safest, is to use your
approach, but only uncomment the engines built into the firmware (=y),
and not the ones built as modules.

Cheers,

Eneas
Florian Eckert April 23, 2021, 6:10 a.m. UTC | #2
Hello Daniel,
Hello Eneas,

On 2021-04-23 01:36, Eneas U de Queiroz wrote:
> On Thu, Apr 22, 2021 at 3:55 AM Daniel Danzberger <daniel@dd-wrt.com> 
> wrote:
>> 
>> Automatically enable an engine in the openssl.cnf if it has been 
>> build.
>> Before this change, /etc/openssl.cnf had to be edited manually on the
>> system to enable the engine.
>> 
> 
>> +define Package/libopenssl-conf/enable
>> +       $(if $(CONFIG_PACKAGE_libopenssl-$(2)),sed -i 
>> s/^\#*$(2)=$(2)/$(2)=$(2)/ $(1)/etc/ssl/openssl.cnf)
>> +endef
> 
>>  define Package/libopenssl-conf/install
>>         $(INSTALL_DIR) $(1)/etc/ssl
>>         $(CP) $(PKG_INSTALL_DIR)/etc/ssl/openssl.cnf $(1)/etc/ssl/
>> +       $(call Package/libopenssl-conf/enable,$(1),devcrypto)
>> +       $(call Package/libopenssl-conf/enable,$(1),afalg)
>> +       $(call Package/libopenssl-conf/enable,$(1),padlock)

> 
> I do like the idea, though. My first thought was to add an install
> script to the engine packages.  The problem is that the config file
> may have been changed in a way that sed may produce unwanted results.

How about if we create a uci default script and check on the running 
system what is installed?
And then we could generate a file and add or remove an include line form 
the openssl.cnf [1]?

> Another option, which may be the easiest and safest, is to use your
> approach, but only uncomment the engines built into the firmware (=y),
> and not the ones built as modules.

I think this is not an option, because not all want to have all engines 
installed.

That is my opinion.
Thanks florian

[1] https://github.com/openssl/openssl/blob/master/apps/openssl.cnf#L10
Eneas U de Queiroz April 23, 2021, 12:31 p.m. UTC | #3
On Fri, Apr 23, 2021 at 3:11 AM Florian Eckert <fe@dev.tdt.de> wrote:
> How about if we create a uci default script and check on the running
> system what is installed?
> And then we could generate a file and add or remove an include line form
> the openssl.cnf [1]?

Hi Florian, Daniel

I think we can manage something like that.  The .include option can
load all files in a directory (/etc/ssl/engines.d/), and won't fail if
there aren't any files--the directory itself must exist.  Each engine
package can install its own file there, ahd have a post-install script
that adds a line to an "engines.cnf" file if there isn't any:

add_engine() {
# $1 = engine name (engine .so file without the .so extension)
    grep -q "$1=$1" /etc/ssl/engines.d/engines.cnf && return
    echo "$1=$1" >> /etc/ssl/engines.d/engines.cnf
}

/etc/ssl/engines.d/engines.cnf would start out with just the [engines]
header and some comments explaining its use and warning not to edit
something that would break things.

What do you think?

Cheers,

Eneas
Florian Eckert April 27, 2021, 6:13 a.m. UTC | #4
Hello Eneas

>> How about if we create a uci default script and check on the running
>> system what is installed?
>> And then we could generate a file and add or remove an include line 
>> form
>> the openssl.cnf [1]?
> 
> Hi Florian, Daniel
> 
> I think we can manage something like that.  The .include option can
> load all files in a directory (/etc/ssl/engines.d/), and won't fail if
> there aren't any files--the directory itself must exist.  Each engine
> package can install its own file there, ahd have a post-install script
> that adds a line to an "engines.cnf" file if there isn't any:
> 
> add_engine() {
> # $1 = engine name (engine .so file without the .so extension)
>     grep -q "$1=$1" /etc/ssl/engines.d/engines.cnf && return
>     echo "$1=$1" >> /etc/ssl/engines.d/engines.cnf
> }
> 
> /etc/ssl/engines.d/engines.cnf would start out with just the [engines]
> header and some comments explaining its use and warning not to edit
> something that would break things.
> 
> What do you think?

The plan sounds good :+1:

> 
> Cheers,
> 
> Eneas
Eneas U de Queiroz April 27, 2021, 1:38 p.m. UTC | #5
> >> How about if we create a uci default script and check on the running
> >> system what is installed?
> >> And then we could generate a file and add or remove an include line
> >> form
> >> the openssl.cnf [1]?
> >
> > I think we can manage something like that.  The .include option can
> > load all files in a directory (/etc/ssl/engines.d/), and won't fail if
> > there aren't any files--the directory itself must exist.  Each engine
> > package can install its own file there, ahd have a post-install script
> > that adds a line to an "engines.cnf" file if there isn't any:
> >
> > add_engine() {
> > # $1 = engine name (engine .so file without the .so extension)
> >     grep -q "$1=$1" /etc/ssl/engines.d/engines.cnf && return
> >     echo "$1=$1" >> /etc/ssl/engines.d/engines.cnf
> > }
> >
> > /etc/ssl/engines.d/engines.cnf would start out with just the [engines]
> > header and some comments explaining its use and warning not to edit
> > something that would break things.
> >
> > What do you think?
>
> The plan sounds good :+1:
>
Hi
I'm testing that proposal, and it's almost ready.  I've expanded it to
use uci to enable/disable the engines, but I'm still running tests to
catch corner cases.  I am not able to test the padlock engine, but its
usage should be like devcrypto.  Afalg is more complicated if built
into the library, because openssl does not initialize it like other
builtin engines. There's no way to configure it for general use when
built that way.
Cheers,
Eneas
diff mbox series

Patch

diff --git a/package/libs/openssl/Makefile b/package/libs/openssl/Makefile
index 7ab4c6ccd0..d101ee3aa2 100644
--- a/package/libs/openssl/Makefile
+++ b/package/libs/openssl/Makefile
@@ -375,9 +375,16 @@  define Package/libopenssl/install
 	$(if $(CONFIG_OPENSSL_ENGINE),$(INSTALL_DIR) $(1)/usr/lib/$(ENGINES_DIR))
 endef
 
+define Package/libopenssl-conf/enable
+	$(if $(CONFIG_PACKAGE_libopenssl-$(2)),sed -i s/^\#*$(2)=$(2)/$(2)=$(2)/ $(1)/etc/ssl/openssl.cnf)
+endef
+
 define Package/libopenssl-conf/install
 	$(INSTALL_DIR) $(1)/etc/ssl
 	$(CP) $(PKG_INSTALL_DIR)/etc/ssl/openssl.cnf $(1)/etc/ssl/
+	$(call Package/libopenssl-conf/enable,$(1),devcrypto)
+	$(call Package/libopenssl-conf/enable,$(1),afalg)
+	$(call Package/libopenssl-conf/enable,$(1),padlock)
 endef
 
 define Package/openssl-util/install