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

Allows disabling of hostname verification. #64

Closed

Conversation

nicklan
Copy link

@nicklan nicklan commented May 4, 2017

This PR would allow a user to call: danger_disable_host_name_verification on a ClientConfig to disable host-name verification.

I know this has been sort-of submitted as a PR before and rejected but I do think there are use cases for this and this tries to make it clear that there is a risk to doing this. A few things to consider:

My use case is building a tool that integrates with existing infrastructure, where in some cases the existing things are configured to use a cert that doesn't verify. I imagine this isn't an isolated case (hence all the other tools that support this), and as long as it's very clear what you're doing it's nice to support.

Forces user to call danger_disable_host_name_verification to make it
clear this isn't a great idea.
@coveralls
Copy link

Coverage Status

Coverage decreased (-50.9%) to 42.013% when pulling 2299b0c on nicklan:allow-disable-hostname-verification into dac2274 on ctz:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 92.852% when pulling 23a5b01 on nicklan:allow-disable-hostname-verification into dac2274 on ctz:master.

ctz added a commit that referenced this pull request May 12, 2017
ctz added a commit that referenced this pull request May 12, 2017
ctz added a commit that referenced this pull request May 13, 2017
ctz added a commit that referenced this pull request May 13, 2017
@ctz
Copy link
Member

ctz commented May 14, 2017

Thanks! This is effectively (but not exactly) landed on master.

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

Successfully merging this pull request may close these issues.

None yet

3 participants