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

Bug #2003668 reported by bugproxy
16
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ubuntu on IBM z Systems
In Progress
High
Skipper Bug Screeners
opencryptoki (Ubuntu)
In Progress
High
Simon Chopin

Bug Description

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.

bugproxy (bugproxy)
tags: added: architecture-s39064 bugnameltc-201342 severity-high targetmilestone-inin2304
Changed in ubuntu:
assignee: nobody → Skipper Bug Screeners (skipper-screen-team)
affects: ubuntu → linux (Ubuntu)
Revision history for this message
Frank Heimes (fheimes) wrote :

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

affects: linux (Ubuntu) → openssl (Ubuntu)
Changed in ubuntu-z-systems:
assignee: nobody → Skipper Bug Screeners (skipper-screen-team)
Changed in openssl (Ubuntu):
importance: Undecided → High
Changed in ubuntu-z-systems:
importance: Undecided → High
Changed in openssl (Ubuntu):
status: New → Incomplete
Changed in ubuntu-z-systems:
status: New → Incomplete
Revision history for this message
bugproxy (bugproxy) wrote : Comment bridged from LTC Bugzilla

------- 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

tags: added: targetmilestone-inin2310
removed: targetmilestone-inin2304
Frank Heimes (fheimes)
summary: - [23.04 FEAT] openSSL: PKCS #11 provider for signing operations (crypto)
+ [23.10 FEAT] openSSL: PKCS #11 provider for signing operations (crypto)
Revision history for this message
bugproxy (bugproxy) wrote :

------- 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)
summary: - [23.10 FEAT] openSSL: PKCS #11 provider for signing operations (crypto)
+ [23.10 FEAT] [needs-packaging] openssl-pkcs11-sign-provider
Changed in ubuntu-z-systems:
status: Incomplete → Triaged
Changed in openssl (Ubuntu):
status: Incomplete → Triaged
description: updated
Revision history for this message
Frank Heimes (fheimes) wrote :

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...

affects: openssl (Ubuntu) → opencryptoki (Ubuntu)
Changed in opencryptoki (Ubuntu):
assignee: Skipper Bug Screeners (skipper-screen-team) → Alexandre Erwin Ittner (aittner)
status: Triaged → In Progress
Frank Heimes (fheimes)
Changed in ubuntu-z-systems:
status: Triaged → In Progress
Revision history for this message
Alexandre Erwin Ittner (aittner) wrote :

 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

Frank Heimes (fheimes)
information type: Private → Public
tags: added: ubuntu-sponsors
Revision history for this message
Simon Chopin (schopin) wrote :

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

Changed in opencryptoki (Ubuntu):
assignee: Alexandre Erwin Ittner (aittner) → Simon Chopin (schopin)
Revision history for this message
Simon Chopin (schopin) wrote :

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?

Revision history for this message
bugproxy (bugproxy) wrote :

------- 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)
...
---------------------------

Revision history for this message
Alexandre Erwin Ittner (aittner) wrote :

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?

Revision history for this message
Simon Chopin (schopin) wrote :

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?

Revision history for this message
Steve Langasek (vorlon) wrote :

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.)

Revision history for this message
bugproxy (bugproxy) wrote :

------- 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.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.