Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net-libs/libmicrohttpd: add support for 'verify-sig' USE flag #34772

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Karlson2k
Copy link
Contributor

@Karlson2k Karlson2k commented Jan 12, 2024

The PR adds support for verify-sig for libmicrohttpd.
The relevant "keys" package is added as well.

The upstream updates release signature keys from time to time. To support different keys for different versions, the slotting is implemented in the sec-keys/openpgp-keys-libmicrohttpd package.

As the downloadable public key files can be changed from time to time (with the same fingerprint) due to additional user email, change of primary user ID or any other reason, to avoid breaking hash match the stable snapshots of the keys are integrated into the "keys" ebuild package.

@gentoo-bot gentoo-bot added new package The PR is adding a new package. self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else) assigned PR successfully assigned to the package maintainer(s). no signoff One or more commits do not indicate GCO sign-off. labels Jan 12, 2024
Signed-off-by: Karlson2k (Evgeny Grin) <k2k@narod.ru>
@Karlson2k Karlson2k changed the title net-libs/libmicrohttpd: add support for 'verify-sig' USE flag [please reassign] net-libs/libmicrohttpd: add support for 'verify-sig' USE flag Jan 12, 2024
@gentoo-bot gentoo-bot changed the title [please reassign] net-libs/libmicrohttpd: add support for 'verify-sig' USE flag net-libs/libmicrohttpd: add support for 'verify-sig' USE flag Jan 12, 2024
@gentoo-bot
Copy link

Pull Request assignment

Submitter: @Karlson2k
Areas affected: ebuilds
Packages affected: net-libs/libmicrohttpd, sec-keys/openpgp-keys-libmicrohttpd

net-libs/libmicrohttpd: @Karlson2k, @gentoo/proxy-maint
sec-keys/openpgp-keys-libmicrohttpd: @gentoo/proxy-maint (new package)

Linked bugs

No bugs to link found. If your pull request references any of the Gentoo bug reports, please add appropriate GLEP 66 tags to the commit message and request reassignment.


In order to force reassignment and/or bug reference scan, please append [please reassign] to the pull request title.

Docs: Code of ConductCopyright policy (expl.) ● DevmanualGitHub PRsProxy-maint guide

@gentoo-bot gentoo-bot added new package The PR is adding a new package. self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else) assigned PR successfully assigned to the package maintainer(s). and removed new package The PR is adding a new package. assigned PR successfully assigned to the package maintainer(s). self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else) no signoff One or more commits do not indicate GCO sign-off. labels Jan 12, 2024
Signed-off-by: Karlson2k (Evgeny Grin) <k2k@narod.ru>
@gentoo-repo-qa-bot
Copy link
Collaborator

Pull request CI report

Report generated at: 2024-01-12 08:53 UTC
Newest commit scanned: c99b82d
Status: ✅ good

There are existing issues already. Please look into the report to make sure none of them affect the packages in question:
https://qa-reports.gentoo.org/output/gentoo-ci/5df13bbf99/output.html

Comment on lines +76 to +93
ECONF_SOURCE="${S}" \
econf \
--enable-shared \
$(use_enable static-libs static) \
--disable-nls \
--enable-bauth \
--enable-dauth \
--disable-examples \
--enable-messages \
--enable-postprocessor \
--enable-httpupgrade \
--disable-experimental \
--disable-heavy-tests \
$(use_enable thread-names) \
$(use_enable epoll) \
$(use_enable test curl) \
$(use_enable ssl https) \
$(use_with ssl gnutls)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ECONF_SOURCE="${S}" \
econf \
--enable-shared \
$(use_enable static-libs static) \
--disable-nls \
--enable-bauth \
--enable-dauth \
--disable-examples \
--enable-messages \
--enable-postprocessor \
--enable-httpupgrade \
--disable-experimental \
--disable-heavy-tests \
$(use_enable thread-names) \
$(use_enable epoll) \
$(use_enable test curl) \
$(use_enable ssl https) \
$(use_with ssl gnutls)
local myeconfargs=(
--enable-shared
$(use_enable static-libs static)
--disable-nls
--enable-bauth
--enable-dauth
--disable-examples
--enable-messages
--enable-postprocessor
--enable-httpupgrade
--disable-experimental
--disable-heavy-tests
$(use_enable thread-names)
$(use_enable epoll)
$(use_enable test curl)
$(use_enable ssl https)
$(use_with ssl gnutls)
)
ECONF_SOURCE="${S}" econf ${myeconfargs[@]}

same for the 0.9.77 ebuild

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is not related to the idea of this PR.
The suggested change is mostly formatting and should not be required to merge this PR.

@eli-schwartz
Copy link
Contributor

As the downloadable public key files can be changed from time to time (with the same fingerprint) due to additional user email, change of primary user ID or any other reason, to avoid breaking hash match the stable snapshots of the keys are integrated into the "keys" ebuild package.

This feels unusual compared to other sec-keys packages. Changing keydata is usually a sign to bump the keys package to a new version (in particular for expiration date changes).

Especially since I seem to recall that savannah project-release keyrings are manually updated by the project admins, so it should be possible to update here at ~the same time.

@Karlson2k
Copy link
Contributor Author

Karlson2k commented Jan 21, 2024

As the downloadable public key files can be changed from time to time (with the same fingerprint) due to additional user email, change of primary user ID or any other reason, to avoid breaking hash match the stable snapshots of the keys are integrated into the "keys" ebuild package.

This feels unusual compared to other sec-keys packages. Changing keydata is usually a sign to bump the keys package to a new version (in particular for expiration date changes).

Especially since I seem to recall that savannah project-release keyrings are manually updated by the project admins, so it should be possible to update here at ~the same time.

This is correct. However, just downloading the keys from the savannah will break building the old versions (unless the old version of key are already cached).

Most of sec-keys packages download keys from the fixed upstream location, but this contradicts with the generic idea if ebuilds: the upstream files (usually sources) are expected to be unmodified for the the file lifetime, while upsteam keys could be updated from time to time without changing the filename.

I'm trying to solve both problems:

  • When the signature keys are updated by the upstream to match the latest release, this package will install the correct keys for the old versions. The cached file could be not reliable and the package (old versions) does not need to rely on cached file.
  • There is no problem with changed context/hash of downloaded files.

At the same time sec-keys/openpgp-keys-libmicrohttpd-999999.ebuild can be used anytime to get the actual keys.

Many sec-keys packages download fixed keys from fixed locations and they will need to be manually updated if keys are changed. In this package valid keys are already provided.

I'm the upstream maintainer and I'll update this package if nessesary.
I may add commented out section to download keys from the savannah so this ebuild could be easily picked up by another maintaner when nessesary.

@eli-schwartz
Copy link
Contributor

Right, my point is that other sec keys packages have the same issue but typically solve them by renaming the download files with the -> syntax (to add ${PV} to the filename).

Gentoo's mirror network ensures the versioned filename is available as long as the user doesn't disable mirrors, and as long as the ebuild is still in ::gentoo. This provides adequate time to merge an updated ebuild without locally committing the files to gentoo.git -- especially when the ebuild maintainer knows when the update will happen. :)

IMO the Gentoo distfiles mirror is the correct solution here.

@Karlson2k
Copy link
Contributor Author

Karlson2k commented Jan 21, 2024

Right, my point is that other sec keys packages have the same issue but typically solve them by renaming the download files with the -> syntax (to add ${PV} to the filename).

Gentoo's mirror network ensures the versioned filename is available as long as the user doesn't disable mirrors, and as long as the ebuild is still in ::gentoo. This provides adequate time to merge an updated ebuild without locally committing the files to gentoo.git -- especially when the ebuild maintainer knows when the update will happen. :)

IMO the Gentoo distfiles mirror is the correct solution here.

As far as I see there are two issues with the suggested solution:

  • It is not updated automatically, when keys are updated upstream. Manual update is required.
  • The keys are merged into gentoo.git

For the first issue: manual update is requed anyway when upstream update the keys. I'll take care about it, if anyone else would like to bump the sec file, one can just uncomment lines in .ebuild. In any case, the manual work is needed for the new keys.

