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

Feature: Remove support for common names in certificates #497

Open
sigmavirus24 opened this Issue Oct 31, 2014 · 88 comments

Comments

Projects
None yet
@sigmavirus24
Member

sigmavirus24 commented Oct 31, 2014

We should follow the lead of Chromium. We should deprecate support for use of common names when a certificate doesn't provide a subjectAltName.

This will bring us into compliance with the >10 year old RFC 2818

@sigmavirus24 sigmavirus24 changed the title from Feature: to Feature: Remove support for common names in certificates Oct 31, 2014

@sigmavirus24

This comment has been minimized.

Show comment
Hide comment
@sigmavirus24

sigmavirus24 Oct 31, 2014

Member

Probably also of interest: https://bugzil.la/1088998

Member

sigmavirus24 commented Oct 31, 2014

Probably also of interest: https://bugzil.la/1088998

@shazow

This comment has been minimized.

Show comment
Hide comment
@shazow

shazow Oct 31, 2014

Collaborator

I'm in a very remove-things mood, +1.

Collaborator

shazow commented Oct 31, 2014

I'm in a very remove-things mood, +1.

@alex

This comment has been minimized.

Show comment
Hide comment
@alex

alex Oct 31, 2014

Contributor

We should try to collect some numbers on how common this is before we move too quickly IMO, but I like the idea obviously!

Contributor

alex commented Oct 31, 2014

We should try to collect some numbers on how common this is before we move too quickly IMO, but I like the idea obviously!

@sigmavirus24

This comment has been minimized.

Show comment
Hide comment
@sigmavirus24

sigmavirus24 Nov 1, 2014

Member

I suspect @Lukasa will be more conservative, but yeah, it'd be nice have some numbers about this.

Member

sigmavirus24 commented Nov 1, 2014

I suspect @Lukasa will be more conservative, but yeah, it'd be nice have some numbers about this.

@shazow

This comment has been minimized.

Show comment
Hide comment
@shazow

shazow Nov 1, 2014

Collaborator

These numbers you speak of, where do we acquire them?

Collaborator

shazow commented Nov 1, 2014

These numbers you speak of, where do we acquire them?

@alex

This comment has been minimized.

Show comment
Hide comment
@alex

alex Nov 1, 2014

Contributor

We might be able to ask the Chromium folks for theirs, we could survey the
top 1 million sites or something like that, we could put a warning in and
see if anyone complains. I'm open to other ideas.

On Fri Oct 31 2014 at 7:04:18 PM Andrey Petrov notifications@github.com
wrote:

These numbers you speak of, where do we acquire them?


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

Contributor

alex commented Nov 1, 2014

We might be able to ask the Chromium folks for theirs, we could survey the
top 1 million sites or something like that, we could put a warning in and
see if anyone complains. I'm open to other ideas.

On Fri Oct 31 2014 at 7:04:18 PM Andrey Petrov notifications@github.com
wrote:

These numbers you speak of, where do we acquire them?


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

@sigmavirus24

This comment has been minimized.

Show comment
Hide comment
@sigmavirus24

sigmavirus24 Nov 1, 2014

Member

Deprecation Warning for the next release and see how many people report it as a bug?

Member

sigmavirus24 commented Nov 1, 2014

Deprecation Warning for the next release and see how many people report it as a bug?

@shazow

This comment has been minimized.

Show comment
Hide comment
@shazow

shazow Nov 1, 2014

Collaborator

I like the DeprecationWarning, let's do that.

I assume Chromium's threshold for deprecating is much more conservative than ours, so if they're doing it... On the other hand, our demographics are fairly different.

Collaborator

shazow commented Nov 1, 2014

I like the DeprecationWarning, let's do that.

I assume Chromium's threshold for deprecating is much more conservative than ours, so if they're doing it... On the other hand, our demographics are fairly different.

@GrahamDumpleton

This comment has been minimized.

Show comment
Hide comment
@GrahamDumpleton

GrahamDumpleton Dec 15, 2014

Note that false SSL certificate warnings may be seen if using Python 2.7.2 or older.

This is because in older Python versions the _ssl module does not return 'subjectAltName' in the set of fields in the SSL certificate. That the field isn't returned even if present means that even if the SSL certificate is valid, the check will fail and the warning logged.

To eliminate the warnings you have two choices. The first is to surround the API call to urllib3 which generates it with a warnings.catch_warnings() context manager and override the warnings filter for that context.

        import warnings

        with warnings.catch_warnings():
            warnings.simplefilter("ignore")
                r = http.request('GET', 'https://example.com/')

The alternative is to disable urllib3 warnings globally using:

import urllib3
urllib3.disable_warnings()

Note that if you are using the requests module, which contains a bundled version of urllib3 you will most likely need to use:

import requests.packages.urllib3
requests.packages.urllib3.disable_warnings()

The summary of this related problem has been copied over from #523 given that the warning generated refers to this ticket. For more information see #523.

GrahamDumpleton commented Dec 15, 2014

Note that false SSL certificate warnings may be seen if using Python 2.7.2 or older.

This is because in older Python versions the _ssl module does not return 'subjectAltName' in the set of fields in the SSL certificate. That the field isn't returned even if present means that even if the SSL certificate is valid, the check will fail and the warning logged.

To eliminate the warnings you have two choices. The first is to surround the API call to urllib3 which generates it with a warnings.catch_warnings() context manager and override the warnings filter for that context.

        import warnings

        with warnings.catch_warnings():
            warnings.simplefilter("ignore")
                r = http.request('GET', 'https://example.com/')

The alternative is to disable urllib3 warnings globally using:

import urllib3
urllib3.disable_warnings()

Note that if you are using the requests module, which contains a bundled version of urllib3 you will most likely need to use:

import requests.packages.urllib3
requests.packages.urllib3.disable_warnings()

The summary of this related problem has been copied over from #523 given that the warning generated refers to this ticket. For more information see #523.

@alex

This comment has been minimized.

Show comment
Hide comment
@alex

alex Dec 24, 2014

Contributor

@GrahamDumpleton perhaps it would make sense to change the warning text on 2.7.2, I think that should definitely still emit a warning though -- it's increasingly not practical to do TLS on the web without SANs, you can't even access pypi without them AFAIK /cc @dstufft

Contributor

alex commented Dec 24, 2014

@GrahamDumpleton perhaps it would make sense to change the warning text on 2.7.2, I think that should definitely still emit a warning though -- it's increasingly not practical to do TLS on the web without SANs, you can't even access pypi without them AFAIK /cc @dstufft

@shazow

This comment has been minimized.

Show comment
Hide comment
@shazow

shazow Dec 24, 2014

Collaborator

Definitely open to having different warning text for the affected versions. @GrahamDumpleton, what do you think? Proposals for such text/PRs are welcome.

Collaborator

shazow commented Dec 24, 2014

Definitely open to having different warning text for the affected versions. @GrahamDumpleton, what do you think? Proposals for such text/PRs are welcome.

@dstufft

This comment has been minimized.

Show comment
Hide comment
@dstufft

dstufft Dec 24, 2014

Contributor

You cannot access PyPI without SAN support that is correct, or most of the sites on Python's TLS.

Contributor

dstufft commented Dec 24, 2014

You cannot access PyPI without SAN support that is correct, or most of the sites on Python's TLS.

@BenjamenMeyer

This comment has been minimized.

Show comment
Hide comment
@BenjamenMeyer

BenjamenMeyer Jan 2, 2015

Contributor

@GrahamDumpleton better solution is to use Python's logging module and then redirect the information to the log:

import logging
logging.captureWarnings(True)

Works without having to do anything specific to urllib3 or Python Requests; and keeps the messages off the console so the user doesn't get annoyed by things that are out-of-their-control.

Contributor

BenjamenMeyer commented Jan 2, 2015

@GrahamDumpleton better solution is to use Python's logging module and then redirect the information to the log:

import logging
logging.captureWarnings(True)

Works without having to do anything specific to urllib3 or Python Requests; and keeps the messages off the console so the user doesn't get annoyed by things that are out-of-their-control.

@GrahamDumpleton

This comment has been minimized.

Show comment
Hide comment
@GrahamDumpleton

GrahamDumpleton Jan 2, 2015

@BenjamenMeyer Having it fill the logs is no better for us. We use with warnings.catch_warnings() around the one call in our code and make it go away entirely. No need to modify requests/urllib3 for that either.

GrahamDumpleton commented Jan 2, 2015

@BenjamenMeyer Having it fill the logs is no better for us. We use with warnings.catch_warnings() around the one call in our code and make it go away entirely. No need to modify requests/urllib3 for that either.

@BenjamenMeyer

This comment has been minimized.

Show comment
Hide comment
@BenjamenMeyer

BenjamenMeyer Jan 2, 2015

Contributor

@GrahamDumpleton another good resolution too.

I wasn't so much as referring to modifying requests/urllib3 as having to call:

requests.packages.urllib3.disable_warnings()

or

urllib3.disable_warnings()

Code specific to either library and probably removes more warnings than desired.

Contributor

BenjamenMeyer commented Jan 2, 2015

@GrahamDumpleton another good resolution too.

