Spec URL: https://copr-be.cloud.fedoraproject.org/results/gtb/libfido2/fedora-31-x86_64/01121536-libfido2/libfido2.spec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/gtb/libfido2/fedora-31-x86_64/01121536-libfido2/libfido2-1.3.0-1.fc31.src.rpm Description: libfido2 is an open source library to support the FIDO2 protocol. FIDO2 is an open authentication standard that consists of the W3C Web Authentication specification (WebAuthn API), and the Client to Authentication Protocol (CTAP). CTAP is an application layer protocol used for communication between a client (browser) or a platform (operating system) with an external authentication device (for example the Yubico Security Key). Fedora Account System Username: gtb Benefit to Fedora: In addition to the ability to manage and utilize U2F/FIDO2 authenticators, this package will allow OpenSSH to use a U2F authenticator for storage/protection of ssh keys. This package compliments the python3-fido package. This is my first Fedora package, and I need a sponsor. I am not an upstream maintainer of the libfido2 project, although I have submitted issues/patches that have been accepted. I am a packager in rpmfusion for the xmltv package. The package is available in Fedora Copr (as the provided spec/srpm references show) at: https://copr.fedorainfracloud.org/coprs/gtb/libfido2/ Note: the libfido2 package ships a "naked" .so library file libsk-libfido2.so (in addition to versioned libraries for other uses) for integration with OpenSSH (see https://marc.info/?l=openssh-unix-dev&m=157259802529972&w=2 for the how-to). This naked .so file appears to technically be in violation of one of the MUSTs in packaging if a -devel library is shipped. The alternative would seem to be to not ship a -devel library.
Thank you for working on this; please find below the review. Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated Issues: ======= - ldconfig not called in %post and %postun for Fedora 28 and later. Note: /sbin/ldconfig called in libfido2 See: https://fedoraproject.org/wiki/Changes/Removing_ldconfig_scriptlets - Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files directly in %_libdir. See: https://docs.fedoraproject.org/en-US/packaging- guidelines/#_devel_packages -> I think this is okay, as it's a runtime library that applications (e.g. OpenSSH) use. - Static libraries in -static or -devel subpackage, providing -devel if present. Note: Package has .a files: libfido2-devel. Does not provide -static: libfido2-devel. See: https://docs.fedoraproject.org/en-US/packaging- guidelines/#packaging-static-libraries -> Please remove .a files in %build or %install. - fido2-tools should have a fully versioned dependency on libfido2. I'm also not sure if this explicit naming is ok; I would rather use libfido2-tools. - It would be good to bundle the gpg signature and verify it at %prep. ===== MUST items ===== C/C++: [-]: Provides: bundled(gnulib) in place as required. Note: Sources not installed [-]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Header files in -devel subpackage, if present. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: There is no build directory. Running licensecheck on vanilla upstream sources. No licenses found. Please check the source files for licenses manually. [x]: License file installed when any subpackage combination is installed. [x]: Package does not own files or directories owned by other packages. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package must not depend on deprecated() packages. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [!]: Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in fido2-tools [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Scriptlets must be sane, if used. [!]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [?]: Package should compile and build into binary rpms on all supported architectures. [-]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: The placement of pkgconfig(.pc) files are correct. [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM.
(In reply to Daiki Ueno from comment #1) > Thank you for working on this; please find below the review. Thanks for your review (sorry for taking so long to reply, life happens). Comments inline follow: > Issues: > ======= > - ldconfig not called in %post and %postun for Fedora 28 and later. Accepted/Completed - My only (weak) defense is that the existing ldconfig %post/%postun was a legacy of starting this before the new file trigger support was in place. I should have seen it (I did in other packages I maintain). > - Development (unversioned) .so files in -devel subpackage, if present. > ... > -> I think this is okay, as it's a runtime library that applications > (e.g. OpenSSH) use. Acknowledged, will remain. > - Static libraries in -static or -devel subpackage, providing -devel if > present. > .... > -> Please remove .a files in %build or %install. Accepted/Completed (.a files removed in %install) > - fido2-tools should have a fully versioned dependency on libfido2. Accepted/Completed (requirements updated) > I'm also not sure if this explicit naming is ok; I would rather > use libfido2-tools. I selected the name to be compatible with the debian packages that upstream provides. My initial thought was name compatibility was likely desirable to try to minimize unnecessary distro differences. If you prefer libfido2-tools I can make that change, but I have not for this spec update. I await further feedback. > - It would be good to bundle the gpg signature and verify it at %prep. Currently the project does not sign releases. I have opened an issue with the project requesting that they do include signatures for future releases. As there is currently not a verifiable project release key, as I understand the packaging guidelines, this is not currently required. ---- In addition to the requested changes, a new rpm requirement (that was self-identified as missing) of u2f-hidraw-policy was added to the package to help udev properly set permissions on u2f keys at insert. Current spec/SRPMs: Spec URL: https://copr-be.cloud.fedoraproject.org/results/gtb/libfido2/fedora-31-x86_64/01129329-libfido2/libfido2.spec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/gtb/libfido2/fedora-31-x86_64/01129329-libfido2/libfido2-1.3.0-2.fc31.src.rpm
(In reply to Gary Buhrmaster from comment #2) Thank you for the update; the updated spec looks good to me. I realized that I missed that the FE-NEEDSPONSOR flag was set while I don't have the privilege. Tomas, could you take a look perhaps? > > I'm also not sure if this explicit naming is ok; I would rather > > use libfido2-tools. > > I selected the name to be compatible with the debian packages that upstream > provides. My initial thought was name compatibility was likely desirable to > try to minimize unnecessary distro differences. If you prefer > libfido2-tools I can make that change, but I have not for this spec update. > > I await further feedback. Sounds reasonable to me. > > - It would be good to bundle the gpg signature and verify it at %prep. > > Currently the project does not sign releases. I have opened an issue with > the project requesting that they do include signatures for future releases. I was referring to the Yubico's release page, where .sig is also provided: https://developers.yubico.com/libfido2/Releases/
(In reply to Daiki Ueno from comment #3) > I was referring to the Yubico's release page, where .sig is also provided: > https://developers.yubico.com/libfido2/Releases/ Thanks for the pointer. I had not been aware that yubico had started releasing on their corporate site with signatures (they initially only released on github, and they still do release there, but without and sigs). I'll work to revise the sources/spec files to pull the sources from yubico corporate site and validate the signing from there. Thanks.
Updated SPEC/SRPMSs with gpg signature verification added: SPEC URL: https://copr-be.cloud.fedoraproject.org/results/gtb/libfido2/fedora-31-x86_64/01129960-libfido2/libfido2.spec SRPM URL: https://copr-be.cloud.fedoraproject.org/results/gtb/libfido2/fedora-31-x86_64/01129960-libfido2/libfido2-1.3.0-3.fc31.src.rpm
Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed ===== MUST items ===== C/C++: [x]: Package does not contain kernel modules. [x]: Package contains no static executables. [x]: If your application is a C or C++ application you must list a BuildRequires against gcc, gcc-c++ or clang. [x]: Header files in -devel subpackage, if present. [x]: ldconfig not called in %post and %postun for Fedora 28 and later. [x]: Package does not contain any libtool archives (.la) [x]: Rpath absent or only used for internal libs. Generic: [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "BSD 2-clause "Simplified" License", "ISC License", "ISC License BSD 2-clause "Simplified" License", "BSD (unspecified)". 181 files have unknown license. Detailed output of licensecheck in /home/mraz/rhsrc/review-libfido2/licensecheck.txt [x]: License file installed when any subpackage combination is installed. [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [x]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 10240 bytes in 2 files. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package must not depend on deprecated() packages. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local ===== SHOULD items ===== Generic: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [?]: Package functions as described. [x]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [x]: Sources are verified with gpgv2. [-]: Description and summary sections in the package spec file contains translations for supported Non-English languages, if available. [?]: Package should compile and build into binary rpms on all supported architectures. [-]: %check is present and all tests pass. [?]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Fully versioned dependency in subpackages if applicable. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: The placement of pkgconfig(.pc) files are correct. [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on debuginfo package(s). Note: No rpmlint messages. [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM. Rpmlint ------- Checking: libfido2-1.3.0-3.fc32.x86_64.rpm libfido2-devel-1.3.0-3.fc32.x86_64.rpm fido2-tools-1.3.0-3.fc32.x86_64.rpm libfido2-debuginfo-1.3.0-3.fc32.x86_64.rpm libfido2-debugsource-1.3.0-3.fc32.x86_64.rpm libfido2-1.3.0-3.fc32.src.rpm libfido2.x86_64: W: no-soname /usr/lib64/libsk-libfido2.so 6 packages and 0 specfiles checked; 0 errors, 1 warnings. This is OK because it is a plugin, not a regular library, although this ssh plugin is not usable with the openssh version we have in Fedora it is possible to use it with upstream openssh snapshots. Rpmlint (debuginfo) ------------------- Checking: libfido2-debuginfo-1.3.0-3.fc32.x86_64.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. Rpmlint (installed packages) ---------------------------- libfido2-debuginfo.x86_64: W: invalid-url URL: https://github.com/Yubico/libfido2 <urlopen error [Errno -2] Name or service not known> libfido2.x86_64: W: invalid-url URL: https://github.com/Yubico/libfido2 <urlopen error [Errno -2] Name or service not known> libfido2.x86_64: W: no-soname /usr/lib64/libsk-libfido2.so libfido2-devel.x86_64: W: invalid-url URL: https://github.com/Yubico/libfido2 <urlopen error [Errno -2] Name or service not known> fido2-tools.x86_64: W: invalid-url URL: https://github.com/Yubico/libfido2 <urlopen error [Errno -2] Name or service not known> libfido2-debugsource.x86_64: W: invalid-url URL: https://github.com/Yubico/libfido2 <urlopen error [Errno -2] Name or service not known> 5 packages and 0 specfiles checked; 0 errors, 6 warnings. This is OK Unversioned so-files -------------------- libfido2: /usr/lib64/libsk-libfido2.so This is OK Source checksums ---------------- https://developers.yubico.com/libfido2/Releases/libfido2-1.3.0.tar.gz.sig : CHECKSUM(SHA256) this package : f550ce8ce8a6297d0034a81a42156c78f4934916a58d10a5209434e5061c0786 CHECKSUM(SHA256) upstream package : f550ce8ce8a6297d0034a81a42156c78f4934916a58d10a5209434e5061c0786 https://developers.yubico.com/libfido2/Releases/libfido2-1.3.0.tar.gz : CHECKSUM(SHA256) this package : 0b2e3671c4c5d42fd5604a08e45f89f49592b97cf66d7d3bfbc7e6a4d5a0fec7 CHECKSUM(SHA256) upstream package : 0b2e3671c4c5d42fd5604a08e45f89f49592b97cf66d7d3bfbc7e6a4d5a0fec7 Requires -------- libfido2 (rpmlib, GLIBC filtered): ld-linux-x86-64.so.2()(64bit) libc.so.6()(64bit) libcbor.so.0()(64bit) libcrypto.so.1.1()(64bit) libcrypto.so.1.1(OPENSSL_1_1_0)(64bit) libcrypto.so.1.1(OPENSSL_1_1_1)(64bit) libfido2.so.1()(64bit) libudev.so.1()(64bit) libudev.so.1(LIBUDEV_183)(64bit) rtld(GNU_HASH) u2f-hidraw-policy libfido2-devel (rpmlib, GLIBC filtered): /usr/bin/pkg-config libfido2(x86-64) libfido2.so.1()(64bit) pkgconfig(libcrypto) fido2-tools (rpmlib, GLIBC filtered): libc.so.6()(64bit) libcrypto.so.1.1()(64bit) libcrypto.so.1.1(OPENSSL_1_1_0)(64bit) libfido2(x86-64) libfido2.so.1()(64bit) rtld(GNU_HASH) libfido2-debuginfo (rpmlib, GLIBC filtered): libfido2-debugsource (rpmlib, GLIBC filtered): Provides -------- libfido2: libfido2 libfido2(x86-64) libfido2.so.1()(64bit) libsk-libfido2.so()(64bit) libfido2-devel: libfido2-devel libfido2-devel(x86-64) pkgconfig(libfido2) fido2-tools: fido2-tools fido2-tools(x86-64) libfido2-debuginfo: debuginfo(build-id) libfido2-debuginfo libfido2-debuginfo(x86-64) libfido2-debugsource: libfido2-debugsource libfido2-debugsource(x86-64) Generated by fedora-review 0.7.4 (54fa030) last change: 2019-12-07 Command line :/usr/bin/fedora-review -u https://copr-be.cloud.fedoraproject.org/results/gtb/libfido2/fedora-31-x86_64/01129960-libfido2/ Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, C/C++, Shell-api Disabled plugins: Perl, fonts, Ocaml, R, Python, PHP, Java, SugarActivity, Haskell Disabled flags: EPEL6, EPEL7, DISTTAG, BATCH, EXARCH Package is ACCEPTed.
I've sponsored you into the packager group.
Gary what's the status of getting this imported/built?
My apologies, I got distracted, and somehow missed the email that I got sponsored. I will try to start the next steps this week.
(In reply to Gary Buhrmaster from comment #9) > My apologies, I got distracted, and somehow missed the email that I got > sponsored. I will try to start the next steps this week. No issues, I request the repo be created, ticket here: https://pagure.io/releng/fedora-scm-requests/issue/22235
(In reply to Peter Robinson from comment #10) > > No issues, I request the repo be created, ticket here: > https://pagure.io/releng/fedora-scm-requests/issue/22235 It got closed. Better if Gary opens a new ticket himself.
It needs to be assigned to the reviewer!
(In reply to Tomas Mraz from comment #11) > (In reply to Peter Robinson from comment #10) > > It got closed. Better if Gary opens a new ticket himself. Opened as: https://pagure.io/releng/fedora-scm-requests/issue/22260
(fedscm-admin): The Pagure repository was created at https://src.fedoraproject.org/rpms/libfido2
Bodhi says that the package has been pushed to stable for rawhide and f32 (and I have submitted a bodhi request to make the package available for f30 and f31). f30 bodhi status: https://bodhi.fedoraproject.org/updates/FEDORA-2020-46be289c16 f31 bodhi status: https://bodhi.fedoraproject.org/updates/FEDORA-2020-2c9f72bbaa As I understand the new package process (and as this is my first package I am still muddling through), that means it is now appropriate to close this new package request ticket (and I will). btw, yubico released a minor patch update (1.3.1 from 1.3.0) to pull in a few minor changes just yesterday. I'll try to get to creating an update when they post the signature on their official release site (it was not yet available yesterday). Thank you to everyone that helped in getting this new package request complete.
Clearing the "needinfo" request, as I believe this is now resolved.