Mailman 3 Proposal for better SSL errors - Python-Dev

newer
Re: [Python-Dev] cpython: simplify...

Proposal for better SSL errors

older
Hacking on the compiler in ways...

Antoine Pitrou

26 May 2012 26 May '12

7:28 p.m.

Hello, In http://bugs.python.org/issue14837 I have attached a proof-of-concept patch to improve the exceptions raised by the ssl module when OpenSSL signals an error. The current situation is quite dismal, since you get a sometimes cryptic error message with no viable opportunities for programmatic introspection:

...

...
...
ctx = ssl.SSLContext(ssl.PROTOCOL_TLSv1) ctx.verify_mode = ssl.CERT_REQUIRED sock = socket.create_connection(("svn.python.org", 443)) sock = ctx.wrap_socket(sock) Traceback (most recent call last): [...] ssl.SSLError: [Errno 1] _ssl.c:420: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed

SSLError instances only have a "errno" attribute which doesn't actually contain a meaningful value. With the posted patch, the above error becomes:

...

...
...
ctx = ssl.SSLContext(ssl.PROTOCOL_TLSv1) ctx.verify_mode = ssl.CERT_REQUIRED sock = socket.create_connection(("svn.python.org", 443)) sock = ctx.wrap_socket(sock) Traceback (most recent call last): [...] ssl.SSLError: [Errno 5] [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:494) [88296 refs]

Not only does the error string contain more valuable information (the mnemonics "SSL" and "CERTIFICATE_VERIFY_FAILED" indicate, respectively, in which subpart of OpenSSL and which precise error occurred), but they are also introspectable:

...

...
...
e = sys.last_value e.library 'SSL' e.reason 'CERTIFICATE_VERIFY_FAILED'

(these mnemonics correspond to OpenSSL's own #define'd numeric codes. I find it more Pythonic to expose the mnemonics than the numbers, though. Of course, the numbers <-> mnemnonics mappings can be separately exposed) You'll note there is still a "Errno 5" in that error message; I don't really know what to do with it. Hard-wiring the errno attribute to something like None *might* break existing software, although that would be unlikely since the current errno value is quite meaningless and confusing (it has nothing to do with POSIX errnos). To clarify a bit my request, I am asking for feedback on the principle more than on the implementation right now. Regards Antoine.

Show replies by date

Terry Reedy

26 May 26 May

9:44 p.m.

On 5/26/2012 3:28 PM, Antoine Pitrou wrote:

...

Hello,

In http://bugs.python.org/issue14837 I have attached a proof-of-concept patch to improve the exceptions raised by the ssl module when OpenSSL signals an error. The current situation is quite dismal, since you get a sometimes cryptic error message with no viable opportunities for programmatic introspection:

...
...
...
ctx = ssl.SSLContext(ssl.PROTOCOL_TLSv1) ctx.verify_mode = ssl.CERT_REQUIRED sock = socket.create_connection(("svn.python.org", 443)) sock = ctx.wrap_socket(sock) Traceback (most recent call last): [...] ssl.SSLError: [Errno 1] _ssl.c:420: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed

I agree that this is not easy to read ;-)

...

SSLError instances only have a "errno" attribute which doesn't actually contain a meaningful value.

With the posted patch, the above error becomes:

...
...
...
ctx = ssl.SSLContext(ssl.PROTOCOL_TLSv1) ctx.verify_mode = ssl.CERT_REQUIRED sock = socket.create_connection(("svn.python.org", 443)) sock = ctx.wrap_socket(sock) Traceback (most recent call last): [...] ssl.SSLError: [Errno 5] [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:494) [88296 refs]

Repeating the same reason in upper and lower case is unhelpful noise. Here is my suggested human-readable message. ssl.SSLError: in ssl sublibrary, certificate verify failed

...

Not only does the error string contain more valuable information (the mnemonics "SSL" and "CERTIFICATE_VERIFY_FAILED" indicate, respectively, in which subpart of OpenSSL and which precise error occurred), but they are also introspectable:

