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

config: accept verify=False #80

Merged
merged 3 commits into from
Dec 19, 2018
Merged

config: accept verify=False #80

merged 3 commits into from
Dec 19, 2018

Conversation

greut
Copy link
Contributor

@greut greut commented Dec 5, 2018

This allows such configuration, which is mostly useful for special developer cases.

endpoint = https://192.168.1.1/compute
dangerous_no_tls_verify = false

http://docs.python-requests.org/en/master/user/advanced/?#ssl-cert-verification

Signed-off-by: Yoan Blanc <yoan.blanc@exoscale.ch>
cs/client.py Outdated
try:
verify = strtobool(config['verify'])
except ValueError:
pass
Copy link

Choose a reason for hiding this comment

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

Should we not throw an error message if the verify configuration is not correctly set ?

Copy link
Contributor Author

@greut greut Dec 6, 2018

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

verify is of type Union[str, bool]

@brutasse
Copy link
Member

brutasse commented Dec 6, 2018

I've tried to avoid adding such a possibility in the past, e.g. #18 and #27

The reasoning is that an insecure setup should be either hard to get or it should be obvious that it's insecure. If there really is no other way I'd love something in the spirit of hdparm's --yes-i-know-what-i-am-doing and --please-destroy-my-drive flags, i.e. something more glaring than verify=False.

@greut
Copy link
Contributor Author

greut commented Dec 6, 2018

@brutasse requests comes with its own set of warnings once false is set.

Signed-off-by: Yoan Blanc <yoan.blanc@exoscale.ch>
@greut
Copy link
Contributor Author

greut commented Dec 19, 2018

@brutasse verify = false still does nothing useful, but dangerous_no_tls_verify = true does.

Signed-off-by: Yoan Blanc <yoan.blanc@exoscale.ch>
Copy link
Member

@brutasse brutasse left a comment

Choose a reason for hiding this comment

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

⚠️

@greut greut merged commit cb31810 into master Dec 19, 2018
@greut greut deleted the verify-false branch December 19, 2018 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants