Re: [PATCH 3/3] openssl: configure engines with uci
Hi Florian
On Thu, Apr 29, 2021 at 3:44 AM Florian Eckert wrote:
> > $(if
> > CONFIG_OPENSSL_ENGINE_BUILTIN_DEVCRYPTO,/etc/ssl/engines.cnf.d/devcrypto.cnf)
> > $(if
> > CONFIG_OPENSSL_ENGINE_BUILTIN_PADLOCK,/etc/ssl/engines.cnf.d/padlock.cnf)
>
> I think AFALG is missing there?
>
As I mentioned in the earlier thread, builtin AFALG is weird. If I
enable it in openssl.cnf, it will always look for afalg.so, and will
fail. I think it was on oversight, but AFALG is not part of
OPENSSL_INIT_ENGINE_ALL_BUILTIN [1], so it will not be enabled by
default, unless you call
OPENSSL_init_crypto(OPENSSL_INIT_ENGINE_AFALG, NULL). The AFALG
engine does not have any control commands, so configuration is a noop
anyway.
[1]
https://github.com/openssl/openssl/blob/0f077b5fd86e2df0b41608fbd5684fa1a2b58f59/include/openssl/crypto.h.in#L452
> > endef
> > @@ -378,15 +377,17 @@ define Package/libopenssl/install
> > endef
> >
> > define Package/libopenssl-conf/install
> > - $(INSTALL_DIR) $(1)/etc/ssl/engines.cnf.d
> > + $(INSTALL_DIR) $(1)/etc/ssl/engines.cnf.d $(1)/etc/config
> > $(1)/etc/init.d
> > $(CP) $(PKG_INSTALL_DIR)/etc/ssl/openssl.cnf $(1)/etc/ssl/
> > - $(CP) ./files/engines.cnf $(1)/etc/ssl/engines.cnf.d/
> > + $(INSTALL_BIN) ./files/openssl.init $(1)/etc/init.d/openssl
> > + $(SED) 's!%ENGINES_DIR%!/usr/lib/$(ENGINES_DIR)!'
> > $(1)/etc/init.d/openssl
>
> I do not understand that waht you are doing there.
ENGINES_DIR is where the engine so files are stored. It is versioned,
so it is stored in a variable in engine.mk. I'm just setting it in
/etc/init.d/openssl, from ./files/openssl.init#3:
ENGINES_DIR="%ENGINES_DIR%"
The final result, installed in /etc/init.d/openssl#3 is:
ENGINES_DIR="/usr/lib/engines-1.1"
> > $(if $(CONFIG_OPENSSL_ENGINE_BUILTIN_PADLOCK),
> > $(CP) ./files/padlock.cnf $(1)/etc/ssl/engines.cnf.d/
> > - echo padlock=padlock >>
> > $(1)/etc/ssl/engines.cnf.d/engines.cnf)
> > + echo -e "\nconfig engine 'padlock'\n\toption enabled '1'" >>
> > $(1)/etc/config/openssl)
>
> What about AFALG?
The same explanation above fits here.
> > #!/bin/sh
> > +OPENSSL_UCI="{IPKG_INSTROOT}/etc/config/openssl"
> > +if [ -n "{IPKG_INSTROOT}" ] || ! uci -q get openssl.$(1) >/dev/null;
> > then
> > +cat << EOF >> "{OPENSSL_UCI}"
> > +config engine '$(1)'
> > + option enabled '1'
> > +EOF
>
> From my point of view, I think it would be better if we used the uci cli
> command directly here.
> to add the config engine section and enable this engine.
However, uci is not available when the package is installed by the
buildsystem, such as when building the firmware image. That's why I
always check for $IPKG_INSTROOT before calling any commands available
in the target only, as seen above.
>
> > fi
> > +[ -z "{IPKG_INSTROOT}" ] && /etc/init.d/openssl reload
> >endef
> >
> > - define Package/$$(OSSL_ENG_PKG)/prerm :=
> > + define Package/$$(OSSL_ENG_PKG)/postrm :=
> > #!/bin/sh
> > -ENGINES_CNF="{IPKG_INSTROOT}/etc/ssl/engines.cnf.d/engines.cnf"
> > -[ -f "{ENGINES_CNF}" ] || exit 0
> > -sed -e '/$(1)=$(1)/d' -i "{ENGINES_CNF}"
> > +[ -z "{IPKG_INSTROOT}" ] && /etc/init.d/openssl reload
>
> Should we not also remove the uci option on an uninstall wit the uci
> command?
>
I'll change this. My idea was to save the configuration, if user
later reinstall the package. However, since the %ENGINE%.cnf file is
not removed, then openssl will try to enable the removed engine and
fail.
> > +++ b/package/libs/openssl/files/openssl-engines.init
> > @@ -0,0 +1,19 @@
> > +#!/bin/sh /etc/rc.common
>
> Is the init script also switched on at the first boot?
> So that the service runs immediately?
> Not that the service has to be switched on in /etc/rc.d/ first - that
> would be unpleasant.
Yes, it is: file
build_dir/target-arm_cortex-a9+vfpv3-d16_musl_eabi/root-mvebu/etc/rc.d/S13openssl
build_dir/target-arm_cortex-a9+vfpv3-d16_musl_eabi/root-mvebu/etc/rc.d/S13openssl:
symbolic link to ../init.d/openssl
>
> > +
> > +START=05
> > +OSSL_ENGINES_CNF="/etc/ssl/engines.cnf.d/engines.cnf"
> > +
> > +enable_engine() {
> > + echo "$1=$1" >> "${OSSL_ENGINES_CNF}"
>
> The writing happens here on the persistent storage at every boot!
> This is not so good for embedded target with FLASH.
> It would be better to write this to the tmp.
>
This file, along with engines.cnf were left over from a previous idea,
and not are not used. I will take care of them in the v2. The list
is actually saved in /var/etc/ssl/engines.cnf.
> > + config_list_foreach openssl.openssl[0] engines enable_engine
>
> How about the named uci section globals
> config openssl globals
>
This is also part of the leftover file.
I've spotted a missing fix for the postinst/postrm scripts that were
failing when building the final image. I'll send a v2 in a bit.
Thanks for the review!
Eneas
___
Re: [PATCH 3/3] openssl: configure engines with uci
Hello Eneas,
Nice work.
My remarks or suggestions see inline.
define Package/libopenssl-conf/conffiles
/etc/ssl/openssl.cnf
-/etc/ssl/engines.cnf.d/engines.cnf
$(if
CONFIG_OPENSSL_ENGINE_BUILTIN_DEVCRYPTO,/etc/ssl/engines.cnf.d/devcrypto.cnf)
$(if
CONFIG_OPENSSL_ENGINE_BUILTIN_PADLOCK,/etc/ssl/engines.cnf.d/padlock.cnf)
I think AFALG is missing there?
endef
@@ -378,15 +377,17 @@ define Package/libopenssl/install
endef
define Package/libopenssl-conf/install
- $(INSTALL_DIR) $(1)/etc/ssl/engines.cnf.d
+ $(INSTALL_DIR) $(1)/etc/ssl/engines.cnf.d $(1)/etc/config
$(1)/etc/init.d
$(CP) $(PKG_INSTALL_DIR)/etc/ssl/openssl.cnf $(1)/etc/ssl/
- $(CP) ./files/engines.cnf $(1)/etc/ssl/engines.cnf.d/
+ $(INSTALL_BIN) ./files/openssl.init $(1)/etc/init.d/openssl
+ $(SED) 's!%ENGINES_DIR%!/usr/lib/$(ENGINES_DIR)!'
$(1)/etc/init.d/openssl
I do not understand that waht you are doing there.
+ touch $(1)/etc/config/openssl
$(if $(CONFIG_OPENSSL_ENGINE_BUILTIN_DEVCRYPTO),
$(CP) ./files/devcrypto.cnf $(1)/etc/ssl/engines.cnf.d/
- echo devcrypto=devcrypto >>
$(1)/etc/ssl/engines.cnf.d/engines.cnf)
+ echo -e "config engine 'devcrypto'\n\toption enabled '1'" >>
$(1)/etc/config/openssl)
$(if $(CONFIG_OPENSSL_ENGINE_BUILTIN_PADLOCK),
$(CP) ./files/padlock.cnf $(1)/etc/ssl/engines.cnf.d/
- echo padlock=padlock >> $(1)/etc/ssl/engines.cnf.d/engines.cnf)
+ echo -e "\nconfig engine 'padlock'\n\toption enabled '1'" >>
$(1)/etc/config/openssl)
What about AFALG?
endef
define Package/openssl-util/install
diff --git a/package/libs/openssl/engine.mk
b/package/libs/openssl/engine.mk
index 482b5ad5e8..efa46d7214 100644
--- a/package/libs/openssl/engine.mk
+++ b/package/libs/openssl/engine.mk
@@ -23,60 +23,20 @@ define Package/openssl/add-engine
define Package/$$(OSSL_ENG_PKG)/postinst :=
#!/bin/sh
-# 1 == non-empty: suggest reinstall
-error_out() {
-[ "$1" ] && cat <<- EOF
- Reinstalling the libopenssl-conf package may fix this:
+OPENSSL_UCI="{IPKG_INSTROOT}/etc/config/openssl"
- opkg install --force-reinstall libopenssl-conf
- EOF
-cat <<- EOF
+if [ -n "{IPKG_INSTROOT}" ] || ! uci -q get openssl.$(1)
>/dev/null; then
+cat << EOF >> "{OPENSSL_UCI}"
- Then, you will have to reinstall this package, and any other engine
package you have
- you have previously installed to ensure they are enabled:
-
- opkg install --force-reinstall $$(OSSL_ENG_PKG)
[OTHER_ENGINE_PKG]...
-
- EOF
-exit 1
-}
-ENGINES_CNF="{IPKG_INSTROOT}/etc/ssl/engines.cnf.d/engines.cnf"
-OPENSSL_CNF="{IPKG_INSTROOT}/etc/ssl/openssl.cnf"
-if [ ! -f "{OPENSSL_CNF}" ]; then
-echo -e "ERROR: File {OPENSSL_CNF} not found."
-error_out reinstall
-fi
-if ! grep -q "^.include /etc/ssl/engines.cnf.d" "{OPENSSL_CNF}";
then
-cat <<- EOF
- Your /etc/ssl/openssl.cnf file is not loading engine configuration
files from
- /etc/ssl/engines.cnf.d. You should consider start with a fresh,
updated OpenSSL config by
- running:
-
- opkg install --force-reinstall --force-maintainer libopenssl-conf
-
- The above command will overwrite any changes you may have made to
both /etc/ssl/openssl.cnf
- and /etc/ssl/engines.cnf.d/engines.cnf files, so back them up first!
- EOF
-error_out
-fi
-if [ ! -f "{ENGINES_CNF}" ]; then
-echo "Can't configure $$(OSSL_ENG_PKG): File {ENGINES_CNF} not
found."
-error_out reinstall
-fi
-if grep -q "$(1)=$(1)" "{ENGINES_CNF}"; then
-echo "$$(OSSL_ENG_PKG): $(1) engine was already configured.
Nothing to be done."
-else
-echo "$(1)=$(1)" >> "{ENGINES_CNF}"
-echo "$$(OSSL_ENG_PKG): $(1) engine enabled. All done!"
+config engine '$(1)'
+ option enabled '1'
+EOF
From my point of view, I think it would be better if we used the uci cli
command directly here.
to add the config engine section and enable this engine.
fi
+[ -z "{IPKG_INSTROOT}" ] && /etc/init.d/openssl reload
endef
- define Package/$$(OSSL_ENG_PKG)/prerm :=
+ define Package/$$(OSSL_ENG_PKG)/postrm :=
#!/bin/sh
-ENGINES_CNF="{IPKG_INSTROOT}/etc/ssl/engines.cnf.d/engines.cnf"
-[ -f "{ENGINES_CNF}" ] || exit 0
-sed -e '/$(1)=$(1)/d' -i "{ENGINES_CNF}"
+[ -z "{IPKG_INSTROOT}" ] && /etc/init.d/openssl reload
Should we not also remove the uci option on an uninstall wit the uci
command?
endef
endef
-
-
diff --git a/package/libs/openssl/files/openssl-engines.init
b/package/libs/openssl/files/openssl-engines.init
new file mode 100644
index 00..050a96f70a
--- /dev/null
+++ b/package/libs/openssl/files/openssl-engines.init
@@ -0,0 +1,19 @@
+#!/bin/sh /etc/rc.common
Is the init script also switched on at the first boot?
So that the service runs immediately?
Not that the service has to be switched on in
[PATCH 3/3] openssl: configure engines with uci
This uses uci to configure engines, by generating a list of enabled
engines in /var/etc/ssl/engines.cnf from engines configured in
/etc/config/openssl:
config engine 'devcrypto'
option enabled '1'
Currently the only options implemented are 'enabled', which defaults to
true and enables the named engine, and the 'force' option, that enables
the engine even if the init script thinks the engine does not exist.
The existence test is to check for either a configuration file
/etc/ssl/engines.cnf.d/%ENGINE%.cnf, or a shared object file
/usr/lib/engines-1.1/%ENGINE%.so.
The engine list is generated by an init script which is set to run after
'log' because it informs the engines being enabled or skipped. It
should run before any service using OpenSSL as the crypto library,
otherwise the service will not use any engine.
Signed-off-by: Eneas U de Queiroz
---
package/libs/openssl/Makefile | 13 +++--
package/libs/openssl/engine.mk| 58 +++
.../libs/openssl/files/openssl-engines.init | 19 ++
package/libs/openssl/files/openssl.init | 31 ++
.../150-openssl.cnf-add-engines-conf.patch| 5 +-
5 files changed, 70 insertions(+), 56 deletions(-)
create mode 100644 package/libs/openssl/files/openssl-engines.init
create mode 100755 package/libs/openssl/files/openssl.init
diff --git a/package/libs/openssl/Makefile b/package/libs/openssl/Makefile
index 238f7ecf02..0bf9e7a45f 100644
--- a/package/libs/openssl/Makefile
+++ b/package/libs/openssl/Makefile
@@ -11,7 +11,7 @@ PKG_NAME:=openssl
PKG_BASE:=1.1.1
PKG_BUGFIX:=k
PKG_VERSION:=$(PKG_BASE)$(PKG_BUGFIX)
-PKG_RELEASE:=3
+PKG_RELEASE:=4
PKG_USE_MIPS16:=0
PKG_BUILD_PARALLEL:=1
@@ -128,7 +128,6 @@ endef
define Package/libopenssl-conf/conffiles
/etc/ssl/openssl.cnf
-/etc/ssl/engines.cnf.d/engines.cnf
$(if
CONFIG_OPENSSL_ENGINE_BUILTIN_DEVCRYPTO,/etc/ssl/engines.cnf.d/devcrypto.cnf)
$(if CONFIG_OPENSSL_ENGINE_BUILTIN_PADLOCK,/etc/ssl/engines.cnf.d/padlock.cnf)
endef
@@ -378,15 +377,17 @@ define Package/libopenssl/install
endef
define Package/libopenssl-conf/install
- $(INSTALL_DIR) $(1)/etc/ssl/engines.cnf.d
+ $(INSTALL_DIR) $(1)/etc/ssl/engines.cnf.d $(1)/etc/config
$(1)/etc/init.d
$(CP) $(PKG_INSTALL_DIR)/etc/ssl/openssl.cnf $(1)/etc/ssl/
- $(CP) ./files/engines.cnf $(1)/etc/ssl/engines.cnf.d/
+ $(INSTALL_BIN) ./files/openssl.init $(1)/etc/init.d/openssl
+ $(SED) 's!%ENGINES_DIR%!/usr/lib/$(ENGINES_DIR)!'
$(1)/etc/init.d/openssl
+ touch $(1)/etc/config/openssl
$(if $(CONFIG_OPENSSL_ENGINE_BUILTIN_DEVCRYPTO),
$(CP) ./files/devcrypto.cnf $(1)/etc/ssl/engines.cnf.d/
- echo devcrypto=devcrypto >>
$(1)/etc/ssl/engines.cnf.d/engines.cnf)
+ echo -e "config engine 'devcrypto'\n\toption enabled '1'" >>
$(1)/etc/config/openssl)
$(if $(CONFIG_OPENSSL_ENGINE_BUILTIN_PADLOCK),
$(CP) ./files/padlock.cnf $(1)/etc/ssl/engines.cnf.d/
- echo padlock=padlock >> $(1)/etc/ssl/engines.cnf.d/engines.cnf)
+ echo -e "\nconfig engine 'padlock'\n\toption enabled '1'" >>
$(1)/etc/config/openssl)
endef
define Package/openssl-util/install
diff --git a/package/libs/openssl/engine.mk b/package/libs/openssl/engine.mk
index 482b5ad5e8..efa46d7214 100644
--- a/package/libs/openssl/engine.mk
+++ b/package/libs/openssl/engine.mk
@@ -23,60 +23,20 @@ define Package/openssl/add-engine
define Package/$$(OSSL_ENG_PKG)/postinst :=
#!/bin/sh
-# 1 == non-empty: suggest reinstall
-error_out() {
-[ "$1" ] && cat <<- EOF
- Reinstalling the libopenssl-conf package may fix this:
+OPENSSL_UCI="{IPKG_INSTROOT}/etc/config/openssl"
- opkg install --force-reinstall libopenssl-conf
- EOF
-cat <<- EOF
+if [ -n "{IPKG_INSTROOT}" ] || ! uci -q get openssl.$(1) >/dev/null; then
+cat << EOF >> "{OPENSSL_UCI}"
- Then, you will have to reinstall this package, and any other engine
package you have
- you have previously installed to ensure they are enabled:
-
- opkg install --force-reinstall $$(OSSL_ENG_PKG)
[OTHER_ENGINE_PKG]...
-
- EOF
-exit 1
-}
-ENGINES_CNF="{IPKG_INSTROOT}/etc/ssl/engines.cnf.d/engines.cnf"
-OPENSSL_CNF="{IPKG_INSTROOT}/etc/ssl/openssl.cnf"
-if [ ! -f "{OPENSSL_CNF}" ]; then
-echo -e "ERROR: File {OPENSSL_CNF} not found."
-error_out reinstall
-fi
-if ! grep -q "^.include /etc/ssl/engines.cnf.d" "{OPENSSL_CNF}"; then
-cat <<- EOF
- Your /etc/ssl/openssl.cnf file is not loading engine configuration
files from
- /etc/ssl/engines.cnf.d. You should consider start with a fresh,
updated OpenSSL config by
- running:
-
- opkg install --force-reinstall --force-maintainer libopenssl-conf
-
- The above command will overwrite any changes you may have made to both
/etc/ssl/openssl.cnf
- and