...
...
...
e = sys.last_value e.library 'SSL'

Not being up on ssl sublibraries, I would tend to think that means the main ssl library that gets imported. If that is wrong, .sublibrary would be clearer to me, but knowledgable users may be fine with it as it is.

...

...
...
...
e.reason 'CERTIFICATE_VERIFY_FAILED'

(these mnemonics correspond to OpenSSL's own #define'd numeric codes. I find it more Pythonic to expose the mnemonics than the numbers, though. Of course, the numbers<-> mnemnonics mappings can be separately exposed)

Python is not a 'minimize characters written' language and library. Inside an exception branch, if e.reason == 'CERTIFICATE_VERIFY_FAILED': is really clear, more so than any abbreviation.

...

You'll note there is still a "Errno 5" in that error message; I don't really know what to do with it. Hard-wiring the errno attribute to something like None *might* break existing software, although that would be unlikely since the current errno value is quite meaningless and confusing (it has nothing to do with POSIX errnos).

Given what you have written, I think the aim should be to get rid of it. If you want to be conservative and not just delete it now, give SSLError a __getattr__(self,name) method that looks for name == 'errno' and when so, issues a DeprecationWarning "SSLError.errno is meaningless and will be removed in the future. It is currently fixed at 0." before returning 0.

...

To clarify a bit my request, I am asking for feedback on the principle more than on the implementation right now.

My view: better exception data is good. The exception class is useful both to people and programs. The exception message is mainly for people in tracebacks for uncaught exceptions. Other attributes are mainly for programs that catch the exception and need more information than just the class. Exceptions, like SSLErrors, reporting external conditions that a program can respond to, are prime candidates for such attributes. +1 to this enhancement. -- Terry Jan Reedy

Antoine Pitrou

11:23 p.m.

On Sat, 26 May 2012 17:44:08 -0400 Terry Reedy wrote:

...

...
Traceback (most recent call last): [...] ssl.SSLError: [Errno 5] [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:494) [88296 refs]

Repeating the same reason in upper and lower case is unhelpful noise. Here is my suggested human-readable message.

ssl.SSLError: in ssl sublibrary, certificate verify failed

I should have made clearer that "certificate verify failed" is a string returned by OpenSSL. In this case it's exactly similar to the mnemonic, which might not always be the case (I honestly have not done any research on this point). Also, the mnemonic is useful to know which reason to test for, when seen in a traceback. (here I'd draw a parallel with POSIX errnos, where it has been a common request for years for OSError's str() to display the errno mnemonic, such as e.g. ENOENT, rather than its number)

...

...
...
...
...
e = sys.last_value e.library 'SSL'

Not being up on ssl sublibraries, I would tend to think that means the main ssl library that gets imported. If that is wrong, .sublibrary would be clearer to me, but knowledgable users may be fine with it as it is.

Well, it's called "library" in OpenSSL-speak, so I kept that name, but I am not particularly attached to it, so "sublibrary" could work too. As for what it means, "SSL" refers to the implementation of the SSL network protocol (or TLS), while other OpenSSL "libraries" cater with e.g. certificate management ("X509"), parsing of certificate files ("PEM"), etc.

...

...
You'll note there is still a "Errno 5" in that error message; I don't really know what to do with it. Hard-wiring the errno attribute to something like None *might* break existing software, although that would be unlikely since the current errno value is quite meaningless and confusing (it has nothing to do with POSIX errnos).

Given what you have written, I think the aim should be to get rid of it.

I also think it's desireable. Thanks for sharing your opinion Antoine.

Cameron Simpson

27 May 27 May

2 a.m.

On 26May2012 21:28, Antoine Pitrou wrote: | Not only does the error string contain more valuable information (the | mnemonics "SSL" and "CERTIFICATE_VERIFY_FAILED" indicate, respectively, | in which subpart of OpenSSL and which precise error occurred), but they | are also introspectable: | | >>> e = sys.last_value | >>> e.library | 'SSL' | >>> e.reason | 'CERTIFICATE_VERIFY_FAILED' | | (these mnemonics correspond to OpenSSL's own #define'd numeric codes. I | find it more Pythonic to expose the mnemonics than the numbers, though. | Of course, the numbers <-> mnemnonics mappings can be separately | exposed) Would you be inclined to exposed both? Eg add .ssl_errno (or whatever short name is conventionally used in the SSL library itself, just as "errno" matches the POSIX error code name). | You'll note there is still a "Errno 5" in that error message; I don't | really know what to do with it. Hard-wiring the errno attribute to | something like None *might* break existing software, although that | would be unlikely since the current errno value is quite meaningless | and confusing (it has nothing to do with POSIX errnos). It is EIO ("I/O error"), and not inappropriate for a communictions failure. I don't think POSIX prohibits other library functions from setting errno, either. Cheers, -- Cameron Simpson DoD#743 http://www.cskk.ezoshosting.com/cs/ Principles have no real force except when one is well fed. - Mark Twain

Antoine Pitrou

9:29 a.m.

On Sun, 27 May 2012 12:00:57 +1000 Cameron Simpson wrote:

...

On 26May2012 21:28, Antoine Pitrou wrote: | Not only does the error string contain more valuable information (the | mnemonics "SSL" and "CERTIFICATE_VERIFY_FAILED" indicate, respectively, | in which subpart of OpenSSL and which precise error occurred), but they | are also introspectable: | | >>> e = sys.last_value | >>> e.library | 'SSL' | >>> e.reason | 'CERTIFICATE_VERIFY_FAILED' | | (these mnemonics correspond to OpenSSL's own #define'd numeric codes. I | find it more Pythonic to expose the mnemonics than the numbers, though. | Of course, the numbers <-> mnemnonics mappings can be separately | exposed)

Would you be inclined to exposed both? Eg add .ssl_errno (or whatever short name is conventionally used in the SSL library itself, just as "errno" matches the POSIX error code name).

OpenSSL has a diversity of error codes. In this case there's the result code returned by OpenSSL's SSL_get_error(), which is 1 (SSL_ERROR_SSL) and is already recorded as "errno" (see below). There's the reason, as returned by OpenSSL's ERR_get_reason(), which is SSL_R_CERTIFICATE_VERIFY_FAILED. And I'm sure other oddities are lurking.

...

| You'll note there is still a "Errno 5" in that error message; I don't | really know what to do with it. Hard-wiring the errno attribute to | something like None *might* break existing software, although that | would be unlikely since the current errno value is quite meaningless | and confusing (it has nothing to do with POSIX errnos).

It is EIO ("I/O error"), and not inappropriate for a communictions failure.

That's a nice coincidence, but it's actually an OpenSSL-specific code. Also, there's a bug in the current patch, the right value should be 1 (SSL_ERROR_SSL) not 5. That said, I remember there's legacy code doing things like: except SSLError as e: if e.args[0] == SSL_ERROR_WANT_READ: ... so we can't ditch the errno, although in 3.3 you would write: except SSLWantReadError: ... Regards Antoine.

Cameron Simpson

28 May 28 May

2:52 a.m.

On 27May2012 11:29, Antoine Pitrou wrote: | On Sun, 27 May 2012 12:00:57 +1000 | Cameron Simpson wrote: | > On 26May2012 21:28, Antoine Pitrou wrote: | > | You'll note there is still a "Errno 5" in that error message; I don't | > | really know what to do with it. Hard-wiring the errno attribute to | > | something like None *might* break existing software, although that | > | would be unlikely since the current errno value is quite meaningless | > | and confusing (it has nothing to do with POSIX errnos). | > | > It is EIO ("I/O error"), and not inappropriate for a communictions failure. | | That's a nice coincidence, but it's actually an OpenSSL-specific code. Oh. -- Cameron Simpson DoD#743 http://www.cskk.ezoshosting.com/cs/ Do not taunt Happy Fun Coder.

Gregory P. Smith

27 May 27 May

2:31 a.m.

