Closed invisiblethreat closed 8 years ago
I think you can disable these at a global level with the warnings
module. Further, to work with logging (if I remember correctly) you need to reach into urllib3
(and we document that), so I'm not against documenting this for users who are not going to use certificate verification for HTTPS connections.
I remain strongly in favour of leaving these warnings in place. Yes, they're annoying, but they're also there for a reason. If anything, I'm tempted to switch it off and replace it with one of our own! =P
At this point it's fairly obvious that @Lukasa and I are -1 on this feature. @kennethreitz @shazow any opinions?
To some extent, I agree that the warnings are important, but I think that there are multiple factors that might need to be considered.
From a developer perspective, know that I know about this, I can just turn this off if I feel inclined. I'm newer to packages, so it when I read the docs, in the warning, that solution didn't actually work. I like the idea that @Lukasa presented about making something that 's specific to requests
.
From a user perspective, I installed pyvmomi
with pip
today, which uses requests
in its internals. It's really a non-transparent error that gets emitted back to the user in a case where requests
is a silent supporting library.
Yes, requests.packages.urllib3.disable_warnings()
is a shortcut for turning it off using the warnings module's filtering.
I'd strongly recommend having some kind of warning to this effect. +0.5 on having the urllib3 one propagate, +1 if you want to put in the effort and adding a requests-specific one. -1 on having no warning.
If you'd like, we can make the urllib3 warning message configurable and you can override it so you can piggyback on the same logic otherwise.
Once again, I don't consider this message user-hostile, I consider it extremely valuable. You now know that pyvmomi has turned off TLS certificate verification, which is fairly important information!
That said, I don't object to us having a more requests-y way to silence it.
Yeah, really I think this is a bug in pyvmomi
. If they don't want to be embarrassed like this, they should be disabling warnings in their tool. It isn't our job to not warn users that the connections some tool is making could expose them to MITM attacks because the tool is not performing certificate verification.
Thanks for the discussion folks! I do appreciate everyone commenting, and helping me see the value in the way that things are done, and why. I was having a hard time seeing the more general cases! :)
Will work on a patch today, as time allows, to have a 'requests-y' way to silence. Feedback will be welcome!
@invisiblethreat feel free to hop into IRC if you have questions
Just wondering if you considered the case where requests is used in a webhook. I have to suppress the warning so it doesn't pollute the JSON output of the script (or am I missing something?)
@macterra I'm not sure I understand. Are you looking for alternative strategies for disabling the warning or ...?
It's also extremely unclear to me why your webhook is disabling certificate verification. I'd be wanting to leave that on.
Further, if you're piping stdout from some script to get the results, warnings will come out from stderr so they should not pollute your JSON output.
Right, if the warning is on stderr then no problem. I couldn't tell from looking at the output in the console, my mistake.
I think we should customize this to point to the requests documentation instead of urllib3's documentation.
I'm not sure I understand what this warning feature is meant to achieve and how the warning is meant to be controlled.
I have a module which uses requests
and has to make requests with the verify=False
argument. This causes the users of my module to see the unnecessary warning. Unnecessary warnings make important warnings more difficult to see,
It would make sense for my module to disable the warning, but then I would be disabling the warning also in any other module using requests
in the application!
Things are not better if I need to instruct the user of my module to disable the warning: The fact that I'm using requests
is normally just an invisible implementation detail the user shouldn't need to know. And the user still would only have the option to silence everything or noting.
I don't think global warnings are going to be helpful.
I could subclass urllib3.HTTPSConnectionPool
and override _validate_conn()
and make requests
use that in my module to avoid hiding warnings from other modules, but that seems to be too much work for a simple thing.
I'm not sure I understand what this warning feature is meant to achieve
causes the users of my module to see the unnecessary warning.
When you set verify=False
you are no longer securing your network connections. That, IMHO, is not an unnecessary warning, it's a hugely relevant warning. Your users, who previously had no idea that you weren't verifying certificates, now know this to be true.
If the warning is not valuable for your module, it is not likely to be valuable for any other module in the application (once you've thrown away security in one network point there's no point crowing about it everywhere else). I don't really see a problem with disabling it globally.
When the user explicitly requests verify=False
for a particular request, I don't see the value of showing a warning. When I as a module author set verify=False
, I set it by user's request (or I'm being malicious, but the warning doesn't help there either, because I can silence the warnings). Indeed, I want to avoid being malicious and don't want to globally silence warnings, because that takes away the useful warning for those requests some other part of the application is unknowingly making insecurely.
Also, having the warning on for those requests where verification is explicitly turned off by by the user also hides the requests where the user would want the warning, since the warning is given only once. The warning is also not really helpful for the user, since it doesn't mention the URL of the particular request.
I don't agree that throwing away security in one network point somehow makes any security checks useless in the application, neither do any browser vendors. Browsers let me bypass security checks for individual URLs but keep checking the rest and I like that.
When I have a tool which talks to an internal server with a self-signed cert in an internal network, but also communicates with outside hosts, I do want to verify the outside communication. This would be the situation where I would like as a user to see warnings about accidentally unsecured requests.
Consider I'm making service_foo
and someone uses it in an app:
import service_foo
import requests
session = service_foo.Session('https://10.0.0.1', verify=False)
data = session.get_data()
requests.put('https://example.com/submit', data=data)
I have 2 options for service_foo
:
https://10.0.0.1
https://example.com/submit
is not safehttps://example.com/submit
is not safeI don't think either option is good, but option 1 is worse, because it is giving false alarms. But I'm not comfortable having security checks turned off for the user as a side-effect of using my module.
If I would do this with a shell script, the user would be happier and safer:
curl --insecure -o data https://10.0.0.1/get_data
curl --upload-file data https://example.com/submit
To me it only makes sense to give a warning if the configuration of the Python platform is broken. The page https://urllib3.readthedocs.org/en/latest/security.html linked to in the InsecureRequestWarning
message is indeed geared to show how to fix problems in the platform. If the user requests to skip verification, there shouldn't be a warning like there isn't a warning if the user requests an http
URL instead of an https
one.
When the user explicitly requests verify=False for a particular request, I don't see the value of showing a warning.
Who is the 'user'? Throughout your post this question kept coming to my mind, because I believe you're conflating two audiences.
When I as a module author set verify=False, I set it by user's request (or I'm being malicious).
Or you're being negligent. You have had had users complain that they couldn't interop with their self-signed certs and so you turned cert verification off, despite the fact that turning off cert verification is not the way to deal with that problem.
that takes away the useful warning for those requests some other part of the application is unknowingly making insecurely
This sentence baffles me. It suggests that it is acceptable to warn when an application unknowingly makes insecure requests, but that an application knowingly making them is somehow OK. I do not see how knowingly making insecure requests should in any way be considered 'more secure' than unknowingly doing so.
Also, having the warning on for those requests where verification is explicitly turned off by by the user
Which user? How are we to distinguish between the module author and the 'user', whoever they may be?
The warning is also not really helpful for the user, since it doesn't mention the URL of the particular request.
The warning should not mention the URL of the request, because that would risk generating warning spam. We warn once, saying 'this application is at risk', not 'this particular communication is at risk'.
Browsers let me bypass security checks for individual URLs but keep checking the rest and I like that.
Browser vendors warn when you access a URL with an invalid cert! They print dialog boxes and highlight the URL bar in red! That's exactly what we're doing. We aren't stopping you from doing anything, we're just saying "hey, this is bad!" What you're asking us to do is the same as asking browser vendors to allow users to turn off that red warning for particular URLs, and I guarantee that they will refuse to do so because the security implications are monstrous.
When I have a tool which talks to an internal server with a self-signed cert in an internal network, but also communicates with outside hosts, I do want to verify the outside communication.
No, you want to verify all the communication. Verify the self-signed cert! Validate that you got the certificate you expected to get. verify=False
should be considered a sledgehammer approach to security, effectively saying "screw security, just make it work". That's absolutely fine, you have a right to say that, but we have an obligation to call it out as insecure.
I don't think either option is good, but option 1 is worse, because it is giving false alarms.
Option 1 is not giving false alarms, it's giving real alarms. The communication to 10.0.0.1 is insecure, and we should not be pretending otherwise.
If I would do this with a shell script, the user would be happier and safer.
The user may well be happier, but they wouldn't be safer. They'd be exactly as safe as they were before. You seem to be under the impression that turning off this warning somehow magically makes certificate verification go away, and it does not. I'll touch on this again at the end of this response.
To me it only makes sense to give a warning if the configuration of the Python platform is broken.
No, we should fail hard if the configuration of the Python platform is broken and you didn't ask for unverified requests. If your platform cannot make safe TLS connections then we absolutely should not be making them, except in the situation where our user expressly tells us not to care (by setting verify=False
), in which case we should warn that what you're about to do is dangerous.
I think you're labouring under a misapprehension, so I'd like to make something very clear: there is no way to make an unvalidated HTTPS request with requests without either a) setting verify=False
(our warn behaviour) or b) deliberately sabotaging the ssl
module. We cannot catch b) and do not warn on it. This is the only situation that would fall under the notion you raised of "problems with the platform". The advice on urllib3's help page does not apply to us because we take all the necessary platform related steps, including bundling trusted certs and manually verifying certificates.
There is a dangerous view in the web community that you should only verify certificates signed by trusted root certificates. This view is utterly misguided. If you're encountering self-signed certificates you should damn well validate them. That's totally do-able! Add the self-signed cert to a .pem
file and pass it as an argument to verify
!
If you're having trouble with combining that with the bundled .pem
file, please let me know and I'll enhance mkcert.org to enable you to concatenate your own certificates with the trusted roots. But please, don't pretend that setting verify=False
is safe: it simply isn't.
Also, having the warning on for those requests where verification is explicitly turned off by by the user also hides the requests where the user would want the warning, since the warning is given only once.
This is also a bit baffling. By setting verify=False
you maybe explicitly turning it off for just that request, but there is no way to convey that beyond the point where we construct our request. There's also no reason to convey it beyond that point because you've disabled certificate verification. The context in which you did so is of no consequence to us or anyone using your app.
What you're asking us to do is the same as asking browser vendors to allow users to turn off that red warning for particular URLs, and I guarantee that they will refuse to do so because the security implications are monstrous.
My browsers allow me to permanently accept an unverified certificate, which is "monstrously insecure".
The communication to 10.0.0.1 is insecure, and we should not be pretending otherwise.
The connection is insecure in the way that you can't verify a digital certificate, but verifying a certificate doesn't really tell you whether the server you are talking to is secure. But when I'm talking to a server in a closed network I can really make sure the security of the server.
I think you're labouring under a misapprehension, so I'd like to make something very clear: there is no way to make an unvalidated HTTPS request with requests without either a) setting verify=False (our warn behaviour) or b) deliberately sabotaging the ssl
I'm trying to wonder how I could be a good citizen in my module by honoring the user's wish to ignore certificate checks and warnings for the URL they give to me. And what value the warning model adds. What is the case where a request with verify=False
should show a warning to the user?
I don't see how the warning mechanism can catch negligent code, since it can't distinguish whether a request is made because of sloppy coding or because a user requested so. I don't think a module like requests
should dictate a security policy either. I've understood warnings are usually aimed for developers so they can fix incorrect code, but this warning isn't like that. If the warning is just for the general education of the user, there should be an easy way for the user to hide it.
Getting the warning isn't only cosmetic either, because it messes the output of a program.
I only see negative value of the warning so I'm turning it off in my module even if I hate to hide such a global policy change there.
There is a dangerous view in the web community that you should only verify certificates signed by trusted root certificates. This view is utterly misguided.
Didn't know there's a view like that. A cert signed by a root cert doesn't really prove anything about the security of the site. It's cheap to get an anonymous cert if you want to do bad things.
If you're encountering self-signed certificates you should damn well validate them. That's totally do-able! Add the self-signed cert to a .pem file and pass it as an argument to verify!
The user would need a secure channel to obtain the cert, like an internal trusted network. But if the server is itself in the same internal network, there is not much gained. But this is in any case something the user decides, I can't impose a policy in my module.
I agree with @kankri, for the most part. That was the original design intention.
I propose something — that we disable by default but have our own function for re-enabling it, or documenting how to turn it on. I don't want out users to have to go out of the way to use the code as intended. verify=False
is a feature, albeit not a best-practice one. That's none of our business.
I agree that verify=False
is a feature, but I don't agree that it's a feature on the same level as params=
or cert=
. It's a feature that defaults to a secure value and may be set to an insecure one. It's a giant, tempting option for people to throw security out the window for the sake of expediency, and I think that impulse should be resisted (but not disallowed). I will always lean towards the 'you must explicitly be insecure' school of thought, and I don't care if that means flicking two switches and not one.
Regardless, this is your call not mine. =)
Just wanted to say I agree with @kankri and that @kennethreitz's remark
verify=False is a feature, albeit not a best-practice one. That's none of our business.
sums it up well.
For those who also want to disable the warnings, this is how to do it. You need to use the warnings modules, which is part of the standard library:
import warnings
import requests
from requests.packages.urllib3 import exceptions
with warnings.catch_warnings():
warnings.simplefilter("ignore", exceptions.InsecureRequestWarning)
warnings.warn('a non-requests warning is not blocked')
print requests.get('https://rsa-md5.ssl.hboeck.de/', verify=False)
This configures a warning filter that will ignore any warning of category InsecureRequestWarning
. The output looks like:
test.py:46: UserWarning: a non-requests warning
warnings.warn('a non-requests warning is not blocked')
<Response [403]>
(The test site happens to return a 403 Forbidden page, but that's not important here.)
Note that you need to use the class from the bundled urllib3
package, and not the class from a top-level urllib3
package, if you happen to have one installed.
You can (and probably should) make a small function that uses the context manager in the smallest possible region of code:
def silent_unverified_get(*args, **kwargs):
kwargs['verify'] = False
with warnings.catch_warnings():
warnings.simplefilter("ignore", exceptions.InsecureRequestWarning)
return requests.get(*args, **kwargs)
Or simply do this:
requests.packages.urllib3.disable_warnings()
@Lukasa
Or simply do this:
requests.packages.urllib3.disable_warnings()
Except that I find no mention of this function in the requests manual.
While it is far from everybody who knows about it, I would argue that the warnings
module is the standard tool a Python programmer should look to when he wants to disable warnings. It is part of the standard library and is well documented.
I suggest putting a reference to warnings
into the requests
documentation — or to the convenience disable_warnings
function if you like, as long as there is a corresponding enable_warnings
function (it seems that there isn't such a function).
Once again: I don't want to disable warnings in general. I just want this particular warning to go away when I explicitly set verify=False in my code. There may be other, useful, warnings, unlike this particular, useless, warning. What is so difficult to understand about this?!
@zaitcev At the risk of repeating myself:
requests.packages.urllib3.disable_warnings()
And if even that is too broad for you:
from requests.packages.urllib3.exceptions import InsecureRequestWarning
requests.packages.urllib3.disable_warnings(InsecureRequestWarning)
Finally, a note, @zaitcev: you will find that taking the exasperated tone you just did wins you no favours at all. Remember that we are all volunteers, and that we have a limited amount of our time to give to build you things. Please try to treat us in the manner you would like to be treated.
@zaitcev It doesn't look like this will change in the requests module itself, but I hope you can use the code I put in my other comment. That should allow you to selectively disable the warnings emitted by urllib3.
You can also suppress it with:
with warnings.catch_warnings():
warnings.filterwarnings("ignore", message=".*InsecurePlatformWarning.*")
...
In my case, I'm not using requests directly, so suppressing like this lets me worry a little less about being broken later.
@zaitcev Pulling all the previous suggestions together, you can do something like this:
verify = False
if not verify:
from requests.packages.urllib3.exceptions import InsecureRequestWarning
requests.packages.urllib3.disable_warnings(InsecureRequestWarning)
r = requests.get('https://www.example.com', verify=verify)
@utkonos That would leave warnings disabled for all subsequent requests.
Putting other examples together, I extended the default Session
(as requests.get
and other shortcuts create a temporary Session
, anyway):
from requests.packages.urllib3 import exceptions
class Session(requests.sessions.Session):
def request(self, *args, **kwargs):
if not kwargs.get('verify', self.verify):
with warnings.catch_warnings():
warnings.simplefilter('ignore', exceptions.InsecurePlatformWarning)
warnings.simplefilter('ignore', exceptions.InsecureRequestWarning)
return super(Session, self).request(*args, **kwargs)
else:
return super(Session, self).request(*args, **kwargs)
Disabling all warnings from requests
is likely a bad idea, a bit better might be:
import requests
from requests.packages.urllib3.exceptions import InsecureRequestWarning
requests.packages.urllib3.disable_warnings(InsecureRequestWarning)
To summarize how I handled this:
import warnings
with warnings.catch_warnings():
warnings.simplefilter("error")
try:
req = requests.get("https://an-insecure-server.com")
except (RuntimeWarning, requests.exceptions.SSLError)::
log.error("Making an insecure request")
warnings.simplefilter("ignore")
req = requests.get("https://an-insecure-server.com")
This allows me to check if a request is insecure, hide the urllib warning and raise a warning of my own formatting for the user. It does require that the request is made twice. Edited to make the except clause less broad.
except Exception:
is VERY broad. you really don't want that.
The above goes some way to addressing the concerns on both sides of this discussion.
doesn't it throw some subclass of Exception you can catch instead?
Or use logging.captureWarnings()
The alternative is to know that urllib3 is involved and hardcode its namespace, see comment by tuukkamustonen. This was my principal objection: they could've made it work right, I even provided a patch in a pull request. But they are in denial that the problem exists and tell all the users to come up with terrible workarounds like "except Exception" or "from requests.packages.urllib3 import exceptions". At this point someone have to admit they were wrong all along and so we're stuck.
This was my principal objection: they could've made it work right, I even provided a patch in a pull request. But they are in denial that the problem exists and tell all the users to come up with terrible workarounds like "except Exception" or "from requests.packages.urllib3 import exceptions". At this point someone have to admit they were wrong all along and so we're stuck.
@zaitcev Once again, let me remind you that this is a volunteer community doing the best they can. We have left this issue free for discussion, we have not attempted to lock it or prevent further discussion. We are listening to you. What we aren't doing is immediately agreeing with your assessment of the situation. Please consider the possibility that we care about more use cases than simply yours, and that we need to balance the needs of all of them.
As to your pull request, it was rejected for a very specific reason that you are constantly ignoring! Let me quote myself quoting Ian:
The closing statement was: "Given that this is mostly in urllib3 and would rely on acceptance there, I'm closing this until progress has been made there." (Emphasis mine.)
As of today I still see no associated pull request or issue for this problem in urllib3. No-one from this project has stood in your way or prevented this work from occurring, we simply haven't chosen to do it ourselves because we do not currently agree with you.
However, at the risk of going down this rabbit hole again, let me reiterate:
This was my principal objection: they could've made it work right.
I do not believe your patch makes this work "right". As I have said many times in this thread, I consider the current behaviour to be desirable. Doing insecure TLS requests is a bad idea, and users should be admonished against doing so.
My position is that a user deserves to know when they are making a TLS request that is not adequately secured, especially in any system that is handling their passwords.
There is agreement in this thread that we should consider having a requests-level hook to disable these warnings. On the other hand, currently no-one but you believes that some previously-nonexistent distinction between verify=False
and verify=None
should be added in order to implicitly silence these warnings. You will find it much easier to do the former than the latter.
+1 to not differentiating between verify=False and verify=None. I would support either:
And thanks to all the volunteers for supporting requests, whether or not this gets fixed: it's a great library :)
This is an awesome library, thanks for all your hard work.
I cam across this issue after upgrading my python packages recently and noticing a slew of new InsecurePlatformWarning printouts. So I'm contributing my use case, which I think is compelling.
I am using requests to manage jenkins servers across 4 different environments. Three of the environments (dev, staging, production) all have valid certificates. The 4th environment is a vagrant virtual box, which developers can use to test changes on their local machines. This does not have a valid certificate, but as a matter of policy, all the server configurations reject unencrypted requests.
The jenkins connection settings (server name, token, etc) for an environment include a specific flag for turning off ssl verification, which is only set to True for the vagrant environment.
In my setup, disabling the warning globally wold be a bad idea because the project is rather large and a lot of requests may be made, with or without the requests library. Disabling the warning in scope would be alright, except part of the project includes a flask application and other possibly multi-threaded cases.
In my opinion, it seems like use of verify=False should be supported and work as expected, without warnings. It is up to the application developer to decide when and if this should be allowed. For example, if I were writing a browser for general use, then I would never set this to True without displaying a big confirmation dialog with lots of red text. But if I own the server and the client and have my own reasons for not issuing a certificate, I should be able to have a clean log and not hide other potential issues.
It is up to the application developer to decide when and if this should be allowed.
This contention is where I differ with you. I believe it is up to the developer to decide when they should use it. But I believe it is up to the user to decide if that choice is acceptable. It is critical that users understand when they are being placed at risk by developer choices, and that they be able to evaluate that risk.
But if I own the server and the client and have my own reasons for not issuing a certificate, I should be able to have a clean log and not hide other potential issues.
And you can do that, by using a logging context manager to capture the warning. We are also considering having requests make this warning more specific to requests so that it's easier to capture, but that has not yet happened.
I have a situation similar to @jamie-sparked.
I understand the point of Lukasa on enforcing security, but I think you should let YOUR user decide what's best for them. Requests is a library, not an end-user application. IMO you should consider developers as your users. Application developers should be liable for security mistakes, if they decide to turn off the cert verification (i.e. verify=False)
As developer, I value flexibility over a library trying to dictate what I should do.
BTW as others said, I find requests excellent and I appreciate all your effort. Thanks.
@thalesac We do let developers decide. As discussed many times in this thread, it is quite possible to turn this warning off. However, we don't have one switch that turns all the warnings off: you need to manually do each one. This is an attempt to make our users consciously remove each safety guard.
Think of it as defense-in-depth. To use a footgun analogy, we're handing you a gun with the safety on and no bullets in it, and a magazine. If we had verify=False
disable all the warnings, that would be the equivalent of having a gun that, when a magazine was inserted, automatically disabled the safety and chambered a round. Convenient? Sure. Dangerous? You bet.
I'm afraid, I disagree with your analogy model. I'd say verify=False IS your safety/security mechanism. If you explicitly (or manually) disabled it, you don't want the gun warning you all the time when you're shooting the bad guys. Obviously the default behaviour must enforce security thought. Anyway, I understand this is just my view and you should do what you think is best for your project. Maybe that's why it's a good library. :) Thanks
I definitely agree with Lukasa, security its first and if as a developer I am using verify=False in one part of my code then I should suppress the warning, if I don't want to be warned.
Anyway excellent library huge fan of your team work, keep it up, +10000 for the patience to respond to us.
As of 1.9 of
urllib3
, the following warning appears once per invocation:When using
verify=False
would it be useful to also setrequests.packages.urllib3.disable_warnings()
?I understand that this is a design decision that not everyone might agree with. :)