Is it a fundamental thing to download the key from some URL in ebuild instead of providing the key directly if the key is already know and small? From my point of view, hardcoded key is safer for releases that already made. And it works even if upstream updated the keys and user disabled mirrors (or can't access the mirror, or wants to build the old version, removed already from ebuilds).

@thesamesam
Copy link
Member

If it really is small, we can tolerate it, per #33961 (comment).

@mgorny @ulm: thoughts?

Copy link
Member

@mgorny mgorny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If they were small, sure. However, these keys are definitely not small, and I feel like this is a case of wasteful overengineering over a theoretical problem that doesn't really affect our use cases. In the end, libmicrohttpd is not some kind of special package that would require "better" handling than all the other packages receive.

REQUIRED_USE="epoll? ( kernel_linux )"
RESTRICT="!test? ( test )"

KEYRING_VER=201906
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you actually need to version it? The general policy is that we add new keys to new versions, not require a very specific version for a given release.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allow building both old and new libmicrohttpd versions. The library changed the keyring in the past and will change it in future. With most probably API and ABI bump, slotting would be needed for the library. The infrastructure would be already there.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, what's the problem with adding new keys to the old ones?

Copy link
Contributor Author

@Karlson2k Karlson2k Jan 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If far as I see it is not common way for other packages.
Not sure how to implement it:

  • Download individual signing keys (and cache them on mirrors) and combine to the local keyring, add new signing keys when necessary. This way the old keys may be left only on gentoo mirrors, so users must rely on mirrors. Problematic to maintain as when cleanup of old keys would be required (after removal of the old packages versions) there are not clear connections between the keys and the releases. Anyway. it would need individual updates for every keychain update.
  • Download the current keyring from the upstream (and cache it on mirrors) and combine it with the old ones. The problems are similar to previous version, but the old key would not be available for sure. To cache the keyring on mirrors, manual update of ebuild would be needed for every key update anyway.
  • Download the current keyring during the emerge, do not cache file on mirrors. Non-standard way, typically used on portage only for "live" builds. The new keys would be added to the local installed keyring. In such case the local keyring will grow indefinitely as now cleanup is exist for it. New users will be unable to build the old version of the package as old keys are not available anymore as the "current upstream keyring".
    In any such case the signature checking is weaker as it would succeed if outdated old key (stolen, revoked) would be used to sign the new (unmatched) release.

The suggested solution is simple and straightforward.

The packages have versions and the relevant signature keys have versions as well. What's the problem with it?
The world is not frozen, including the keyrings used for signing.

The gentoo itself has keyring versioning: https://github.com/gentoo/gentoo/tree/master/sec-keys/openpgp-keys-gentoo-developers

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The keys should be added and removed from time to time. The reason for the keychain update is not only the new developer joined, but also developers left or keys expired and replaced.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the point you're trying to make? Literally every keyring package has versioned ebuilds. No one is suggesting to not have a versioned keyring ebuild. @mgorny is questioning why you need to have a special variable to define which version of the keyring you depend on, instead of simply depending on the latest version of the package by default.

If the keyring package adds an additional allowed signing key I'm not sure it's a reasonable vulnerability to worry that a newly added signing key can start signing old releases without authorization.

If the keyring package removes an allowed signing key then the ebuild for the newer release can use a >= requirement on the keyring package. But depending on the reason for removal it might be necessary to pull old releases! Which would mean updating those ebuilds anyway...

This tends to be a rarer occurrence anyway, and it's still possible to update the older ebuilds to add a <= on the keyring package build dependency. It is still only relevant for people doing fresh builds of the old package.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your examples do however demonstrate key files being downloaded via SRC_URI rather than being checked into gentoo.git and referenced from the filesdir.

@Karlson2k
Copy link
Contributor Author

If they were small, sure. However, these keys are definitely not small,

Could you please give some idea of what is small enough for the direct merge? The keys are 79 and 69 lines long and can be reduced to 73 and 64 lines. Many .ebuild files are much larger in terms of lines.
However, if it is a fundamental thing, I can convert them to downloadable.

and I feel like this is a case of wasteful overengineering over a theoretical problem that doesn't really affect our use cases.

This library changed keyring several times in the past and will change it in the future. With expected API and ABI bumps and slotting it would be nice to handle the keys for several versions (as several versions could be installed in parallel while the packages are transitioning).

In the end, libmicrohttpd is not some kind of special package that would require "better" handling than all the other packages receive.

The idea was to contribute a new (probably better) way to handle the signature keys. It is not specific to libmicrohtpd.
I saw two points that could be improved in keys handling:

  • keys are often downloaded from unstable sources. For other files unstable sources are allowed only for "live" builds and not cached by gentoo mirrors. The keys files handling is inconsistent with other source files handling.
  • once keychain for specific package is updated, there is no way to build old versions with verify-sig flag enabled (some packages allow it but have to rely on the data (keys) cached only on gentoo mirrors, which is inconsistent as for other files gentoo mirrors are not allowed to be the only source of files, the mirror must always have a copy of file hosted somewhere).

If it is principal position to not integrate the minimal keys to the git, I can convert this PR to download keys from various sources, however the key content could be unstable (with the same fingerprint) and larger (keys are not minimal).

@thesamesam
Copy link
Member

If it is principal position to not integrate the minimal keys to the git, I can convert this PR to download keys from various sources, however the key content could be unstable (with the same fingerprint) and larger (keys are not minimal).

Could you please do this for now, and start a discussion on the gentoo-dev ML please? It needs some wider discussion about how we handle this kind of case.

@Karlson2k
Copy link
Contributor Author

If it is principal position to not integrate the minimal keys to the git, I can convert this PR to download keys from various sources, however the key content could be unstable (with the same fingerprint) and larger (keys are not minimal).

Could you please do this for now, and start a discussion on the gentoo-dev ML please? It needs some wider discussion about how we handle this kind of case.

@thesamesam Implemented as an alternative PR #34994

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
assigned PR successfully assigned to the package maintainer(s). new package The PR is adding a new package. self-maintained The PR changes only packages that are maintained by the submitter (i.e. no need to ask anybody else)
Projects
None yet
7 participants