I wasn't so much as referring to modifying requests/urllib3 as having to call:

requests.packages.urllib3.disable_warnings()

or

urllib3.disable_warnings()

Code specific to either library and probably removes more warnings than desired.

@shazow

This comment has been minimized.

Show comment
Hide comment
@shazow

shazow Jan 2, 2015

Collaborator

@BenjamenMeyer Note that disable_warnings accepts a category that allows you to filter by whatever you wish. https://github.com/shazow/urllib3/blob/master/urllib3/__init__.py#L62

Collaborator

shazow commented Jan 2, 2015

@BenjamenMeyer Note that disable_warnings accepts a category that allows you to filter by whatever you wish. https://github.com/shazow/urllib3/blob/master/urllib3/__init__.py#L62

@BenjamenMeyer

This comment has been minimized.

Show comment
Hide comment
@BenjamenMeyer

BenjamenMeyer Jan 2, 2015

Contributor

@shazow And I would have to have intricate knowledge of urllib3 to figure out what was the right filter to apply. Even so, it's better to capture it in the logger which can also be filtered as desired at the application level.

Of course, I come from a school of thought of toss everything in the log and then filter it all together. I'd rather having a single setting for that than a dozen; and certainly not on a per-API level when an application may use multiple APIs to provide its functionalities.

Contributor

BenjamenMeyer commented Jan 2, 2015

@shazow And I would have to have intricate knowledge of urllib3 to figure out what was the right filter to apply. Even so, it's better to capture it in the logger which can also be filtered as desired at the application level.

Of course, I come from a school of thought of toss everything in the log and then filter it all together. I'd rather having a single setting for that than a dozen; and certainly not on a per-API level when an application may use multiple APIs to provide its functionalities.

@jcea

This comment has been minimized.

Show comment
Hide comment
@jcea

jcea Jan 21, 2015

I am getting this warning. I would love to have a recipe to create X.509 certificates with OpenSSL usable without "requests" warnings.