On Sat, May 26, 2012 at 12:28 PM, Antoine Pitrou wrote:

...

Hello,

In http://bugs.python.org/issue14837 I have attached a proof-of-concept patch to improve the exceptions raised by the ssl module when OpenSSL signals an error. The current situation is quite dismal, since you get a sometimes cryptic error message with no viable opportunities for programmatic introspection:

...
...
...
ctx = ssl.SSLContext(ssl.PROTOCOL_TLSv1) ctx.verify_mode = ssl.CERT_REQUIRED sock = socket.create_connection(("svn.python.org", 443)) sock = ctx.wrap_socket(sock) Traceback (most recent call last): [...] ssl.SSLError: [Errno 1] _ssl.c:420: error:14090086:SSL routines:SSL3_GET_SERVER_CERTIFICATE:certificate verify failed

SSLError instances only have a "errno" attribute which doesn't actually contain a meaningful value.

With the posted patch, the above error becomes:

...
...
...
ctx = ssl.SSLContext(ssl.PROTOCOL_TLSv1) ctx.verify_mode = ssl.CERT_REQUIRED sock = socket.create_connection(("svn.python.org", 443)) sock = ctx.wrap_socket(sock) Traceback (most recent call last): [...] ssl.SSLError: [Errno 5] [SSL: CERTIFICATE_VERIFY_FAILED] certificate verify failed (_ssl.c:494) [88296 refs]

Not only does the error string contain more valuable information (the mnemonics "SSL" and "CERTIFICATE_VERIFY_FAILED" indicate, respectively, in which subpart of OpenSSL and which precise error occurred), but they are also introspectable:

...
...
...
e = sys.last_value e.library 'SSL' e.reason 'CERTIFICATE_VERIFY_FAILED'

(these mnemonics correspond to OpenSSL's own #define'd numeric codes. I find it more Pythonic to expose the mnemonics than the numbers, though. Of course, the numbers <-> mnemnonics mappings can be separately exposed)

You'll note there is still a "Errno 5" in that error message; I don't really know what to do with it. Hard-wiring the errno attribute to something like None *might* break existing software, although that would be unlikely since the current errno value is quite meaningless and confusing (it has nothing to do with POSIX errnos).

To clarify a bit my request, I am asking for feedback on the principle more than on the implementation right now.

+1 I like it. It is better than what we have today. As for the misleading errno attribute, since it is not a posix errno I think it could be hard wired to 0 for SSL errors if and only if openssl is not actually setting it to something meaningful. The fact that an exception was raised is the error and what the exception was about in the case of SSL errors can come from your new library and reason attributes. There is a long term caveat to the overall approach: It still leaves the exception details being OpenSSL specific. If someone wants to ditch OpenSSL and use something else such as NSS (for example) in a future _ssl implementation what would its exception error info story look like? I would go ahead with this work regardless. It improves on what we have today. Defining a nicer way for SSL exceptions that is library agnostic is a larger project that should be done independent of making what we have today easier to work with. -gps

Antoine Pitrou

9:08 a.m.

On Sat, 26 May 2012 19:31:55 -0700 "Gregory P. Smith" wrote:

...

There is a long term caveat to the overall approach: It still leaves the exception details being OpenSSL specific. If someone wants to ditch OpenSSL and use something else such as NSS (for example) in a future _ssl implementation what would its exception error info story look like?

That's a general issue with the ssl module. Unless we come up with our own API and abstraction layer (which has a cost in design effort and risks), we're following the OpenSSL architecture (e.g. the SSLContext idea). Regards Antoine.

4350

Age (days ago)

4352

Last active (days ago)

List overview

Download

7 comments

4 participants

participants (4)

Antoine Pitrou
Cameron Simpson
Gregory P. Smith
Terry Reedy

Proposal for better SSL errors

Antoine Pitrou

Terry Reedy

Antoine Pitrou

Cameron Simpson

Antoine Pitrou

Cameron Simpson

Gregory P. Smith

Antoine Pitrou

tags

participants (4)