mbox series

[0/3] sign-file,extract-cert: switch to PROVIDER API for OpenSSL >= 3.0

Message ID cover.1720728319.git.jstancek@redhat.com (mailing list archive)
Headers show
Series sign-file,extract-cert: switch to PROVIDER API for OpenSSL >= 3.0 | expand

Message

Jan Stancek July 12, 2024, 7:11 a.m. UTC
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

Comments

Herbert Xu Aug. 2, 2024, 1:10 p.m. UTC | #1
On Fri, Jul 12, 2024 at 09:11:13AM +0200, 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

Adding Cc to Jarkko Sakkinen <jarkko@kernel.org>.
Jarkko Sakkinen Aug. 2, 2024, 5:59 p.m. UTC | #2
On Fri Aug 2, 2024 at 4:10 PM EEST, Herbert Xu wrote:
> On Fri, Jul 12, 2024 at 09:11:13AM +0200, 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
>
> Adding Cc to Jarkko Sakkinen <jarkko@kernel.org>.

I can download this but is it likely that there will be v2? If so,
I'll hoold on for that. If not, I'll download it.

BR, Jarkko
Jan Stancek Aug. 2, 2024, 6:27 p.m. UTC | #3
On Fri, Aug 2, 2024 at 8:07 PM Jarkko Sakkinen <jarkko.sakkinen@iki.fi> wrote:
>
> On Fri Aug 2, 2024 at 4:10 PM EEST, Herbert Xu wrote:
> > On Fri, Jul 12, 2024 at 09:11:13AM +0200, 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
> >
> > Adding Cc to Jarkko Sakkinen <jarkko@kernel.org>.
>
> I can download this but is it likely that there will be v2? If so,
> I'll hoold on for that. If not, I'll download it.

Noone requested any changes to v1 so far, so if you can have a look,
it would be much appreciated.

Thanks,
Jan

>
> BR, Jarkko
>
Jarkko Sakkinen Aug. 2, 2024, 7:54 p.m. UTC | #4
On Fri Aug 2, 2024 at 9:27 PM EEST, Jan Stancek wrote:
> On Fri, Aug 2, 2024 at 8:07 PM Jarkko Sakkinen <jarkko.sakkinen@iki.fi> wrote:
> >
> > On Fri Aug 2, 2024 at 4:10 PM EEST, Herbert Xu wrote:
> > > On Fri, Jul 12, 2024 at 09:11:13AM +0200, 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
> > >
> > > Adding Cc to Jarkko Sakkinen <jarkko@kernel.org>.
> >
> > I can download this but is it likely that there will be v2? If so,
> > I'll hoold on for that. If not, I'll download it.
>
> Noone requested any changes to v1 so far, so if you can have a look,
> it would be much appreciated.

OK that is totally fine! I'm just prioritizing and postponing stuff
that I can (WHEN it makes sense pragmatically)  to get back in the 
phase ;-)

I take a look at this next week.

> Thanks,
> Jan

BR, Jarkko
Neal Gompa Aug. 6, 2024, 8:27 p.m. UTC | #5
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@gompa.dev>
R Nageswara Sastry Sept. 3, 2024, 8:11 a.m. UTC | #6
>Date: Fri, 12 Jul 2024 09:11:13 +0200	[thread overview]
>Message-ID: <cover.1720728319.git.jstancek@redhat.com> (raw)
>
>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

Tested on a system where openssl-engine package was not available.
With out the patch, couldn't compile upstream kernel (tried 6.11-rc5)
With patch, could compile upstream kernel (tried 6.11-rc5)

Tested-by: R Nageswara Sastry <rnsastry@linux.ibm.com>

>
>-- 
>2.39.3
Neal Gompa Sept. 20, 2024, 11:42 a.m. UTC | #7
On Tue, Aug 6, 2024 at 4:27 PM Neal Gompa <neal@gompa.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@gompa.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! :)
Jarkko Sakkinen Sept. 20, 2024, 3:34 p.m. UTC | #8
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@gompa.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@gompa.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.

BR, Jarkko
Jan Stancek Sept. 20, 2024, 8:05 p.m. UTC | #9
On Fri, Sep 20, 2024 at 5:34 PM Jarkko Sakkinen <jarkko.sakkinen@iki.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@gompa.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@gompa.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
Jarkko Sakkinen Sept. 20, 2024, 10:16 p.m. UTC | #10
On Fri Sep 20, 2024 at 11:05 PM EEST, Jan Stancek wrote:
> 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

I fixed a couple of alignment issues reported by "checkpatch.pl
--strict" and sent a pull request:

https://lore.kernel.org/keyrings/D4B9WXZC9B9U.34I6BHWMYXOS2@kernel.org/T/#u

> Regards,
> Jan

BR, Jarkko