| Summary: | Review Request: testssl - Testing TLS/SSL encryption | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Michael Kuhn <suraia> |
| Component: | Package Review | Assignee: | Denis Fateyev <denis> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | denis, package-review |
| Target Milestone: | --- | Flags: | denis:
fedora-review+
|
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2016-03-09 15:54:52 UTC | Type: | --- |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
|
Description
Michael Kuhn
2016-02-18 14:26:39 UTC
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.
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 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. > […] > > 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 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. Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/testssl testssl-2.6-3.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2016-938187ae52 Could you also provide it for epel7? Should be useful there. testssl-2.6-3.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2016-687fa9a95f 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 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 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. 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. 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. |