Bug 2231209 - Review Request: openvpn-otp - OpenVPN OTP authentication support
Summary: Review Request: openvpn-otp - OpenVPN OTP authentication support
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Petr Pisar
QA Contact: Fedora Extras Quality Assurance
URL: https://github.com/evgeny-gridasov/%{...
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-08-11 02:40 UTC by Spike
Modified: 2023-08-16 09:37 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2023-08-16 09:37:40 UTC
Type: ---
Embargoed:
ppisar: fedora-review+


Attachments (Terms of Use)
The .spec file difference from Copr build 6265230 to 6267033 (1.80 KB, patch)
2023-08-11 17:27 UTC, Fedora Review Service
no flags Details | Diff
The .spec file difference from Copr build 6267033 to 6300331 (1.01 KB, patch)
2023-08-15 21:49 UTC, Fedora Review Service
no flags Details | Diff

Description Spike 2023-08-11 02:40:07 UTC
Spec URL: https://spike.fedorapeople.org/openvpn-otp/openvpn-otp.spec
SRPM URL: https://spike.fedorapeople.org/openvpn-otp/openvpn-otp-1.0-1.20230731git47f8ccf.fc38.src.rpm
Description: This plug-in adds support for time based OTP (totp) and HMAC based OTP (hotp) tokens for OpenVPN. Compatible with Google Authenticator software token, other software and hardware based OTP tokens.
Fedora Account System Username: spike

A copr build can be found here: https://copr.fedorainfracloud.org/coprs/spike/openvpn-otp/

Comment 1 Fedora Review Service 2023-08-11 02:47:24 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6265230
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2231209-openvpn-otp/fedora-rawhide-x86_64/06265230-openvpn-otp/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.

Comment 2 Petr Pisar 2023-08-11 11:14:15 UTC
URL and Souce0 addresses are Ok.
Source0 archive (SHA-512: b64bbe12a63c0b138909022a6232c2ce65fa7cac2a2b08cdd9a49086df4403b6d9d37fb29692301b139461bafcb787cad39feb486038d51a4304e47a2a2150e9) is original. Ok.

FIX: Follow a version scheme for snapshots <https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/#_snapshots>. I.e. "Version: 1.0^20230731git%{shortcommit}" and "Release: 1%{?dist}".

Summary verified from README.md. Ok.
TODO: Spell "Authentication" with a lower case "a".

Description verified from README.md. Ok.

Licenses found:

LICENSE: GPL-3.0 text
src/base32.c: Apache-2.0
src/base32.h: Apache-2.0
src/base64.c: APSL-2.0 AND Apache-1.0
src/base64.h: APSL-2.0 AND Apache-1.0

FATAL: Distribute APSL-2.0 license text <https://opensource.apple.com/apsl/> with the sources and within the binary RPM package. This is required by APSL-2.0 license and an upstream not doing it violates the license. Also report it to the upstream.
FIX: Extract Apache-1.0 text from src/base64.h and package it in a binary package as required by the license.
FATAL: Distribute Apache-2.0 license text <https://www.apache.org/licenses/LICENSE-2.0> with the sources and within the binary RPM package. This requires by Apache-2.0 license and an upstream not doing it violates the license. Also report it to the upstream.

FIX: Correct License tag to "GPL-1.0-or-later AND Apache-2.0 AND Apache-1.0 AND APSL-2.0". Source code does not mention which GPL version to apply, hence with in compliance with the GPL-3.0 text, any version can be used.

FIX: Build-require "bash" (autogen.sh:1).
FIX: Build-require "coreutils" (autogen.sh:2).
FIX: Build-require "autoconf" (autogen.sh:3).
FIX: Build-require "automake" (configure.ac:3).

TODO: Constrain "openssl-devel" build-dependency with ">= 1.1.0" (configure.ac:39).

FIX: Build-require "make" (openvpn-otp.spec:30).

TODO: Report to upstream that the codes uses functions (e.g. HMAC_CTX_new()) deprecated in OpenSSL 3.0.

Distribution compiler and flags are respected. Ok.
No tests, no %check phase. Ok.

$ rpmlint openvpn-otp.spec ../SRPMS/openvpn-otp-1.0-1.20230731git47f8ccf.fc40.src.rpm ../RPMS/x86_64/openvpn-otp-*
======================================== rpmlint session starts =======================================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.12/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

========= 4 packages and 1 specfiles checked; 0 errors, 0 warnings, 0 badness; has taken 0.3 s ========
rpmlint is Ok.

$ rpm -q -lv -p  ../RPMS/x86_64/openvpn-otp-1.0-1.20230731git47f8ccf.fc40.x86_64.rpm 
drwxr-xr-x    2 root     root                        0 Jul 31 02:00 /usr/lib/.build-id
drwxr-xr-x    2 root     root                        0 Jul 31 02:00 /usr/lib/.build-id/6f
lrwxrwxrwx    1 root     root                       52 Jul 31 02:00 /usr/lib/.build-id/6f/b58a8b2ab7db1e232741afef51f6b7a7f17b0c -> ../../../../usr/lib64/openvpn/plugins/openvpn-otp.so
-rwxr-xr-x    1 root     root                    32504 Jul 31 02:00 /usr/lib64/openvpn/plugins/openvpn-otp.so
drwxr-xr-x    2 root     root                        0 Jul 31 02:00 /usr/share/doc/openvpn-otp
-rw-r--r--    1 root     root                    13918 Aug  7  2021 /usr/share/doc/openvpn-otp/README.md
drwxr-xr-x    2 root     root                        0 Jul 31 02:00 /usr/share/licenses/openvpn-otp
-rw-r--r--    1 root     root                    35121 Aug  7  2021 /usr/share/licenses/openvpn-otp/LICENSE
File layout and permissions are Ok.

$ rpm -q --requires -p ../RPMS/x86_64/openvpn-otp-1.0-1.20230731git47f8ccf.fc40.x86_64.rpm | sort -f | uniq -c
      1 libc.so.6()(64bit)
      1 libc.so.6(GLIBC_2.2.5)(64bit)
      1 libc.so.6(GLIBC_2.3)(64bit)
      1 libc.so.6(GLIBC_2.3.4)(64bit)
      1 libc.so.6(GLIBC_2.4)(64bit)
      1 openvpn >= 2.0
      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/openvpn-otp-1.0-1.20230731git47f8ccf.fc40.x86_64.rpm | sort -f | uniq -c
      1 openvpn-otp = 1.0-1.20230731git47f8ccf.fc40
      1 openvpn-otp(x86-64) = 1.0-1.20230731git47f8ccf.fc40
Binary provides are Ok.

$ resolvedeps rawhide ../RPMS/x86_64/openvpn-otp-1.0-1.20230731git47f8ccf.fc40.x86_64.rpm 
Binary dependencies are resolvable. Ok.

Building in F40 Koji build target will be verified after resolving the license issues.

Otherwise, the package is in line with Fedora packaging guidelines.

Please fix the FATAL issues. Without resolving this review cannot continue and Fedora cannot distribute the packages.
Please correct all FIX items, consider fixing TODO items, and provide an updated spec file.

Comment 3 Spike 2023-08-11 17:20:26 UTC
Thanks a lot Petr for the very detailed review!

Updated spec and SRPM file can be found here:
https://spike.fedorapeople.org/openvpn-otp/openvpn-otp.spec
https://spike.fedorapeople.org/openvpn-otp/openvpn-otp-1.0%5e20230731git47f8ccf-1.fc38.src.rpm


(In reply to Petr Pisar from comment #2)
> FIX: Follow a version scheme for snapshots
> <https://docs.fedoraproject.org/en-US/packaging-guidelines/Versioning/
Updated the version form the "Traditional versioning" scheme to the current one.

> TODO: Spell "Authentication" with a lower case "a".
Done.

> FATAL: Distribute APSL-2.0 license text <https://opensource.apple.com/apsl/>
> with the sources and within the binary RPM package. This is required by
> APSL-2.0 license and an upstream not doing it violates the license. Also
> report it to the upstream.
APSL-2.0.txt was added. Opened upstream issue: https://github.com/evgeny-gridasov/openvpn-otp/issues/46

> FIX: Extract Apache-1.0 text from src/base64.h and package it in a binary
> package as required by the license.
Apache-1.0.txt was added.

> FATAL: Distribute Apache-2.0 license text
> <https://www.apache.org/licenses/LICENSE-2.0> with the sources and within
> the binary RPM package. This requires by Apache-2.0 license and an upstream
> not doing it violates the license. Also report it to the upstream.
Apache-2.0.txt was added. Opened upstream issue: https://github.com/evgeny-gridasov/openvpn-otp/issues/47

> FIX: Correct License tag to "GPL-1.0-or-later AND Apache-2.0 AND Apache-1.0
> AND APSL-2.0". Source code does not mention which GPL version to apply,
> hence with in compliance with the GPL-3.0 text, any version can be used.
Done.

> FIX: Build-require "bash" (autogen.sh:1).
> FIX: Build-require "coreutils" (autogen.sh:2).
> FIX: Build-require "autoconf" (autogen.sh:3).
> FIX: Build-require "automake" (configure.ac:3).
Done.

> TODO: Constrain "openssl-devel" build-dependency with ">= 1.1.0"
> (configure.ac:39).
Done.

> FIX: Build-require "make" (openvpn-otp.spec:30).
Done.

> TODO: Report to upstream that the codes uses functions (e.g. HMAC_CTX_new())
> deprecated in OpenSSL 3.0.
Upstream issue was opened: https://github.com/evgeny-gridasov/openvpn-otp/issues/48

Comment 4 Fedora Review Service 2023-08-11 17:27:52 UTC
Created attachment 1983058 [details]
The .spec file difference from Copr build 6265230 to 6267033

Comment 5 Fedora Review Service 2023-08-11 17:27:54 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6267033
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2231209-openvpn-otp/fedora-rawhide-x86_64/06267033-openvpn-otp/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.

Comment 6 Petr Pisar 2023-08-15 10:27:38 UTC
Thanks for the fixes.

FIX: Extract a copyright notice from src/base64.c or src/base64.h and package it a binary package. APSL-2.0 license does not only require distributing the license text, but also a copyright notice along the output object files. It is the text between "@APPLE_LICENSE_HEADER_START@" and "@APPLE_LICENSE_HEADER_END@" lines. You can use this sed script in %prep or %build section to do it:

$ sed -n -e '/@APPLE_LICENSE_HEADER_START@/,/@APPLE_LICENSE_HEADER_END@/p' < src/base64.c > base64_copyright

Then don't forget to build-require "sed".

$ rpmlint openvpn-otp.spec ../SRPMS/openvpn-otp-1.0^20230731git47f8ccf-1.fc40.src.rpm ../RPMS/x86_64/openvpn-otp-*
======================================== rpmlint session starts =======================================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.12/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

openvpn-otp.spec:36: E: use-of-RPM_SOURCE_DIR
openvpn-otp.spec:36: E: use-of-RPM_SOURCE_DIR
========= 4 packages and 1 specfiles checked; 2 errors, 0 warnings, 2 badness; has taken 0.3 s ========
FIX: Use "install -m 0644 %{SOURCE1} %{SOURCE2} %{SOURCE3} ." instead of "cp %{_sourcedir}/$license .". The %{SOURCEN} macros avoid using %{_sourcedir}. The "install -m" prevents from mangling a mode of file in case of an unexpected umask.

$ rpm -qlvp ../RPMS/x86_64/openvpn-otp-1.0^20230731git47f8ccf-1.fc40.x86_64.rpm
drwxr-xr-x    2 root     root                        0 Aug 11 02:00 /usr/lib/.build-id
drwxr-xr-x    2 root     root                        0 Aug 11 02:00 /usr/lib/.build-id/16
lrwxrwxrwx    1 root     root                       52 Aug 11 02:00 /usr/lib/.build-id/16/094d59d0ada1d7755e2e5f39103fc8674f0d20 -> ../../../../usr/lib64/openvpn/plugins/openvpn-otp.so
-rwxr-xr-x    1 root     root                    32504 Aug 11 02:00 /usr/lib64/openvpn/plugins/openvpn-otp.so
drwxr-xr-x    2 root     root                        0 Aug 11 02:00 /usr/share/doc/openvpn-otp
-rw-r--r--    1 root     root                    13918 Aug  7  2021 /usr/share/doc/openvpn-otp/README.md
drwxr-xr-x    2 root     root                        0 Aug 11 02:00 /usr/share/licenses/openvpn-otp
-rw-r--r--    1 root     root                    20285 Aug 11 02:00 /usr/share/licenses/openvpn-otp/APSL-2.0.txt
-rw-r--r--    1 root     root                     2507 Aug 11 02:00 /usr/share/licenses/openvpn-otp/Apache-1.0.txt
-rw-r--r--    1 root     root                    10280 Aug 11 02:00 /usr/share/licenses/openvpn-otp/Apache-2.0.txt
-rw-r--r--    1 root     root                    35121 Aug  7  2021 /usr/share/licenses/openvpn-otp/LICENSE
File layout and permissions are Ok.

The package builds in Fedora Rawhide <https://koji.fedoraproject.org/koji/taskinfo?taskID=104872280>. Ok.

Please correct the two "FIX" items, an provide an updated spec file.

Comment 7 Spike 2023-08-15 21:42:54 UTC
Thanks again, Petr. I wasn't aware that %{_sourcedir} also should be avoided when the package is not using files itemized as Source# files.

Updated spec and SRPM file can be found here:
https://spike.fedorapeople.org/openvpn-otp/openvpn-otp.spec
https://spike.fedorapeople.org/openvpn-otp/openvpn-otp-1.0%5e20230731git47f8ccf-1.fc38.src.rpm

(In reply to Petr Pisar from comment #6)
> FIX: Extract a copyright notice from src/base64.c or src/base64.h and
> package it a binary package. APSL-2.0 license does not only require
> distributing the license text, but also a copyright notice along the output
> object files. It is the text between "@APPLE_LICENSE_HEADER_START@" and
> "@APPLE_LICENSE_HEADER_END@" lines.
Done.

> FIX: Use "install -m 0644 %{SOURCE1} %{SOURCE2} %{SOURCE3} ." instead of "cp
> %{_sourcedir}/$license .". The %{SOURCEN} macros avoid using %{_sourcedir}.
> The "install -m" prevents from mangling a mode of file in case of an
> unexpected umask.
Done.

Comment 8 Fedora Review Service 2023-08-15 21:49:10 UTC
Created attachment 1983456 [details]
The .spec file difference from Copr build 6267033 to 6300331

Comment 9 Fedora Review Service 2023-08-15 21:49:12 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/6300331
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2231209-openvpn-otp/fedora-rawhide-x86_64/06300331-openvpn-otp/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.

Comment 10 Petr Pisar 2023-08-16 06:58:17 UTC
$ rpmlint openvpn-otp.spec ../SRPMS/openvpn-otp-1.0^20230731git47f8ccf-1.fc40.src.rpm ../RPMS/x86_64/openvpn-otp-*
======================================== rpmlint session starts =======================================
rpmlint: 2.4.0
configuration:
    /usr/lib/python3.12/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

========= 4 packages and 1 specfiles checked; 0 errors, 0 warnings, 0 badness; has taken 0.3 s ========
rpmlint is Ok.

$ rpm -qlvp ../RPMS/x86_64/openvpn-otp-1.0^20230731git47f8ccf-1.fc40.x86_64.rpm 
drwxr-xr-x    2 root     root                        0 Aug 11 02:00 /usr/lib/.build-id
drwxr-xr-x    2 root     root                        0 Aug 11 02:00 /usr/lib/.build-id/16
lrwxrwxrwx    1 root     root                       52 Aug 11 02:00 /usr/lib/.build-id/16/094d59d0ada1d7755e2e5f39103fc8674f0d20 -> ../../../../usr/lib64/openvpn/plugins/openvpn-otp.so
-rwxr-xr-x    1 root     root                    32504 Aug 11 02:00 /usr/lib64/openvpn/plugins/openvpn-otp.so
drwxr-xr-x    2 root     root                        0 Aug 11 02:00 /usr/share/doc/openvpn-otp
-rw-r--r--    1 root     root                    13918 Aug  7  2021 /usr/share/doc/openvpn-otp/README.md
drwxr-xr-x    2 root     root                        0 Aug 11 02:00 /usr/share/licenses/openvpn-otp
-rw-r--r--    1 root     root                    20285 Aug 11 02:00 /usr/share/licenses/openvpn-otp/APSL-2.0.txt
-rw-r--r--    1 root     root                     2507 Aug 11 02:00 /usr/share/licenses/openvpn-otp/Apache-1.0.txt
-rw-r--r--    1 root     root                    10280 Aug 11 02:00 /usr/share/licenses/openvpn-otp/Apache-2.0.txt
-rw-r--r--    1 root     root                    35121 Aug  7  2021 /usr/share/licenses/openvpn-otp/LICENSE
-rw-r--r--    1 root     root                      979 Aug 11 02:00 /usr/share/licenses/openvpn-otp/base64_copyright
File permissions are Ok.

The package builds in Fedora Rawhide <https://koji.fedoraproject.org/koji/taskinfo?taskID=104908261>. Ok.

The package is in line with Fedora packaging guidelines.
This package is APPROVED.

Comment 11 Fedora Admin user for bugzilla script actions 2023-08-16 09:01:47 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/openvpn-otp


Note You need to log in before you can comment on or make changes to this bug.