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

Jump to bottom

bpo-31870: add timeout parameter for get_server_certificate in ssl.py #4126

Closed
wants to merge 3 commits into from
Closed

bpo-31870: add timeout parameter for get_server_certificate in ssl.py #4126

wants to merge 3 commits into from

Conversation

nixawk
Copy link

@nixawk nixawk commented Oct 26, 2017 •
edited by bedevere-bot

The original get_server_certificate in ssl.py does not support socket timeout,

def get_server_certificate(addr, ssl_version=PROTOCOL_TLS, ca_certs=None):
    """Retrieve the certificate from the server at the specified address,
    and return it as a PEM-encoded string.
    If 'ca_certs' is specified, validate the server cert against it.
    If 'ssl_version' is specified, use it in the connection attempt."""

    host, port = addr
    if ca_certs is not None:
        cert_reqs = CERT_REQUIRED
    else:
        cert_reqs = CERT_NONE
    context = _create_stdlib_context(ssl_version,
                                     cert_reqs=cert_reqs,
                                     cafile=ca_certs)
    with  create_connection(addr) as sock:
        with context.wrap_socket(sock) as sslsock:
            dercert = sslsock.getpeercert(True)
    return DER_cert_to_PEM_cert(dercert)

After the patch, ssl.py can do it with a timeout parameter, ex: www.qq.com.

>>> import ssl
>>> ssl.get_server_certificate(("www.qq.com", 443), timeout=6)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python2.7/ssl.py", line 1017, in get_server_certificate
    with closing(create_connection(addr, timeout)) as sock:
  File "/usr/lib/python2.7/socket.py", line 575, in create_connection
    raise err
socket.error: [Errno 101] Network is unreachable

https://bugs.python.org/issue31870

@nixawk
add timeout for get_server_certificate in ssl.py
2c6a259 
"""
>>> import ssl
>>> ssl.get_server_certificate(("www.qq.com", 443), timeout=6)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/usr/lib/python2.7/ssl.py", line 1017, in get_server_certificate
    with closing(create_connection(addr, timeout)) as sock:
  File "/usr/lib/python2.7/socket.py", line 575, in create_connection
    raise err
socket.error: [Errno 101] Network is unreachable
"""
@the-knights-who-say-ni
Copy link

@the-knights-who-say-ni the-knights-who-say-ni commented Oct 26, 2017

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately our records indicate you have not signed the CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

Thanks again to your contribution and we look forward to looking at it!

@nixawk nixawk changed the title add timeout parameter for get_server_certificate in ssl.py bpo-31870: add timeout parameter for get_server_certificate in ssl.py Oct 26, 2017
tiran
tiran requested changes Nov 3, 2017
View changes
Copy link
Member

@tiran tiran left a comment

Please update documentation and provide a test case.

@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Nov 3, 2017

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@tiran tiran self-assigned this Nov 3, 2017
@nixawk nixawk force-pushed the nixawk-patch-1-get_server_certificate branch from b9ad20e to 97a551d Compare Nov 4, 2017
@corona10
Copy link
Member

@corona10 corona10 commented Dec 31, 2019

@nixawk Do you want to rebase this PR and continue to work with this?

@csabella
Copy link
Contributor

@csabella csabella commented Jan 26, 2020

Since this pull request seems to be abandoned, I'm going to close it. If the original author is interested in continuing to work on it, it can be reopened or someone else can create a new pull request to replace it. If the original author's work is used in a new PR, please give the appropriate credit.

@csabella csabella closed this Jan 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tiran tiran

Assignees

@tiran tiran

Labels
awaiting changes CLA signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@nixawk @the-knights-who-say-ni @bedevere-bot @corona10 @csabella @tiran