Bug 1309707 - Review Request: testssl - Testing TLS/SSL encryption
Review Request: testssl - Testing TLS/SSL encryption
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Denis Fateyev
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2016-02-18 09:26 EST by Michael Kuhn
Modified: 2016-03-13 23:56 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2016-03-09 10:54:52 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
denis: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Michael Kuhn 2016-02-18 09:26:39 EST
Spec URL: https://ikkoku.de/~suraia/testssl/testssl.spec
SRPM URL: https://ikkoku.de/~suraia/testssl/testssl-2.6-1.fc23.src.rpm

Description:
testssl.sh is a free command line tool which checks a server's service on any
port for the support of TLS/SSL ciphers, protocols as well as recent
cryptographic flaws and more.

Fedora Account System Username: suraia
Comment 1 Denis Fateyev 2016-02-23 13:25:49 EST
At the first glance, before the review:

1) testssl uses `nslookup` for resolving which is missing in Requires.
   $ dnf provides '*bin/nslookup'
     ...
   bind-utils-32:9.10.3-1.fc23.x86_64 : Utilities for querying DNS name servers
   -- should be added to R's.

2) testssh uses mapping file(s) which isn't installed.
  You can provide `mapping-rfc.txt` in `/usr/share/testssh` (via macro: `%{_datarootdir}/testssh`). Also, a small patch which sets the new proper `mapping-rfc.txt` location will be required;

3)  mkdir -p %{buildroot}%{_bindir}
    cp -p %{name}.sh %{buildroot}%{_bindir}/%{name}

 can be replaced with one command (which is also sets proper permissions):
    install -Dpm 0755 %{name}.sh %{buildroot}%{_bindir}/%{name}

4) util-linux (hexdump), grep (grep, egrep), gawk (awk) also needed in Requires;

5) openssl version should be constrained (>= 1.0) as mentioned in Readme.
Comment 2 Michael Kuhn 2016-02-23 15:53:07 EST
Thanks for the comments!

> 1) testssl uses `nslookup` for resolving which is missing in Requires.
>    $ dnf provides '*bin/nslookup'
>      ...
>    bind-utils-32:9.10.3-1.fc23.x86_64 : Utilities for querying DNS name
> servers
>    -- should be added to R's.

Somehow, I completely forgot to check which utilities are used, sorry. Fixed.

> 2) testssh uses mapping file(s) which isn't installed.
>   You can provide `mapping-rfc.txt` in `/usr/share/testssh` (via macro:
> `%{_datarootdir}/testssh`). Also, a small patch which sets the new proper
> `mapping-rfc.txt` location will be required;

Fixed, too. I opted to use a small sed invocation to set the location.

> 3)  mkdir -p %{buildroot}%{_bindir}
>     cp -p %{name}.sh %{buildroot}%{_bindir}/%{name}
> 
>  can be replaced with one command (which is also sets proper permissions):
>     install -Dpm 0755 %{name}.sh %{buildroot}%{_bindir}/%{name}

Fixed.

> 4) util-linux (hexdump), grep (grep, egrep), gawk (awk) also needed in
> Requires;

Fixed.

> 5) openssl version should be constrained (>= 1.0) as mentioned in Readme.

Fixed.

New Spec: https://ikkoku.de/~suraia/testssl/testssl.spec
New SRPM: https://ikkoku.de/~suraia/testssl/testssl-2.6-2.fc23.src.rpm
Comment 3 Denis Fateyev 2016-02-23 17:29:21 EST
Looks good now. Only some small things:

1) You can add "coreutils" to BR (`install` belongs to it);

2) Since bash is required, you can set common shebang "#!/bin/sh", getting rid of "/usr/bin/env" RPM dep at the same time;

3) Since `mapping-rfc.txt` is now included, you can add `openssl-rfc.mappping.html` to %doc.

Otherwise package is APPROVED.
Comment 4 Michael Kuhn 2016-02-24 12:33:03 EST
> […]
> 
> Otherwise package is APPROVED.

Thanks! I fixed your comments and uploaded new versions (that I will import) for completeness.

New Spec: https://ikkoku.de/~suraia/testssl/testssl.spec
New SRPM: https://ikkoku.de/~suraia/testssl/testssl-2.6-3.fc23.src.rpm
Comment 5 Denis Fateyev 2016-02-24 13:36:42 EST
To me, "/bin/sh" would go better as with other common shell scripts, and looks clearer in the package deplist along the existing bash dependency. Although not critical, up to you.
Comment 6 Gwyn Ciesla 2016-02-24 16:16:38 EST
Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/testssl
Comment 7 Fedora Update System 2016-02-25 13:08:30 EST
testssl-2.6-3.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-938187ae52
Comment 8 Denis Fateyev 2016-02-25 13:29:32 EST
Could you also provide it for epel7? Should be useful there.
Comment 9 Fedora Update System 2016-02-25 15:50:30 EST
testssl-2.6-3.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-687fa9a95f
Comment 10 Fedora Update System 2016-02-26 15:52:41 EST
testssl-2.6-3.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-938187ae52
Comment 11 Fedora Update System 2016-02-26 21:20:06 EST
testssl-2.6-3.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-687fa9a95f
Comment 12 Fedora Update System 2016-03-09 10:54:50 EST
testssl-2.6-3.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.
Comment 13 Fedora Update System 2016-03-09 15:13:34 EST
testssl-2.6-3.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.
Comment 14 Fedora Update System 2016-03-13 23:56:11 EDT
testssl-2.6-3.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report.

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