Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTTPS validation #65

Closed
pocallaghan opened this issue May 21, 2015 · 5 comments
Closed

HTTPS validation #65

pocallaghan opened this issue May 21, 2015 · 5 comments

Comments

@pocallaghan
Copy link
Contributor

My Mac by default doesn't have any root certs available for PHP/curl. How would you feel about adding something like:

curl_setopt($ch, CURLOPT_SSL_VERIFYHOST, 0);
curl_setopt($ch, CURLOPT_SSL_VERIFYPEER, 0);

We could add an option to enable it if you prefer, but verifying the SSL doesn't seem massively important for a security scanning tool. Happy to make a pull request this evening, just wanted to check your thoughts.

@colinodell
Copy link
Contributor

👍 I like the idea of making it an option.

@pocallaghan
Copy link
Contributor Author

I guess it would make sense to mirror the curl interface -k, --insecure.

@steverobbins
Copy link
Owner

👍

I'd like to see your take on how to include it as an option. Passing it from ScanCommand into all the Request instances might be tricky. This whole project could use some refactoring, but that's another topic. I'm also okay with it being option-less.

@pocallaghan
Copy link
Contributor Author

@steverobbins I think if I was to have a crack at adding the option, my preferred option would be to refactor to create the Request object in ScanCommand and (dependency) inject it into each of the MageScan/Check classes. It'd probably be easier to write tests on the check classes then too.

@pocallaghan
Copy link
Contributor Author

@steverobbins I made a quick PoC of how'd I'd probably go about doing it, you can see it one my fork feature/insecure.

Disclaimer: I just quickly moved some code about as a proof of concept, I've not even ran the command since doing it so there could well be issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants