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

Add -certificate-check and -certificate-verify flags #620

Merged
merged 8 commits into from
Aug 29, 2023

Conversation

hahwul
Copy link
Contributor

@hahwul hahwul commented Aug 5, 2023

Hi projectdiscovery team,

This PR is a continuation of the PR from the this discussion. After discussing it a bit more, I looked into it further and found that interactsh-client consistently uses InsecureSkipVerify=true. While this is for usability purposes, some users might still prefer a more secure connection from time to time.

TLSClientConfig: &tls.Config{
			Renegotiation:      tls.RenegotiateOnceAsClient, // Renegotiation is not supported in TLS 1.3 as per docs
			InsecureSkipVerify: true,
			MinVersion:         tls.VersionTLS10,
}

https://github.com/projectdiscovery/retryablehttp-go/blob/3431517eac0842ae153b681280dfd6e22261d56b/http.go#L46

The two flags added this time are meant to check certificates and either raise a warning or stop the process, thereby alerting users about certificate issues. I'd appreciate your feedback on this :D

   -cc, -certificate-check                  check the certificate of the interactsh server
   -cv, -certificate-verify                 verify the certificate of the interactsh server

Additionally, it seems like the Usage section in the README should be updated. I wanted to include both the new flags(-cc,-cv) and the missing flags in this PR, but I excluded them for now to avoid any potential conflicts and committed the changes.

@Mzack9999 Mzack9999 self-requested a review August 8, 2023 08:42
@Mzack9999
Copy link
Member

@hahwul Thanks for the PR. I was thinking about making TLS behavior customizable via external variable, for example:

TLS_VERIFY=false # default-skip
TLS_VERIFY=true # verify certificate

In this way it can be applied to all tools and be more consistent console standard variables like HTTP_PROXY.
What do you think? (cc @ehsandeep )

Copy link
Member

@ehsandeep ehsandeep left a comment

Choose a reason for hiding this comment

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

We can make use of ENV Var instead of CLI option as mzack suggested.

@hahwul
Copy link
Contributor Author

hahwul commented Aug 11, 2023

@ehsandeep @Mzack9999
Hello again.
I've followed your suggestion and switched from using flags to relying on environment variables for TLS verification. Now, it's set up to work only when the TLS_VERIFY environment variable is true.

@hahwul
Copy link
Contributor Author

hahwul commented Aug 11, 2023

This is a thought that has occurred to me personally. It might be worth considering the creation of distinct ENV variables specific to each tool, with a higher priority than TLS_VERIFY. For instance, something like INTERACTSH_TLS_VERIFY could be used.

This approach would allow for tailored behaviors for individual tools, while maintaining a consistent approach to TLS verification.

@hahwul hahwul requested a review from ehsandeep August 11, 2023 14:55
@dogancanbakir dogancanbakir linked an issue Aug 17, 2023 that may be closed by this pull request
@Mzack9999 Mzack9999 added the Type: Enhancement Most issues will probably ask for additions or changes. label Aug 29, 2023
@Mzack9999 Mzack9999 merged commit 61dc9ee into projectdiscovery:dev Aug 29, 2023
@hahwul hahwul deleted the feature/verify-cert-flag branch August 29, 2023 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement Most issues will probably ask for additions or changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Certificate verification bypass flag in interactsh-client
3 participants