[23.10 FEAT] [needs-packaging] openssl-pkcs11-sign-provider

[bugproxy]

openssl-pkcs11-sign-provider

OpenSSL Provider for asymmetric signing operations with private PKCS#11 keys.
 OpenSSL 3.0 replaces the engine pluging framework by a provider plugin frame work. Even though the engine framework shall remain as a deprecated framework in openSSL 3.0, Linux distributions may build openSSL without engine support.
 Therefore a restricted PKCS #11 provider is required that supports signing operations via PKCS #11 for existing keys reference by an PKCS #11 object URI.

URL: https://github.com/opencryptoki/openssl-pkcs11-sign-provider
License: Apache-2.0
Notes:
 v1.0.0 was recently tagged: https://github.com/opencryptoki/openssl-pkcs11-sign-provider/releases/tag/v1.0.0
 A package for FC exists as well: https://packages.fedoraproject.org/pkgs/openssl-pkcs11-sign-provider/openssl-pkcs11-sign-provider/
__________

Feature Description:

openSSL 3.0 replaces the engine pluging framework by a provider plugin frame work. Even though the engine framework shall remain as a deprecated framework in openSSL 3.0, Linux distributions may build openSSL without engine support.
Therefore a restricted PKCS #11 provider is required that supports signing operations via PKCS #11 for existing keys reference by an PKCS #11 object URI.

[Frank Heimes (fheimes)]

Is this feature part of a certain (minor) openssl release,
or is it required to cherrypick certain commits?

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2023-02-02 08:32 EDT-------
The openssl-pkcs11-sign-provider is still under development and will not make it in time for lunar FF.

Therefore, we need to postpone this item to Ubuntu 23.10.
Thanks

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2023-07-02 22:15 EDT-------
Update by Holger:

In the meantime, the openssl-pkcs11-sign-provider has been release in the upstream repository on github
https://github.com/opencryptoki/openssl-pkcs11-sign-provider/releases/tag/v1.0.0

The openssl-pkcs11-sign-provider package is now available for fc37/fc38/rawhide, see:
https://packages.fedoraproject.org/pkgs/openssl-pkcs11-sign-provider/openssl-pkcs11-sign-provider/

This package includes the above mentioned release v1.0.0.

[Frank Heimes (fheimes)]

Since openssl-pkcs11-sign-provider is hosted by the opencryptoki project, I'll mark it as affecting opencryptoki (rather than openssl) - has obviously a relation to both...

[Alexandre Erwin Ittner (aittner)]

 Just uploaded the package for review in this ppa (no debdiff as it is a new one): https://launchpad.net/~aittner/+archive/ubuntu/newpkgs/+sourcepub/15156507/+listing-archive-extra

[Simon Chopin (schopin)]

Removing ubuntu-sponsors as I'll be doing the review for that one :)

[Simon Chopin (schopin)]

Overall, nice work! I do have some questions and remarks, though.

* Why not Arch: any ? opencryptoki is available on all platforms specifically
  to make testing easier, I think this provider should do the same.
  Anecdotically, it builds fine on amd64.

* Can we install the .so files in /usr/lib/$ARCH/ossl-modules ? This would
  allow better integration with oepnss, make the openssl.cnf.in a bit simpler,
  and probably shut lintian up on some of its warnings. An easy way to get the
  path is via `openssl version -m`

* Can it work with any pkcs11 implementation, or is it just opencryptoki? The
  naming suggests that it is implementation-agnostic.

* d/control: why Ubuntu MOTU as maintainers? Customarily we use Ubuntu
  Developers <email address hidden>

* error: d/control: Section: why specify 'universe'? I don't have an example of
  a Ubuntu-only universe package at hand, but all packages imported from Debian
  simply have "libs" as a section, and they all go to their designated section
  anyway. Leaving it unspecified allows for an MIR without having to change the
  packaging, which is always nice.

* d/copyright: the packaging copyright should be under the same license as
  upstream, to avoid issues when dealing with patches. That's in alignment with
  Canonical's policy.

* Lintian: the package isn't Lintian-clean, but then again I'm expecting most of these
  to go away if you address all the previous points:
W: openssl-pkcs11-sign-provider: package-name-doesnt-match-sonames pkcs11sign
W: openssl-pkcs11-sign-provider: shared-library-lacks-version usr/lib/x86_64-linux-gnu/pkcs11sign.so pkcs11sign.so
W: openssl-pkcs11-sign-provider: unknown-section universe/libs
W: openssl-pkcs11-sign-provider-dbgsym: unknown-section universe/debug
I: openssl-pkcs11-sign-provider: no-symbols-control-file usr/lib/x86_64-linux-gnu/pkcs11sign.so
I: openssl-pkcs11-sign-provider: typo-in-manual-page "allows to" "allows one to" [usr/share/man/man5/pkcs11sign.cnf.5.gz:123]

* d/control: This is pedantism in its purest form: there's usually a space
  between '=' and the version number in your dependency contraints.

* d/control: a neat tool to keep things organized in your control file is
  wrap-and-sort, it would help keeping your Build-Depends in check and make
  evolutions easier to review.

* d/watch: I'm far from an expert in uscan and watchfiles. However, there seems
  to be something wrong there, the filenamemangle talks about tar.xz whereas
  the GH api is apparently giving a tar.gz. In addition, upstream provides
  signature files (.asc), and uscan had the capability to automatically check
  them, could we enable that?

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2023-09-18 10:52 EDT-------
I agree, the openssl-pkcs11-sign-provider is not architecture specific. Please change to "Architecture: any".

The configure: target in debian/rules should also set the libdir to the "modulesdir" variable of libcrypto, because openssl is expecting the provider so-file there. pkg-config is providing this information, so get it from there.

debian/rules --------------
...
configure:
dh_auto_configure -- --libdir $(pkg-config --variable modulesdir libcrypto)
...
---------------------------

[Alexandre Erwin Ittner (aittner)]

I just uploaded the updates to https://launchpad.net/~aittner/+archive/ubuntu/lp2003668-test/+packages

The new watchfile was a lot trickier than I expected, trying to get the releases from the API failed after a rate limit was reached, so I reverted to reading the list of tags and trying to find a matching release tarball (by tag name) and its signature. Seems convoluted, but works:

opts="pgpsigurlmangle=s/$/.asc/,\
filenamemangle=s#v@ANY_VERSION@\.tar\.gz#@PACKAGE@-$1.tar.gz#,\
oversionmangle=s#v@ANY_VERSION@#$1#,\
downloadurlmangle=s#archive/refs/tags/v(.*)\.tar\.gz#releases/download/v$1/@PACKAGE@-$1\.tar\.gz#" \
https://github.com/opencryptoki/@PACKAGE@/tags \
https://github.com/opencryptoki/@PACKAGE@/archive/refs/tags/(.*)\.tar\.gz

Should we keep it?

[Simon Chopin (schopin)]

Many thanks Holger for the pkg-config snippet!

Alexandre, I'll trust you on the watchfile, since my knowledge of uscan arcane is very thin.

I uploaded the package as is, as it's in a fairly good shape and we're cutting it pretty close to the release. However, I do have following remarks that should be addressed in the next upload *after the release*:

* issue: Please add the pubkey used to sign the releases, Lintian complains (fairly) about it:
  E: openssl-pkcs11-sign-provider source: debian-watch-file-pubkey-file-is-missing [debian/watch]

* suggestion: d/copyright: if you add a stand-alone License: Apache-2 paragraph at the end of the file, you can avoid repeating the full text of the license. That's how the MIT license is handled in this random example:
  https://salsa.debian.org/debian/weechat-matrix/-/blob/master/debian/copyright

* suggestion: d/docs: openssl-ock.cnf.sample should probably be in d/examples (see dh_installexamples)

* nitpick: d/rules: I don't think the makeshlibs override is still necessary now that we fixed the install path?

[Steve Langasek (vorlon)]

