Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

Add testssl.sh formula #46716

Closed
wants to merge 3 commits into from
Closed

Add testssl.sh formula #46716

wants to merge 3 commits into from

Conversation

jsarenik
Copy link
Contributor

@jsarenik jsarenik commented Dec 5, 2015

class Testssl < Formula
desc "Tool which checks for the support of TLS/SSL ciphers and flaws"
homepage "https://testssl.sh/"
url "https://github.com/jsarenik/testssl.sh/releases/download/v2.6/testssl-2.6.tar.gz"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be using the upstream release, not your own personal fork. Most of the audit errors are related to that.

@jsarenik
Copy link
Contributor Author

jsarenik commented Dec 6, 2015

@DomT4 Thanks much for the feedback! I really appreciate it. Please have a look now.

def install
bin.install "testssl.sh"
end
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can a test be added to do something more substantial than e.g. --version or --help? See cmake.rb for an example of an application formula with a good test and tinyxml2.rb for an example of a library formula with a good test. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for ideas. testssl.sh is not a library so that I could test if it's present and usable by compiling some code. It's normally used as an online test.

But what about running testssl.sh --local? That seems like a reasonable way to test things are working.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, --local seems fine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Online tests are also fine.

@DomT4
Copy link
Member

DomT4 commented Dec 6, 2015

Given there's no configure/make process here, we're just installing an executable, and our OpenSSL isn't regularly available in the $PATH I have my doubts it'll actually be using our OpenSSL.

Looking at the script we should probably wrap it in a bin.env_script_all_files call:

following variables make use of $ENV, e.g. OPENSSL=<myprivate_path_to_openssl> ./testssl.sh <host>

@jsarenik
Copy link
Contributor Author

jsarenik commented Dec 8, 2015

$ ls /usr/local/Cellar/openssl/*/bin/openssl
/usr/local/Cellar/openssl/1.0.2e/bin/openssl

@jsarenik
Copy link
Contributor Author

jsarenik commented Dec 8, 2015

@DomT4 What about patching testssl.sh before install so that it uses openssl from the Cellar?

OPENSSL=$(echo /usr/local/Cellar/openssl/*/bin/openssl | sed 1q) testssl.sh <domain>

@MikeMcQuaid
Copy link
Member

@jsarenik Using a wrapper script would be preferable to patching.

@DomT4
Copy link
Member

DomT4 commented Dec 8, 2015

Yeah, wrapper script will probably be more robust here.

@jsarenik
Copy link
Contributor Author

Thanks for feedback! What about this try? I can squash it, just let me know. Have a nice weekend!

@MikeMcQuaid
Copy link
Member

Thanks for your contribution to Homebrew! Without people like you submitting PRs we couldn't run this project. You rock!

For future reference the preferred commit message format for new formulae is testssl 2.6 (new formula).

@jsarenik jsarenik deleted the jasan/testssl.sh branch February 5, 2016 13:34
@Homebrew Homebrew locked and limited conversation to collaborators Jul 10, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants