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

SSL Warnings when SSL is not involved #525

Closed
BenjamenMeyer opened this issue Dec 18, 2014 · 21 comments
Closed

SSL Warnings when SSL is not involved #525

BenjamenMeyer opened this issue Dec 18, 2014 · 21 comments

Comments

@BenjamenMeyer
Copy link

I am doing RESTful APIs and am now all of a sudden getting the message:

urllib3/connection.py:251: SecurityWarning: Certificate has no subjectAltName, falling back to check for a commonName for now. This feature is being removed by major browsers and deprecated by RFC 2818. (See #497 for details.)

I understand the position in the referenced issue (#497), however, this is not a very good thing to do. Not every API can/should be behind SSL. For instance, my current API is in dev mode; when deployed it will be behind SSL properly, but not for developing - which is often on a dev system.

This results in multiple incorrect messages being dumped out, littering output (stdout, logs) that only get in the way.

Note: Filed in this WRT the comment #497 (comment)

@sigmavirus24
Copy link
Contributor

@BenjamenMeyer perhaps that comment was poorly worded but it was not asking people to report new bugs related to it. I was expecting people to not even bother looking for any open bugs (as they usually do not) and just file new bugs. We would pick them up and mark them as a duplicate of #497.

In "dev mode" are you using a certificate anyway? If so, I would guess it's self-signed and one you control (and can add a SAN to). Further, you can silence this warning entirely so that it doesn't litter output.

@BenjamenMeyer
Copy link
Author

No, there are no certificates involved in any manner. It is a straight HTTP request on Port 80; I have not configured any SSL at all.

This is a very big and annoying bug in urllib3.

@sigmavirus24
Copy link
Contributor

@BenjamenMeyer you've read the other issue in its entirety (or so I would hope) so you must realize there's a way to disable this warning. Beyond that, if you're only contacting a server using plain HTTP, you shouldn't be seeing this at all, so you should check to see what is generating this because if there's no certificate returned by the server there's no possible way for you to be seeing this error in "dev mode".

@BenjamenMeyer
Copy link
Author

@sigmavirus24 Yes, I read that issue in its entirety before filing this one. I do see the method of disabling it, but that will also disable other things - which as was noted in that issue is not desirable to do as it may hide things that really are relevant and necessary to find.

And yes, there is no SSL involved.

@kevinburke
Copy link
Contributor

Benjamen, do you have a reproducible test case? Eg a snippet of code that
we can all run that reproduces this issue 100% of the time.

Kevin Burke
phone: 925.271.7005 | twentymilliseconds.com

On Fri, Dec 19, 2014 at 2:55 PM, Benjamen Meyer notifications@github.com
wrote:

@sigmavirus24 https://github.com/sigmavirus24 Yes, I read that issue in
its entirety before filing this one. I do see the method of disabling it,
but that will also disable other things - which as was noted in that issue
is not desirable to do as it may hide things that really are relevant and
necessary to find.

And yes, there is no SSL involved.


Reply to this email directly or view it on GitHub
#525 (comment).

@BenjamenMeyer
Copy link
Author

Yes, I do - via python requests; but it's not a small snippet (a couple public repositories that provide the service and client). I'll try to get something smaller or better instructions together on Monday for you.

@t-8ch
Copy link
Contributor

t-8ch commented Dec 19, 2014

Are you sure there is no https involved? This warning is generated just after wrapping the connection in SSL.
Maybe there are redirects involved.
You could modify urllib3 to print out the certificate it got.

@BenjamenMeyer
Copy link
Author

Ok...I think I know what's doing it - another service that I have to talk to; however, that still doesn't negate the ill-use of this message from something for an API.

@shazow
Copy link
Member

shazow commented Dec 23, 2014

@BenjamenMeyer What would you suggest we do instead?

Someday in the future, this kind of handshake will be completely deprecated and will yield an error instead of a warning (as will browsers who navigate to this kind of URL). I feel it's wise to start warning loudly in advance of this to raise awareness.

That said, all urllib3 warnings can be disabled with urllib3.disable_warnings().

@BenjamenMeyer
Copy link
Author

@shazow It is fine to warn, but make sure that the warning goes to the logger and not the console screen.

First, going to the console screen will hide it for GUI applications; second, it makes it really annoying for scripts. The use-case I had when I came across this ran a single script multiple times from a within a bash script; generating that message for each one - despite the fact that the Python logger was configured to record everything to a file.

For a browser (f.e Chrome, Firefox, Safari), they can warn the user directly.

For an API (f.e urllib3), it is primarily developers that use it, and thus should go to the logger, especially in a language like Python that provides a built-in logger that everyone can use so no special logger should have to be configured. This is especially true of APIs because they often get wrapped by other APIs, like urllib3 does with Python Requests, where many don't have as much control. F.e I'm targetting use of Python Requests so I'm at their mercy for which version of urllib3 gets pulled in; since I'm not using urllib3 directly it's not really of interest to me to be writing something specific to urllib3 - like urllib3.disable_warnings() or even the Python Requests variant (requests.urllib3.disable_warnings()) - that is, after all, what my log capturing is for.

@sigmavirus24
Copy link
Contributor

Sending it to the logger is useless in the majority of the cases where people use requests. If you search GitHub for people using the library perhaps 1% actually bother to configure logging. By only logging the deprecation warning, those users will never know something is actually going to break for them. I haven't researched how users tend to use urllib3, but I suspect the usage isn't vastly different from the way people use requests.

@sigmavirus24
Copy link
Contributor

Perhaps the better solution is to log the warnings as well as using warnings.warn that way if people are using logging and they use disable_warnings() they still have the benefit of the deprecation warnings. Thoughts?

@shazow
Copy link
Member

shazow commented Jan 1, 2015

Not only do few people configure loggers, but few people even know how to configure them properly. We constantly get people opening bugs because they've misconfigured their own loggers, such as using the root namespace rather than module namespace.

@BenjamenMeyer It seems that the current behaviour has helped you catch something unexpected that your code was doing (touching another endpoint you weren't accounting for). Perhaps it's worthwhile keeping it as-is, even if it seems initially annoying?

@BenjamenMeyer
Copy link
Author

@shazow - no, it did not reveal anything I did not already know. I just forgot about it when filing this bug initially. The scripts I am writing require the other end-point (that I don't control, and uses HTTPS) to interact with a service (that I do control, for now) that is under development. I doubt my use-case in this nature is abnormal.

So, I don't agree that keeping it around in current form is worth it. I filed this because of receiving messages for things that I do not need to receive messages on and on the console no less.

Now, if you really want to output to the console then perhaps a check can be done to see if the logger is enabled first and if so use that to determine if the console or the log get the messages? Or perhaps using the Python Warnings module so that integration with the logger can be done natively by Python?

Either one would be much preferred over the current implementation.

@shazow
Copy link
Member

shazow commented Jan 1, 2015

We are using the Python warnings module. There is nothing non-native about what we're doing as far as I can tell. Can you point out the specific code of what you believe we're doing incorrectly?

@BenjamenMeyer
Copy link
Author

I will have to take a closer look and test; but initial impression is documentation that that is a solution.

@BenjamenMeyer
Copy link
Author

By documentation I mean calling out use of https://docs.python.org/2/library/logging.html#logging.captureWarnings as a means of turning off the stdout display.

It's a simple matter of adding:

logging.captureWarnings(True)

in the module that uses urllib3 (or Python Requests). I had to add it in my module that specifically imported Python Requests as opposed to my module that handled the shell interactions, not sure why though.

See #528 😉

@shazow
Copy link
Member

shazow commented Jan 2, 2015

@BenjamenMeyer Won't this have the same side effect for every other library the user is using too?

@BenjamenMeyer
Copy link
Author

@shazow Yes it will. But that's the developer's choice. If they're using the Python logging module then it really makes sense that they should be doing that any way; if not, they can do what was previously advocated. Either way, it's at least documented so developers know they have the choice and can make it appropriately for their application.

@shazow
Copy link
Member

shazow commented Jan 2, 2015

@BenjamenMeyer I'm confused, before you were going on about how we're doing the wrong thing and we need to fix our code immediately, then you suggest we embed a fix that changes the behaviour for every library that the user is using?

@BenjamenMeyer
Copy link
Author

@shazow To summarize the discussion above:

  • I pointed out an incorrect behavior - that it was going to the console and suggested it be logged
  • You all did not want it to be logged and wanted it to be visible because you all seem to have evidence that devs don't usually use the logger correctly
  • I suggested that urllib3 either detect that the logger was not in use and do current behavior or use Python Warnings so that devs could capture to the logger if they used it
  • You pointed that you are already using the Python Warnings module for this
  • I investigated the code and filed PR Updated Security documentation for proper capture of security warnings #528 updating documentation to reflect that you are using Python Warnings and provide devs using urllib3 the information necessary to redirect to the logger instead of out-right disabling all warnings

I think this last line (and the PR) satisfies us both.

My initial objection was that it was seeming to be done in a way that was beyond my control to capture - since the documentation (docs, PR, and issue discussions) did not reflect anything other than using specific APIs relevant to either urllib3 or requests.packages.urllib3. Having the ability to control it is a key factor; what was missing was really the documentation so that people don't have to dig through the urllib3 code all the time to figure out what this discussion led me to figure out.

There is no code-change required; just a documentation change that should help to make everyone's life easier.

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

No branches or pull requests

5 participants