I the meantime I just downgraded to "requests" 2.4.3 :-(.

jcea commented Jan 21, 2015

I am getting this warning. I would love to have a recipe to create X.509 certificates with OpenSSL usable without "requests" warnings.

I the meantime I just downgraded to "requests" 2.4.3 :-(.

@sigmavirus24

This comment has been minimized.

Show comment
Hide comment
@sigmavirus24

sigmavirus24 Jan 21, 2015

Member

A quick search leads to a StackOverflow answer and other apparently similar resources. It isn't the role of requests or urllib3 to educate people on how to generate X.509 certificates.

Member

sigmavirus24 commented Jan 21, 2015

A quick search leads to a StackOverflow answer and other apparently similar resources. It isn't the role of requests or urllib3 to educate people on how to generate X.509 certificates.

@haikuginger

This comment has been minimized.

Show comment
Hide comment
@haikuginger

haikuginger Jan 11, 2018

Contributor

@sigmavirus24 @Lukasa @shazow,

Chrome 58 removed Common Name support as of April 19, 2017. I would like to propose that we define the remainder of our deprecation process as such:

  1. On or about April 19, 2018, we will release a version of urllib3 that will disregard Common Names unless a method to specifically enable support is called. Name is up in the air, but we want something very wordy that clearly conveys to anyone using it that it will be going away soon. Example:
    urllib3.enable_temporary_rfc2818_variance_certificate_common_name_support()
  2. On or about April 19, 2019, we will release a version of urllib3 that disables common name support altogether and only validates based on SAN.

I'm perfectly fine with skipping step 1 altogether if that has support from the rest of the maintenance team.

Contributor

haikuginger commented Jan 11, 2018

@sigmavirus24 @Lukasa @shazow,

Chrome 58 removed Common Name support as of April 19, 2017. I would like to propose that we define the remainder of our deprecation process as such:

  1. On or about April 19, 2018, we will release a version of urllib3 that will disregard Common Names unless a method to specifically enable support is called. Name is up in the air, but we want something very wordy that clearly conveys to anyone using it that it will be going away soon. Example:
    urllib3.enable_temporary_rfc2818_variance_certificate_common_name_support()
  2. On or about April 19, 2019, we will release a version of urllib3 that disables common name support altogether and only validates based on SAN.

I'm perfectly fine with skipping step 1 altogether if that has support from the rest of the maintenance team.

@sigmavirus24

This comment has been minimized.

Show comment
Hide comment
@sigmavirus24

sigmavirus24 Jan 11, 2018

Member

Step 1 I'm on board with but I think it should not be a global function like that. Instead I think it should be part of an SSLContext set-up that the user passes in that way it's per-connection.

Member

sigmavirus24 commented Jan 11, 2018

Step 1 I'm on board with but I think it should not be a global function like that. Instead I think it should be part of an SSLContext set-up that the user passes in that way it's per-connection.

@alex

This comment has been minimized.

Show comment
Hide comment
@alex

alex Jan 11, 2018

Contributor

👍 to everything. For historical reference, Firefox also requires a SAN https://hg.mozilla.org/mozilla-central/rev/dc40f46fae48

Contributor

alex commented Jan 11, 2018

👍 to everything. For historical reference, Firefox also requires a SAN https://hg.mozilla.org/mozilla-central/rev/dc40f46fae48

@tiran

This comment has been minimized.

Show comment
Hide comment
@tiran

tiran Jan 11, 2018

Contributor

@alex Is this also true for private CAs? Firefox used to require SAN only for public CAs in the public trust store (libnssckbi.so PKCS#11 provider), but not for private CAs.

Contributor

tiran commented Jan 11, 2018

@alex Is this also true for private CAs? Firefox used to require SAN only for public CAs in the public trust store (libnssckbi.so PKCS#11 provider), but not for private CAs.

@alex

This comment has been minimized.

Show comment
Hide comment
@alex

alex Jan 11, 2018

Contributor

For Chrome: By default: all. But there's an enterprise option to allow CNs (the option is being removed shortly).

For Firefox: Public and issued in 2016+ only.

Contributor

alex commented Jan 11, 2018

For Chrome: By default: all. But there's an enterprise option to allow CNs (the option is being removed shortly).

For Firefox: Public and issued in 2016+ only.

@Lukasa

This comment has been minimized.

Show comment
Hide comment
@Lukasa

Lukasa Jan 11, 2018

Contributor

+1 about changing. It does seem this should be scoped more tightly than global.

Contributor

Lukasa commented Jan 11, 2018

+1 about changing. It does seem this should be scoped more tightly than global.

@haikuginger

This comment has been minimized.

Show comment
Hide comment
@haikuginger

haikuginger Jan 11, 2018

Contributor

Okay, we'll scope it to SSLContext and require the manual creation of same along with passing it explicitly to the PoolManager. I'm fine with that; every bit of additional cognitive load that we add for this is a Good Thing IMO.

Contributor

haikuginger commented Jan 11, 2018

Okay, we'll scope it to SSLContext and require the manual creation of same along with passing it explicitly to the PoolManager. I'm fine with that; every bit of additional cognitive load that we add for this is a Good Thing IMO.

@alex

This comment has been minimized.

Show comment
Hide comment
@alex

alex Jan 11, 2018

Contributor

Cool, are you interested implementation this?

Contributor

alex commented Jan 11, 2018

Cool, are you interested implementation this?

@haikuginger

This comment has been minimized.

Show comment
Hide comment
@haikuginger

haikuginger Jan 11, 2018

Contributor

Unless another maintainer has time on their hands, I assumed I would be handling it. I'll want a couple reviews, though, to make sure we're not breaking anything in a hidden way.

Contributor

haikuginger commented Jan 11, 2018

Unless another maintainer has time on their hands, I assumed I would be handling it. I'll want a couple reviews, though, to make sure we're not breaking anything in a hidden way.

@alex

This comment has been minimized.

Show comment
Hide comment
@alex

alex Jan 11, 2018

Contributor
Contributor

alex commented Jan 11, 2018

@tiran

This comment has been minimized.

Show comment
Hide comment
@tiran

tiran Jan 11, 2018

Contributor

Sounds like I should expose X509_VERIFY_PARAM_set_hostflags() and X509_CHECK_FLAG_NEVER_CHECK_SUBJECT in 3.7, too.

Contributor

tiran commented Jan 11, 2018

Sounds like I should expose X509_VERIFY_PARAM_set_hostflags() and X509_CHECK_FLAG_NEVER_CHECK_SUBJECT in 3.7, too.

@alex

This comment has been minimized.

Show comment
Hide comment
@alex

alex Jan 11, 2018

Contributor

@tiran yes! If you CC me on the PR I can review

Contributor

alex commented Jan 11, 2018

@tiran yes! If you CC me on the PR I can review

@tiran

This comment has been minimized.

Show comment
Hide comment
@tiran

tiran Jan 11, 2018

Contributor

@alex First I have to get consent that we can drop support for OpenSSL < 1.0.2 and require an OpenSSL 1.0.2 compatible API (recent, non-broken LibreSSL).

So much work, so little time before beta freeze :/

Contributor

tiran commented Jan 11, 2018

@alex First I have to get consent that we can drop support for OpenSSL < 1.0.2 and require an OpenSSL 1.0.2 compatible API (recent, non-broken LibreSSL).

So much work, so little time before beta freeze :/

@tiran

This comment has been minimized.

Show comment
Hide comment
@tiran

tiran Jan 11, 2018

Contributor

Good news: OpenSSL 1.1.0 has X509_CHECK_FLAG_NEVER_CHECK_SUBJECT.

Bad news: It's only OpenSSL && 1.1.0+. If you are running LibreSSL or < OpenSSL 1.1.0, then the flag is not available. This affects Alpine, BSD, RHEL 7, Ubuntu LTS, Apple's macOS builds, and everybody else who thinks that LibreSSL is better than OpenSSL. It's not.

python/cpython#3462

Contributor

tiran commented Jan 11, 2018

Good news: OpenSSL 1.1.0 has X509_CHECK_FLAG_NEVER_CHECK_SUBJECT.

Bad news: It's only OpenSSL && 1.1.0+. If you are running LibreSSL or < OpenSSL 1.1.0, then the flag is not available. This affects Alpine, BSD, RHEL 7, Ubuntu LTS, Apple's macOS builds, and everybody else who thinks that LibreSSL is better than OpenSSL. It's not.

python/cpython#3462

@tiran

This comment has been minimized.

Show comment
Hide comment
@tiran

tiran Jan 18, 2018

Contributor

More bad news, LibreSSL doesn't even support the necessary APIs.

Contributor

tiran commented Jan 18, 2018

More bad news, LibreSSL doesn't even support the necessary APIs.

@alex

This comment has been minimized.

Show comment
Hide comment
@alex
Contributor

alex commented Jan 22, 2018

@scottbarron1313

This comment has been minimized.

Show comment
Hide comment
@scottbarron1313

scottbarron1313 Jan 23, 2018

Hi,
I'm using requests to make some api calls, and as of about a week ago, I get this warning message in terminal every time I make a call:

WARNING:requests.packages.urllib3.contrib.pyopenssl:A problem was encountered with the certificate that prevented urllib3 from finding the SubjectAlternativeName field. This can affect certificate validation. The error was Codepoint U+002A at position 1 of u'*' not allowed
/Library/Python/2.7/site-packages/requests/packages/urllib3/connection.py:337: SubjectAltNameWarning: Certificate for plasma-ticket.geo.apple.com 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.)

Sorry if this is off topic, or not the right place to ask, but since the error points to this thread I figure this is where I should start. Is there anything I can do to get rid of this message? It's mucking up my outputs, making it harder to read script results. Thanks.

scottbarron1313 commented Jan 23, 2018

Hi,
I'm using requests to make some api calls, and as of about a week ago, I get this warning message in terminal every time I make a call:

WARNING:requests.packages.urllib3.contrib.pyopenssl:A problem was encountered with the certificate that prevented urllib3 from finding the SubjectAlternativeName field. This can affect certificate validation. The error was Codepoint U+002A at position 1 of u'*' not allowed
/Library/Python/2.7/site-packages/requests/packages/urllib3/connection.py:337: SubjectAltNameWarning: Certificate for plasma-ticket.geo.apple.com 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.)

Sorry if this is off topic, or not the right place to ask, but since the error points to this thread I figure this is where I should start. Is there anything I can do to get rid of this message? It's mucking up my outputs, making it harder to read script results. Thanks.

@haikuginger

This comment has been minimized.

Show comment
Hide comment
@haikuginger

haikuginger Jan 23, 2018

Contributor

@scottbarron1313, can you open a new issue with details about the specific version of requests and pyopenssl you have installed? Thanks!

Contributor

haikuginger commented Jan 23, 2018

@scottbarron1313, can you open a new issue with details about the specific version of requests and pyopenssl you have installed? Thanks!

@scottbarron1313

This comment has been minimized.

Show comment
Hide comment
@scottbarron1313

scottbarron1313 commented Jan 23, 2018

@haikuginger sure thing.

wmfgerrit pushed a commit to wikimedia/puppet that referenced this issue Feb 28, 2018

puppetmaster: capture warnings in logging for naggen2
urllib3 on stretch issues a SAN warning while talking to nitrogen, have said warnings end up in
logging instead on stderr (and the icinga config)

/usr/lib/python2.7/dist-packages/urllib3/connection.py:337: SubjectAltNameWarning: Certificate for
nitrogen.eqiad.wmnet 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
urllib3/urllib3#497 for details.) SubjectAltNameWarning

Bug: T184562
Change-Id: I9fc8642d6d0ac86c2bd1a7657f7e775321da0f0e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment