Spec URL: https://jlayton.fedorapeople.org/ktls-utils/0.7.1/ktls-utils.spec SRPM URL: https://jlayton.fedorapeople.org/ktls-utils/0.7.1/ktls-utils-0.7.1-1.fc39.src.rpm Description: In-kernel TLS consumers need a mechanism to perform TLS handshakes on a connected socket to negotiate TLS session parameters that can then be programmed into the kernel's TLS record protocol engine. This package of software provides a TLS handshake user agent that listens for kernel requests and then materializes a user space socket endpoint on which to perform these handshakes. The resulting negotiated session parameters are passed back to the kernel via standard kTLS socket options. Fedora Account System Username: jlayton
Copr build: https://copr.fedorainfracloud.org/coprs/build/5714219 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2182151-ktls-utils/fedora-rawhide-x86_64/05714219-ktls-utils/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
[fedora-review-service-build]
Sorry, missed a couple of BuildRequires on the first pass.
Copr build: https://copr.fedorainfracloud.org/coprs/build/5714318 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2182151-ktls-utils/fedora-rawhide-x86_64/05714318-ktls-utils/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Copr build: https://copr.fedorainfracloud.org/coprs/build/5714338 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2182151-ktls-utils/fedora-rawhide-x86_64/05714338-ktls-utils/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
The build check seems to be picking up some cached version of the SRPM. Putting a copy at a new URL: Spec URL: https://jlayton.fedorapeople.org/ktls-utils/0.7.1/ktls-utils.spec SRPM URL: https://jlayton.fedorapeople.org/ktls-utils/0.7.1/ktls-utils-0.7.1-2.fc39.src.rpm
Copr build: https://copr.fedorainfracloud.org/coprs/build/5714477 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2182151-ktls-utils/srpm-builds/05714477/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Copr build: https://copr.fedorainfracloud.org/coprs/build/5714485 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2182151-ktls-utils/fedora-rawhide-x86_64/05714485-ktls-utils/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
The "could not download source" warning is expected at this time. We're going to want to package v0.8 as the first release, but its tarball is not cut yet. Once v0.8 is cut we'll update to that version. This is mainly to make sure that the packaging bits are correct ahead of that.
I revised the COPR repo to hold several packages: https://copr.fedorainfracloud.org/coprs/jlayton/rpctls/ I intend to keep this sporadically updated for testing purposes. The current ktls package (0.7.1) is a snapshot of Chuck's tree from yesterday. I versioned it that way to ensure that 0.8 will supersede it. I didn't see any point in packaging 0.7 since it used the old upcall method. I'm also working on a kernel with the requisite patches, but that's not ready yet.
(In reply to Jeff Layton from comment #12) > The current ktls package (0.7.1) is a snapshot of Chuck's tree from yesterday. I > versioned it that way to ensure that 0.8 will supersede it. I didn't see any > point in packaging 0.7 since it used the old upcall method. > Since this is a git tree snapshot, please follow <https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_snapshots> in using a proper Version string. Alternatively upgrade to 0.8 which seems to have been released. Also the License is wrong. Fedora now uses SPDX license identifiers <https://docs.fedoraproject.org/en-US/legal/allowed-licenses/>.
Thanks Petr. I've updated this to 0.8 and fixed the license string: https://jlayton.fedorapeople.org/ktls-utils/0.8/ktls-utils.spec https://jlayton.fedorapeople.org/ktls-utils/0.8/ktls-utils-0.8-4.fc39.src.rpm ...also fyi, I have this COPR repo set up for testing: https://copr.fedorainfracloud.org/coprs/jlayton/rpctls/ The kernel patches for this will likely land during the (now-open) merge window for v6.4, so it'd be nice to have this ready to ship in Fedora soon.
Created attachment 1959515 [details] The .spec file difference from Copr build 5714485 to 5844323
Copr build: https://copr.fedorainfracloud.org/coprs/build/5844323 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2182151-ktls-utils/fedora-rawhide-x86_64/05844323-ktls-utils/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
URL is Ok. FIX: Source0 address is broken: $ spectool -g ../SPECS/ktls-utils.spec Downloading: https://github.com/oracle/ktls-utils/archive/v0.8/ktls-utils-0.8.tar.gz [...] HTTP request sent, awaiting response... 404 Not Found 2023-04-24 16:13:23 ERROR 404: Not Found. The address advertised by upstream is <https://github.com/oracle/ktls-utils/releases/download/ktls-utils-0.8/ktls-utils-0.8.tar.gz>. I don't know whether it's because of a git tag being "v0.8" while the release archive just "0.8" without "v", or it's a bug in %forgemeta macro. If you do not want to debug it, I recommend specifying Source0 address manually like this: Source0: %{forgeurl}/releases/download/%{name}-%{version}/%{name}-%{version}.tar.gz Source0 archive (SHA-512: 027824a8ffb42bf8b39ce8d8a83f8f3d0c3d2e6cd0c2867f622e04ce914f578767ce7803617fe922c44a5fb5e69636efc6c0fc1726be1a3852b41cb6ad7579eb) is original. Ok. Summary is Ok. Description verified in README. Ok. GPL-2.0-only license verified from configure.ac, autogen.sh, LICENSE.txt, src/tlshd/handshake.c, src/tlshd/config.c, src/tlshd/ktls.c, src/tlshd/tlshd.man, src/tlshd/main.c, src/tlshd/log.c, src/tlshd/tlshd.conf.man, src/tlshd/tlshd.conf, src/tlshd/netlink.c, src/tlshd/server.c, src/tlshd/client.c, src/tlshd/keyring.c, src/tlshd/tlshd.h, COPYING (a duplicate of LICENSE.txt). FIX: These licenses are missing from a License tag: GPL-2.0-only OR BSD-3-Clause: src/tlshd/netlink.h (Fedora ignores Linux-syscall-note "exception" <https://gitlab.com/fedora/legal/fedora-license-data/-/issues/198>). GPL-1.0-or-later: README.md. TODO: I believe a license declaration in REAMDE.md ("Released under the GNU GENERAL PUBLIC LICENSE") is an upstream's omission and that they rather intended "GNU GENERAL PUBLIC LICENSE version 2" there. Please report it to them. Licenses of nonpackaged files: FSFAP: INSTALL FSFUL: configure FSFULLRWD AND GPL-2.0-only: Makefile.in, src/Makefile.in, src/tlshd/Makefile.in, systemd/Makefile.in FSFULLR ANDF FSFULLRWD AND GPL-2.0-or-later WITH Autoconf-exception-generic: aclocal.m4 GPL-1.0-or-later: README GPL-2.0-only: Makefile.am, src/Makefile.am, src/tlshd/Makefile.am, systemd/Makefile.am GPL-2.0-or-later WITH Autoconf-exception-generic: compile, depcomp, missing X11: install-sh FIX: Build-require 'bash' (autogen.sh:1). TODO: Constrain 'autoconf' build dependency with '>= 2.69' (configure.ac:20). FIX: Build-require 'pkgconf-pkg-config >= 0.9.0' (configure.ac:49). TODO: Build-require 'pkgconfig(gnutls) >= 3.3.0' instead of 'gnutls-devel' (configure.ac:50). In Fedora we prefer depending on pkg-config modules over devel subpackages <https://docs.fedoraproject.org/en-US/packaging-guidelines/PkgConfigBuildRequires/>. TODO: Build-require 'pkgconfig(libkeyutils)' instead of 'keyutils-libs-devel' (configure.ac:53). TODO: Build-require 'pkgconfig(glib-2.0) >= 2.6' instead of 'glib2-devel' (configure.ac:56). TODO: Build-require 'pkgconfig(libnl-3.0) >= 3.1' instead of 'libnl3-devel' (configure.ac:59). FIX: Build-require 'coreutils' (systemd/Makefile.am:28). No tests, no %check section. Ok. TODO: Package AUTHORS and ChangeLog files with %doc macro. Systemd unit file, including the disabled default dependencies, is Ok. $ rpmlint ktls-utils.spec ../SRPMS/ktls-utils-0.8-1.fc39.src.rpm ../RPMS/x86_64/ktls-utils-* ======================================== rpmlint session starts ======================================= rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 31, packages: 5 ktls-utils.x86_64: W: crypto-policy-non-compliance-gnutls-1 /usr/sbin/tlshd gnutls_priority_set_direct ========= 4 packages and 1 specfiles checked; 0 errors, 1 warnings, 0 badness; has taken 0.3 s ======== FIX: The daemon does not respect distribution-wide crypto policies. It enables algorithms in tlshd_make_priorities_string() based on what Linux supported at build time of this package. Ideally the daemon should consult crypto policy <https://docs.fedoraproject.org/en-US/packaging-guidelines/CryptoPolicies/> and only enable a disjunction of what Linux offers and what user-space crypto policy mandates. Please contact <security.org> for help. There is a possibility that Linux already does that in other way. Please get a crypto review from the security team on that mailing list. The package build in Fedora 39 (https://koji.fedoraproject.org/koji/taskinfo?taskID=100344410). Ok. $ rpm -q -lv -p ../RPMS/x86_64/ktls-utils-0.8-1.fc39.x86_64.rpm -rw-r--r-- 1 root root 1016 Apr 5 17:24 /etc/tlshd.conf drwxr-xr-x 2 root root 0 Apr 24 02:00 /usr/lib/.build-id drwxr-xr-x 2 root root 0 Apr 24 02:00 /usr/lib/.build-id/d8 lrwxrwxrwx 1 root root 26 Apr 24 02:00 /usr/lib/.build-id/d8/09e2707e2a4a7eb2335e8e605f7e05f9402d7a -> ../../../../usr/sbin/tlshd -rw-r--r-- 1 root root 226 Apr 24 02:00 /usr/lib/systemd/system/tlshd.service -rwxr-xr-x 1 root root 50440 Apr 24 02:00 /usr/sbin/tlshd drwxr-xr-x 2 root root 0 Apr 24 02:00 /usr/share/doc/ktls-utils -rw-r--r-- 1 root root 2140 Apr 5 17:24 /usr/share/doc/ktls-utils/README.md -rw-r--r-- 1 root root 1742 Apr 5 17:24 /usr/share/doc/ktls-utils/SECURITY.md drwxr-xr-x 2 root root 0 Apr 24 02:00 /usr/share/licenses/ktls-utils -rw-r--r-- 1 root root 17994 Apr 5 17:24 /usr/share/licenses/ktls-utils/COPYING -rw-r--r-- 1 root root 1420 Apr 5 17:24 /usr/share/man/man5/tlshd.conf.5.gz -rw-r--r-- 1 root root 1387 Apr 5 17:24 /usr/share/man/man8/tlshd.8.gz File layout and permission are Ok. $ rpm -q --requires -p ../RPMS/x86_64/ktls-utils-0.8-1.fc39.x86_64.rpm | sort -f | uniq -c 3 /bin/sh 1 config(ktls-utils) = 0.8-1.fc39 1 libc.so.6()(64bit) 1 libc.so.6(GLIBC_2.2.5)(64bit) 1 libc.so.6(GLIBC_2.3.4)(64bit) 1 libc.so.6(GLIBC_2.33)(64bit) 1 libc.so.6(GLIBC_2.34)(64bit) 1 libc.so.6(GLIBC_2.4)(64bit) 1 libglib-2.0.so.0()(64bit) 1 libgnutls.so.30()(64bit) 1 libgnutls.so.30(GNUTLS_3_4)(64bit) 1 libgnutls.so.30(GNUTLS_3_6_9)(64bit) 1 libgnutls.so.30(GNUTLS_3_7_3)(64bit) 1 libkeyutils.so.1()(64bit) 1 libkeyutils.so.1(KEYUTILS_0.3)(64bit) 1 libkeyutils.so.1(KEYUTILS_1.5)(64bit) 1 libnl-3.so.200()(64bit) 1 libnl-3.so.200(libnl_3)(64bit) 1 libnl-genl-3.so.200()(64bit) 1 libnl-genl-3.so.200(libnl_3)(64bit) 1 rpmlib(CompressedFileNames) <= 3.0.4-1 1 rpmlib(FileDigests) <= 4.6.0-1 1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 1 rpmlib(PayloadIsZstd) <= 5.4.18-1 1 rtld(GNU_HASH) Binary requires are Ok. $ rpm -q --provides -p ../RPMS/x86_64/ktls-utils-0.8-1.fc39.x86_64.rpm | sort -f | uniq -c 1 config(ktls-utils) = 0.8-1.fc39 1 ktls-utils = 0.8-1.fc39 1 ktls-utils(x86-64) = 0.8-1.fc39 Binary provides are Ok. $ resolvedeps rawhide ../RPMS/x86_64/ktls-utils-0.8-1.fc39.x86_64.rpm Binary dependencies are resolvable. Ok. Otherwise, this package is in line with Fedora packaging guidelines. Please correct all FIX items, consider fixing TODO items, and provide an updated spec file.
Thanks! I've taken your advice: https://jlayton.fedorapeople.org/ktls-utils/0.8/ktls-utils-0.8-5.fc39.src.rpm https://jlayton.fedorapeople.org/ktls-utils/0.8/ktls-utils.spec
Created attachment 1959620 [details] The .spec file difference from Copr build 5844323 to 5845933
Copr build: https://copr.fedorainfracloud.org/coprs/build/5845933 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2182151-ktls-utils/fedora-rawhide-x86_64/05845933-ktls-utils/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
(In reply to Petr Pisar from comment #17) > FIX: These licenses are missing from a License tag: > GPL-2.0-only OR BSD-3-Clause: src/tlshd/netlink.h (Fedora ignores > Linux-syscall-note "exception" So this is a peculiar case. This header is machine-generated by a kernel script (tools/net/ynl/ynl-regen.sh) which inserts that SPDX tag in the hope that it is a broad enough license to satisfy any open source project requirements. This file is copied directly from the Linux kernel (include/uapi/linux/handshake.h) without alteration because we haven't found a way of referring the ktls-utils build infrastructure to a Linux kernel uapi header. Eventually src/tlshd/netlink.h is to be directly generated in user space via the same script using a netlink spec source file. In other words, the license of this file is not directly under the control of ktls-utils. I do not believe it is proper to alter the SPDX tag on this file, and the ynl-regen.sh tool does not provide an option to specify or alter the SPDX tag it inserts in generated files.
Spec: https://jlayton.fedorapeople.org/ktls-utils/0.8/ktls-utils.spec SRPM: https://jlayton.fedorapeople.org/ktls-utils/0.8/ktls-utils-0.8-6.fc39.src.rpm
Created attachment 1959627 [details] The .spec file difference from Copr build 5845933 to 5846225
Copr build: https://copr.fedorainfracloud.org/coprs/build/5846225 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2182151-ktls-utils/fedora-rawhide-x86_64/05846225-ktls-utils/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
This is a reasonable request, but it'll take a bit longer: FIX: The daemon does not respect distribution-wide crypto policies. It enables algorithms in tlshd_make_priorities_string() based on what Linux supported at build time of this package. Ideally the daemon should consult crypto policy <https://docs.fedoraproject.org/en-US/packaging-guidelines/CryptoPolicies/> and only enable a disjunction of what Linux offers and what user-space crypto policy mandates. Please contact <security.org> for help. There is a possibility that Linux already does that in other way. Please get a crypto review from the security team on that mailing list. For this, I think we need to vet each cipher and only enable the ones that are in the current priority list. That might be doable via gnutls_priority_cipher_list(3) but I'll need to experiment.
(In reply to Chuck Lever from comment #21) > (In reply to Petr Pisar from comment #17) > > FIX: These licenses are missing from a License tag: > > GPL-2.0-only OR BSD-3-Clause: src/tlshd/netlink.h (Fedora ignores > > Linux-syscall-note "exception" > > So this is a peculiar case. > > This header is machine-generated by a kernel script > (tools/net/ynl/ynl-regen.sh) which inserts that SPDX tag in the hope that it > is a broad enough license to satisfy any open source project requirements. > > This file is copied directly from the Linux kernel > (include/uapi/linux/handshake.h) without alteration because we haven't found > a way of referring the ktls-utils build infrastructure to a Linux kernel > uapi header. Eventually src/tlshd/netlink.h is to be directly generated in > user space via the same script using a netlink spec source file. > > In other words, the license of this file is not directly under the control > of ktls-utils. I do not believe it is proper to alter the SPDX tag on this > file, and the ynl-regen.sh tool does not provide an option to specify or > alter the SPDX tag it inserts in generated files. I know the file comes from Linux. That's not a problem. In Fedora we only need to list all licenses that come into the bits stored in a binary RPM package. Hence changing "License: GPL-2.0-only" in ktls-utils.spec to "License: GPL-2.0-only AND (GPL-2.0-only OR BSD-3-Clause) AND GPL-1.0-or-later" is enough. Please update the License tag in the spec file. There is no need to change the sources. Otherwise, the updated spec file looks good.
Thanks. I fixed the license and uploaded new package and specfile: Spec: https://jlayton.fedorapeople.org/ktls-utils/0.8/ktls-utils.spec SRPM: https://jlayton.fedorapeople.org/ktls-utils/0.8/ktls-utils-0.8-7.fc39.src.rpm I'll be looking later today at what we can do to ensure that we follow local crypto policy.
Copr build: https://copr.fedorainfracloud.org/coprs/build/5848168 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2182151-ktls-utils/srpm-builds/05848168/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Let's try that again: Spec: https://jlayton.fedorapeople.org/ktls-utils/0.8/ktls-utils.spec SRPM: https://jlayton.fedorapeople.org/ktls-utils/0.8/ktls-utils-0.8-7.fc39.src.rpm
Copr build: https://copr.fedorainfracloud.org/coprs/build/5848169 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2182151-ktls-utils/fedora-rawhide-x86_64/05848169-ktls-utils/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
I've posted a patch to make tlshd respect the distro-wide crypto policy: https://github.com/oracle/ktls-utils/pull/10 This should also make the daemon more efficient (by using cached gnutls_priority_t objects).
Chuck took in the patch to fix the cipher selection to use local preferences. I've updated the package to what is effectively a 0.9 release candidate, so that should address another "Fix" : Spec: https://jlayton.fedorapeople.org/ktls-utils/0.8/ktls-utils.spec SRPM: https://jlayton.fedorapeople.org/ktls-utils/0.8/ktls-utils-0.8%5e20200426.g29431d9e988f-1.fc39.src.rpm
Created attachment 1960185 [details] The .spec file difference from Copr build 5848169 to 5852553
Copr build: https://copr.fedorainfracloud.org/coprs/build/5852553 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2182151-ktls-utils/fedora-rawhide-x86_64/05852553-ktls-utils/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Updated to latest 0.9 prerelease. This has a number of coverity fixes in it as well: Spec: https://jlayton.fedorapeople.org/ktls-utils/0.8/ktls-utils.spec SRPM: https://jlayton.fedorapeople.org/ktls-utils/0.8/ktls-utils-0.8%5e20230516.gc60fab91ef83-1.fc39.src.rpm
Created attachment 1964893 [details] The .spec file difference from Copr build 5852553 to 5922654
Copr build: https://copr.fedorainfracloud.org/coprs/build/5922654 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2182151-ktls-utils/fedora-rawhide-x86_64/05922654-ktls-utils/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Petr can you help interpret the Fedora review service report? In particular: I've tested a mockbuild and it worked: [!]: Reviewer should test that the package builds in mock. Not sure what this means: ktls-utils.spec:50: W: setup-not-quiet I think we've addressed this in the latest: ktls-utils.x86_64: W: crypto-policy-non-compliance-gnutls-2 /usr/sbin/tlshd gnutls_priority_init We do use gnutls_priority_init, but we have to be able to limit the daemon to the crypto algorithms supported by the kernel. We should now respect the distro's crypto preference ordering however. I also emailed security.org a couple of weeks ago to request a review of the crypto usage in this package, but I got zero responses. Is there a more formal way to request this than just nagging a mailing list?
(In reply to Jeff Layton from comment #38) > Petr can you help interpret the Fedora review service report? In particular: > > I've tested a mockbuild and it worked: > [!]: Reviewer should test that the package builds in mock. > That's it. I will check whether it builds by submitting a scratch build to Koji. > Not sure what this means: > ktls-utils.spec:50: W: setup-not-quiet > Probably that %setup macro is missing -q argument. > I think we've addressed this in the latest: > ktls-utils.x86_64: W: crypto-policy-non-compliance-gnutls-2 /usr/sbin/tlshd > gnutls_priority_init > > We do use gnutls_priority_init, but we have to be able to limit the daemon > to the crypto algorithms supported by the kernel. We should now respect the > distro's crypto preference ordering however. > The check is a simple search and found in undefined ELF symbols. The rpmlint warning can be later overridden with ktls-utils.rpmlinrc configuration file in the dist-git repository. > I also emailed security.org a couple of weeks ago to > request a review of the crypto usage in this package, but I got zero > responses. Is there a more formal way to request this than just nagging a > mailing list? I also did not get any response. I don't know any more official contact. You can try mailing crypto-policies package maintainers.
FIX: The ktls-utils-0.8.tar.gz archive (SHA512 9da04aa8cdbb34193cd26a7bb882bd7f02d4fc2c5065ff1088112057b6e14f3ab6a926356366ab7c0827693b90465d5bb398eb7c5ab722a19c1c7ac2279fd3d3) from ktls-utils-0.8%5e20230516.gc60fab91ef83-1.fc39.src.rpm does not match the upstream tar ball (SHA512: 027824a8ffb42bf8b39ce8d8a83f8f3d0c3d2e6cd0c2867f622e04ce914f578767ce7803617fe922c44a5fb5e69636efc6c0fc1726be1a3852b41cb6ad7579eb). THIRD_PARTY_LICENSES is correctly unpackaged. No LGPL code exists in the sources. License tag is Ok. Regarding the global crypto policy conformance, I have doubts about: pstring = strdup("SECURE256:+SECURE128:-COMP-ALL"); in tlshd_gnutls_priority_init(). Subsequent for-cycle with: pstring = tlshd_cipher_string_emit(pstring, ciphers[i]); adds ciphersuits common to default GnuTLS set and Linux set. I worry that in effect it means all 256-bit, all 128-bit suits and all the common suits. What if the global policy excluded 128-bit suits? $ rpmlint ktls-utils.spec ../SRPMS/ktls-utils-0.8^20230516.gc60fab91ef83-1.fc39.src.rpm ../RPMS/x86_64/ktls-utils-* ======================================== rpmlint session starts ======================================= rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 31, packages: 5 ktls-utils.spec:40: W: setup-not-quiet ktls-utils.spec:40: W: setup-not-quiet ktls-utils.x86_64: W: crypto-policy-non-compliance-gnutls-2 /usr/sbin/tlshd gnutls_priority_init ========= 4 packages and 1 specfiles checked; 0 errors, 3 warnings, 0 badness; has taken 0.3 s ======== rpmlint is Ok. "setup-not-quiet" means that %autosetup lists unpacked files, which indeed does not, but that's against RPM documentation <https://rpm-software-management.github.io/rpm/manual/autosetup.html#autosetup-options>. I guess it's a bug in current rpmbuild. FIX: The package fails to build Fedora 39 on i686 (https://koji.fedoraproject.org/koji/taskinfo?taskID=101246895): config.c:155:52: error: comparison of integer expressions of different signedness: '__off_t' {aka 'long int'} and 'unsigned int' [-Werror=sign-compare] 155 | if (statbuf.st_size < 0 || statbuf.st_size > UINT_MAX) { | ^ Please correct the FIX items and provide an updated spec file. Then I will approve this package. I think the crypto policy details can be improved later.
Spec: https://jlayton.fedorapeople.org/ktls-utils/0.9/ktls-utils.spec SRPM: https://jlayton.fedorapeople.org/ktls-utils/0.9/ktls-utils-0.9-1.fc39.src.rpm I updated the package to the 0.9 release. I'm pretty sure that the things in comment #40 are not yet addressed though.
Created attachment 1969914 [details] The .spec file difference from Copr build 5922654 to 6036690
Copr build: https://copr.fedorainfracloud.org/coprs/build/6036690 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2182151-ktls-utils/fedora-rawhide-x86_64/06036690-ktls-utils/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Source0 URL is Ok. Source archive (SHA-512: cee06b1f50574fb338be29dd39befe460fc7d80c3b1097954fc172bb3c28e855428845f16f161506763d81ae397e977e897d81c42f059870c2d858b9623c77e3) is original. Ok. New licenses found: aclocal.m4: FSFULLRWD AND GPL-2.0-or-later WITH Autoconf-exception-generic AND FSFULLR compile: GPL-2.0-or-later WITH Autoconf-exception-generic configure: FSFULLR install-sh: X11 Makefile.in, src/Makefile.in, src/tlshd/Makefile.in, systemd/Makefile.in: FSFULLRWD AND GPL-2.0-only missing: GPL-2.0-or-later WITH Autoconf-exception-generic Changed licenses: README, README.md: GPL-1.0-or-later → GPL-2.0-only Removed licenses: THIRD_PARTY_LICENSES: LGPL-2.1 text FIX: Remove "AND GPL-1.0-or-later" from a License tag because README.md changed the license. TODO: Remove "%autopatch -p1". from %prep section. There are no patches. $ rpmlint ktls-utils.spec ../SRPMS/ktls-utils-0.9-1.fc39.src.rpm ../RPMS/x86_64/ktls-utils-0.9-1.fc39.x86_64.rpm ======================================== rpmlint session starts ======================================= rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 31, packages: 3 ktls-utils.x86_64: W: crypto-policy-non-compliance-gnutls-2 /usr/sbin/tlshd gnutls_priority_init ========= 2 packages and 1 specfiles checked; 0 errors, 1 warnings, 0 badness; has taken 0.2 s ======== rpmlint is Ok. FIX: The package fails to build on i686 in F39 (https://koji.fedoraproject.org/koji/taskinfo?taskID=101969239) with the same error. Either fix it, or exclude building from 32-bit x86 architectures. Please correct the FIX items, consider fixing TODO items, and provide an updates spec file.
(In reply to Petr Pisar from comment #44) > Source0 URL is Ok. > Source archive (SHA-512: > cee06b1f50574fb338be29dd39befe460fc7d80c3b1097954fc172bb3c28e855428845f16f161 > 506763d81ae397e977e897d81c42f059870c2d858b9623c77e3) is original. Ok. > > New licenses found: > aclocal.m4: FSFULLRWD AND GPL-2.0-or-later WITH Autoconf-exception-generic > AND FSFULLR > compile: GPL-2.0-or-later WITH Autoconf-exception-generic > configure: FSFULLR > install-sh: X11 > Makefile.in, src/Makefile.in, src/tlshd/Makefile.in, systemd/Makefile.in: > FSFULLRWD AND GPL-2.0-only > missing: GPL-2.0-or-later WITH Autoconf-exception-generic > > Changed licenses: > README, README.md: GPL-1.0-or-later → GPL-2.0-only > > Removed licenses: > THIRD_PARTY_LICENSES: LGPL-2.1 text > > FIX: Remove "AND GPL-1.0-or-later" from a License tag because README.md > changed the license. > > TODO: Remove "%autopatch -p1". from %prep section. There are no patches. Unless this is perfect code... There will be patches... :-) > > $ rpmlint ktls-utils.spec ../SRPMS/ktls-utils-0.9-1.fc39.src.rpm > rpmlint is Ok. > > > FIX: The package fails to build on i686 in F39 > (https://koji.fedoraproject.org/koji/taskinfo?taskID=101969239) with the > same error. Either fix it, or exclude building from 32-bit x86 architectures. This is a process error!!! Traceback (most recent call last): File "/usr/lib/python3.11/site-packages/mockbuild/trace_decorator.py", line 93, in trace result = func(*args, **kw) ^^^^^^^^^^^^^^^^^ Has nothing to do with the actual package!!! There is no python in the package so how can python cause a build to fail??? > > Please correct the FIX items, consider fixing TODO items, and provide an > updates spec file. Please reconsider approving this package... It should not be this difficult to get a truly need package approved... IMHO
Steve, I cannot approve a package which breaks the packaging guidelines. One of the requirement is that the package builds in Koji for Rawhide. And that this package does not meet. The python trace is just sloppy way to express that the rpmbuild invocation failed. Scanning build.log before that reveals this error: make[3]: Entering directory '/builddir/build/BUILD/ktls-utils-0.9/src/tlshd' gcc -DHAVE_CONFIG_H -I. -I../.. -Werror -Wall -Wextra -I/usr/include/p11-kit-1 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -I/usr/include/sysprof-4 -pthread -I/usr/include/libnl3 -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=3 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m32 -march=i686 -mtune=generic -msse2 -mfpmath=sse -mstackrealign -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -c -o tlshd-config.o `test -f 'config.c' || echo './'`config.c make[3]: Leaving directory '/builddir/build/BUILD/ktls-utils-0.9/src/tlshd' config.c: In function 'tlshd_config_read_datum': config.c:155:52: error: comparison of integer expressions of different signedness: '__off_t' {aka 'long int'} and 'unsigned int' [-Werror=sign-compare] 155 | if (statbuf.st_size < 0 || statbuf.st_size > UINT_MAX) { | ^ cc1: all warnings being treated as errors make[3]: *** [Makefile:478: tlshd-config.o] Error 1 Another item is that the license tag is accurate. Listing a license which is not in the code violates it. I'm really sorry, but it's the packager's due to address these items. I are blaming a wrong person.
s/I are/You are/
Cc'ing @ssorce to see if he can help us with a security review of the new ktls upcall handler and associated code.
The type comparison seems easy enough to fix: https://lore.kernel.org/kernel-tls-handshake/20230620172014.1178395-1-jlayton@kernel.org/T/#u Hopefully Chuck will pull that in soon.
New SRPM with the patch in comment #49. Does this fix the i686 issue? Spec: https://jlayton.fedorapeople.org/ktls-utils/0.9/ktls-utils.spec SRPM: https://jlayton.fedorapeople.org/ktls-utils/0.9/ktls-utils-0.9-2.fc39.src.rpm
Created attachment 1971753 [details] The .spec file difference from Copr build 6036690 to 6098709
Copr build: https://copr.fedorainfracloud.org/coprs/build/6098709 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2182151-ktls-utils/fedora-rawhide-x86_64/06098709-ktls-utils/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Assuming that patch fixes i686, I think that should (hopefully) just leave the licensing issue. The report says: FIX: Remove "AND GPL-1.0-or-later" from a License tag because README.md changed the license. When I look in the 0.9 tree though: $ git grep "AND GPL-1.0-or-later" ...comes back with nothing. I'm unclear on what needs to be fixed here.
Doh! Nevermind. I figured this out. I just needed to fix the line in the specfile: Spec: https://jlayton.fedorapeople.org/ktls-utils/0.9/ktls-utils.spec SRPM: https://jlayton.fedorapeople.org/ktls-utils/0.9/ktls-utils-0.9-3.fc39.src.rpm
Created attachment 1971756 [details] The .spec file difference from Copr build 6098709 to 6098733
Copr build: https://copr.fedorainfracloud.org/coprs/build/6098733 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2182151-ktls-utils/fedora-rawhide-x86_64/06098733-ktls-utils/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
$ rpmlint ktls-utils.spec ../SRPMS/ktls-utils-0.9-1.fc39.src.rpm ../RPMS/x86_64/ktls-utils-* ======================================== rpmlint session starts ======================================= rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 31, packages: 5 ktls-utils.x86_64: W: crypto-policy-non-compliance-gnutls-2 /usr/sbin/tlshd gnutls_priority_init ========= 4 packages and 1 specfiles checked; 0 errors, 1 warnings, 0 badness; has taken 0.3 s ======== rpmlint is Ok. The license tag is correct. Ok. The package builds in F39 (https://koji.fedoraproject.org/koji/taskinfo?taskID=102415514). Ok. Thanks for your hard work on this package. This package is APPROVED.
Given this code is using GnuTLS underneath I am pulling in Daiki to take a look. Daiki, can you check that cert verification is properly handled, I see they provide a custom function.
At first glance, the certificate verification looks good to me, except that I'm unsure what HANDSHAKE_AUTH_UNAUTH actually means: is it a certificate authentication without checking the result, or actual anonymous authentication that can be done with the gnutls_anon_* API? If the answer is the format, we might want to disable it by default. I can take a closer look later.
(In reply to Daiki Ueno from comment #59) > At first glance, the certificate verification looks good to me, except that > I'm unsure what HANDSHAKE_AUTH_UNAUTH actually means: is it a certificate > authentication without checking the result, or actual anonymous > authentication that can be done with the gnutls_anon_* API? If the answer is > the format, we might want to disable it by default. The point of the "anon" authentication type is to use x.509 but only require that the client authenticate the server. The client then does not need an x.509 certificate -- it's one way to enable a private connection without needing to distribute authentication material to perhaps many thousands of clients. The use of this mode is controlled by the administrators of both the client and server -- they can require encryption and/or authentication before the client is permitted access to the server's data, or allow the use of encryption if it's available on both sides. I would rather allow administrators to control whether this option is available.
On a slightly different note, if the netlink interface is not fixed, it might make things simpler to use the gnutls_handshake* API (gnutls_handshake_write and gnutls_handshake_set_read_function) and drive the handshake state machine directly. That way the record protocol (regardless of TLS or DTLS) could be sorely handled in the kernel.
(In reply to Daiki Ueno from comment #61) > On a slightly different note, if the netlink interface is not fixed, it > might make things simpler to use the gnutls_handshake* API > (gnutls_handshake_write and gnutls_handshake_set_read_function) and drive > the handshake state machine directly. That way the record protocol > (regardless of TLS or DTLS) could be sorely handled in the kernel. I think there is some flexibility in the netlink API, and we are interested in supporting DTLS eventually (as well as handling QUICv1 handshakes). The first two consumers (RPC and NVMe/TCP) needed TLSv1.3 so that is what we started with. We have a mailing list: <kernel-tls-handshake.dev> You are very welcome to post patches or suggestions there.
The Pagure repository was created at https://src.fedoraproject.org/rpms/ktls-utils
FEDORA-2023-95ca7514d7 has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2023-95ca7514d7
FEDORA-2023-95ca7514d7 has been pushed to the Fedora 39 stable repository. If problem still persists, please make note of it in this bug report.
I've also added branches for Fedora 38 and EPEL9 and done builds for them. They should show up in the testing repos real soon now.