[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAASaF6zhiDdbfYkgg7wENih2Lp309eUgeu=FHZm0yYaEBt2gRg@mail.gmail.com>
Date: Fri, 20 Sep 2024 22:05:51 +0200
From: Jan Stancek <jstancek@...hat.com>
To: Jarkko Sakkinen <jarkko.sakkinen@....fi>
Cc: Neal Gompa <neal@...pa.dev>, David Howells <dhowells@...hat.com>, dwmw2@...radead.org,
zxu@...hat.com, keyrings@...r.kernel.org, linux-kernel@...r.kernel.org,
Asahi Linux <asahi@...ts.linux.dev>, Hector Martin <marcan@...can.st>, Janne Grunau <j@...nau.net>,
Jarkko Sakkinen <jarkko@...nel.org>
Subject: Re: [PATCH 0/3] sign-file,extract-cert: switch to PROVIDER API for
OpenSSL >= 3.0
On Fri, Sep 20, 2024 at 5:34 PM Jarkko Sakkinen <jarkko.sakkinen@....fi> wrote:
>
> On Fri Sep 20, 2024 at 2:42 PM EEST, Neal Gompa wrote:
> > On Tue, Aug 6, 2024 at 4:27 PM Neal Gompa <neal@...pa.dev> wrote:
> > >
> > > On Friday, July 12, 2024 3:11:13 AM EDT Jan Stancek wrote:
> > > > The ENGINE interface has its limitations and it has been superseded
> > > > by the PROVIDER API, it is deprecated in OpenSSL version 3.0.
> > > > Some distros have started removing it from header files.
> > > >
> > > > Update sign-file and extract-cert to use PROVIDER API for OpenSSL Major >=
> > > > 3.
> > > >
> > > > Tested on F39 with openssl-3.1.1, pkcs11-provider-0.5-2,
> > > > openssl-pkcs11-0.4.12-4 and softhsm-2.6.1-5 by using same key/cert as PEM
> > > > and PKCS11 and comparing that the result is identical.
> > > >
> > > > Jan Stancek (3):
> > > > sign-file,extract-cert: move common SSL helper functions to a header
> > > > sign-file,extract-cert: avoid using deprecated ERR_get_error_line()
> > > > sign-file,extract-cert: use pkcs11 provider for OPENSSL MAJOR >= 3
> > > >
> > > > MAINTAINERS | 1 +
> > > > certs/Makefile | 2 +-
> > > > certs/extract-cert.c | 138 +++++++++++++++++++++++--------------------
> > > > scripts/sign-file.c | 134 +++++++++++++++++++++--------------------
> > > > scripts/ssl-common.h | 32 ++++++++++
> > > > 5 files changed, 178 insertions(+), 129 deletions(-)
> > > > create mode 100644 scripts/ssl-common.h
> > >
> > > The code looks fairly reasonable to me and behaves as expected.
> > >
> > > I have been actively using this patch set for several weeks now across
> > > linux-6.9.y and now linux-6.10.y with good success.
> > >
> > > It is in use in production for Fedora Asahi Linux kernels with good success.
> > > Thanks for the fixes. :)
> > >
> > > Reviewed-by: Neal Gompa <neal@...pa.dev>
> > >
> >
> > Jarkko, could you please consider submitting this for inclusion into
> > 6.12? I've been carrying this for three Linux kernel rebases now
> > (6.9.y, 6.10.y, and now 6.11.y) and it seems to be just fine, and
> > without it, I cannot build kernels anymore with the OpenSSL engine API
> > disabled in Fedora and CentOS/RHEL. I also expect that the engine API
> > will disappear on other platforms in the near future given its
> > deprecated status and recently accelerated conversion of engine
> > backends to the newer provider API.
> >
> > Thanks in advance! :)
>
> Yes, I think I can. And I've yet to do 6.12 PR because I've been
> busy sorting out perf regression in the TPM driver.
>
> ERROR: need consistent spacing around '*' (ctx:WxV)
> #66: FILE: certs/extract-cert.c:69:
> + OSSL_STORE_CTX *store;
> ^
>
> ERROR: need consistent spacing around '*' (ctx:WxV)
> #93: FILE: certs/extract-cert.c:96:
> + ENGINE *e;
> ^
>
> ERROR: need consistent spacing around '*' (ctx:WxV)
> #199: FILE: scripts/sign-file.c:114:
> + OSSL_STORE_CTX *store;
> ^
>
> ERROR: need consistent spacing around '*' (ctx:WxV)
> #229: FILE: scripts/sign-file.c:141:
> + ENGINE *e;
> ^
> Any ideas of these? My guess is that they are unfixable and related
> to non-kernel-standard code.
This looks like false-positive. Following will produce same error:
+#if TEST
+ ENGINE *e;
+#endif
+
$ git diff > 1.patch; ./scripts/checkpatch.pl 1.patch
ERROR: need consistent spacing around '*' (ctx:WxV)
#10: FILE: scripts/sign-file.c:217:
+ ENGINE *e;
^
total: 1 errors, 0 warnings, 10 lines checked
but if first type in #if block is something checkpatch recognizes,
then it reports no issues:
+#if TEST
+ int i;
+ ENGINE *e;
+#endif
+
$ git diff > 1.patch; ./scripts/checkpatch.pl 1.patch
total: 0 errors, 0 warnings, 11 lines checked
Regards,
Jan
Powered by blists - more mailing lists