I'm sorry, having reviewed this for NEW processing, I disagree that this is in fairly good shape.

               debhelper (>= 13),
               debhelper-compat (= 13),

This is just plain incorrect. The latter supersedes the former, they should never be used together.

               pkg-config (>= 1.8.1)

Why is this a *versioned* build-dependency on a virtual package? There has never been a real package named pkg-config at version >= 1.8.1 in the Ubuntu archive; the last upstream version was 0.29.2.

Depends: libopencryptoki0 (>= 3.21.0),
         libssl3 (>= 3.0.10),

Never, ever, ever hard-code runtime dependencies against specific library sonames in a source package. libopencryptoki0 is a special case because of its unique architecture and the fact that this is unfortunately being dlopen()ed. But hard-coding a dependency on libssl3 means this is broken the next time the soname bumps and becomes tech debt.

Files: debian/*
Copyright: 2023 Canonical LTD
License: Apache-2

This is not consistent with Canonical's licensing policy, which says work should be released under GPLv3 by default.

Packaging is not a modification to an existing open source work, so the proviso that "Modifications to existing open source work (including software) should be contributed under the prevailing license" does not apply here.

override_dh_makeshlibs:
        # This is a plugin which is dlopen()ed, not linked against.

Yes, but dh_makeshlibs knows a .so installed somewhere other than the library search path is not a library, so this is unnecessary...

The debian/copyright is per se sufficient for a reject from the NEW queue, but a lot of these other issues while they would normally not be blockers for NEW processing and could be treated as bugs to be resolved post-acceptance, in aggregate they make for a package I would not be willing to sponsor and so for a new Ubuntu-specific source package I do consider this to need to be cleaned up before reupload.

(Thanks for linking a bug report in the changelog, though; that makes it a lot easier to provide this feedback.)

[bugproxy (bugproxy)]

------- Comment From <email address hidden> 2023-11-09 06:37 EDT-------
Hi Steve,

I mostly agree with your objections. Let me add some details about a few items.

- pkg-config: an un-versioned build dependency should be sufficient

- libssl3: This dependency is a bit tricky. We depend on the major version 3 of openssl (aka libssl). This is already covered, because the package name includes the major version number. Unfortunately, the openssl project made some important changes to the provider API, which we also require. Therefore, the provider module will only work with openssl/libssl3-versions >= 3.0.8.

- libopencryptoki0: The openssl-pkcs11-sign-provider module .so does not have a hard dependency in the dynamic section, but it should be able to load at least one pkcs#11 module (via dlopen). So the dependency to a pkcs#11 library depends on the config. Maybe the package does not have/need a runtime dependency to opencryptoki at all.

- license: I also agree on that. The packaging files can be licensed under any opensource license. If the ubuntu guidelines require GPL here, I'm ok with that.

[Alexandre Erwin Ittner (aittner)]

Hi,

I'm sending an updated version of this package in https://launchpad.net/~aittner/+archive/ubuntu/openssl-pkcs11-sign-provider-upload/+packages ... as the previous was never published, I it will still be -ubuntu1, so no debdiff.

Specific changes:

> debhelper (>= 13),
> debhelper-compat (= 13),

I removed the first dependency and left the one on debhelper-compat, so it will replace both d/compat and the one on dh >= 13.

> pkg-config (>= 1.8.1)

Replaced by pkgconf. I probably let the version slip through when I had to chose between virtual pkg-config and the real pkgconf.

> Depends: libopencryptoki0 (>= 3.21.0),
> libssl3 (>= 3.0.10),

I kept the dependencies, as from previous comments we need them to ensure that the package will work. Forcing libssl3 to >= 3.0.10 should not be a problem anymore as we are using versions later than 3.0.8, but making the dependency explicit seems better as the older can fail without raising any compile time errors due to the dlopen.

If keeping these dependencies are also a problem, perhaps I may add a dep8 test to try to load the module ... if will ensure that *something* will fail if the ABI changes, but it also seems we'll be just reimplementing a thing that we can alredy do with an more standard approach.

opencryptoki0 is only required because the sample config calls it; but we can remove both is this becomes a problem.

> Files: debian/*
> Copyright: 2023 Canonical LTD
> License: Apache-2

Reverted the license back to GPL-3.

> override_dh_makeshlibs:
> # This is a plugin which is dlopen()ed, not linked against.

I'm pretty sure this one was causing a lintian false positive, so I kept it. Should I remove and add a lintian override instead ?

I also fixed some of @schopin 's suggestions form #10.

Anyway, any other comment or feedback is welcome!

[Frank Heimes (fheimes)]

Please note that this is needed / got requested for mantic, hence we need to have it in noble first and in mantic (I've updated the 'targets' releases).

[Simon Chopin (schopin)]

The hardcoded libopencryptoki0 is indeed pretty unavoidable due to the dlopen(). However, I'd rather drop the libssl3 dependency altogether, as, in the context of Ubuntu, this package will not be backported to releases between what's currently picked up by shlibs:Depends (3.0.3) and the actual requirements outlined by upstream (3.0.8). We currently have 3.0.2 in Jammy, 3.0.8 in Lunar and 3.0.10 in Mantic. If a user finds themselves trying to install a package compiled in a Noble/Mantic/Lunar environment onto a Kinetic system, they're definitely in an unsupported case.

Using the shlibs stanza will only help in the future maintenance of the package and, as pointed out by Steve, is better for overall archive maintenance.

Regarding libopencryptoki0, you could however demote it to a Recommends, since users can use whatever pkcs11 implementation they want via simple configuration.

The pkgconf fix hasn't made it to the PPA :)

[Alexandre Erwin Ittner (aittner)]

I sent an updated package for noble, with the fixes from #15, to this PPA: https://launchpad.net/~aittner/+archive/ubuntu/lp2003668-noble1/+packages

Feedback is welcome!

[Alexandre Erwin Ittner (aittner)]

New version removing a non-strictly-needed version for pkgconf: https://launchpad.net/~aittner/+archive/ubuntu/lp2003668-noble2

[Simon Chopin (schopin)]

Uploaded :)

[Frank Heimes (fheimes)]

Many thanks aittner and schopin!

[Łukasz Zemczak (sil2100)]

I looked at the package in the NEW queue and it's mostly good, but I think we'd need to tweak the debian/copyright a bit. I see there's at least a few GPLv2+ files in the source that are not listed as such in the debian/copyright (example: ./ltmain.sh). Any reason for not including those there?

[Frank Heimes (fheimes)]

Well, looks like upstream/git-hub is not very precise with the statement:
"Apache-2.0, Unknown licenses found"
Yes, it has GPL-2 and GPL-3 files inside.
I grep-ed the entire source package for copyright and license, and modified d/copyright accordingly, which results in the attached d/copyright.

I think that covers the missing bits and lintian is happy about it too (ran it on src and bin).

An updated package will be provided soon...

[Alexandre Erwin Ittner (aittner)]

Thanks @fheimes !

I uploaded the updated package to this ppa: https://launchpad.net/~aittner/+archive/ubuntu/lp2003668-noble3 ... it's already built but the binaries are still awaiting publication, however.

[Frank Heimes (fheimes)]

Thank you @aittner!

I just did another review and some checks (incl. lintian - looked all good) and
uploaded the updated version with the modified/expanded d/copyright file to noble's NEW queue.

[Frank Heimes (fheimes)]

Package landed in noble-proposed:
 openssl-pkcs11-sign-provider | 1.0.0-0ubuntu1 | noble-proposed/universe | source
hence updating status to Fix Committed for noble
and 'Won't Fix' for Mantic, since it's sufficient (for IBM) to have it in the LTS release.

[Frank Heimes (fheimes)]

Package landed meanwhile in noble's release pocket:
 openssl-pkcs11-sign-provider | 1.0.0-0ubuntu1 | noble/universe | source, amd64, arm64, armhf, ppc64el, s390x
With that this ticket is Fix Released.

Thanks for the work, review, feedback, sponsoring and approval!