> SSLError: _ssl.c:296: Both the key & certificate files must be
> specified for server-side operation
> 
> I would change this behavior in SSLSocket constructor and raise
> ValueError if server_side is True and certfile is None.

Good idea.

> Also, the message coming from the C code should be adjusted to state
> than keyfile argument is not mandatory.

The message is arguably technically correct: you need both a (private)
key and a certificate. It's simply that they can be put in the same
file.

> >>> s = ssl.wrap_socket(socket.socket(), server_side=1)
> >>> s.connect(('blogger.com', 443))
> >>> 
> 
> For consistency I would expect something like ValueError("can't
> connect in server-side mode") on connect().

Indeed.

> ssl.SSLError: [Errno 336445442] _ssl.c:1604: error:140DC002:SSL
> routines:SSL_CTX_use_certificate_chain_file:system lib
> >>> 
> 
> A simple "IOError No such file or directory 'xxx'" exception would be
> a lot more clear.

Agreed, but the OpenSSL error reporting system looks too convoluted (or
braindead) to easily allow such aliasing of errors. If you have an idea,
don't hesitate to share :)

> ssl.SSLError: [Errno 336445449] _ssl.c:1604: error:140DC009:SSL
> routines:SSL_CTX_use_certificate_chain_file:PEM lib
> >>> 
> 
> If possible, the error should be more clear about what happened.
> Something like "malformed certfile was provided" or something.

Same as above: the error message and numeric code come from OpenSSL, not
from us.

>> A simple "IOError No such file or directory 'xxx'" exception would be
>> a lot more clear.
> Agreed, but the OpenSSL error reporting system looks too convoluted (or
> braindead) to easily allow such aliasing of errors. If you have an 
> idea, don't hesitate to share :)

The only idea which comes to mind is try to open() the file before calling load_cert_chain().
That would automatically also take care of permission errors, etc..
Not very clean, but... :-\

>> If possible, the error should be more clear about what happened.
>> Something like "malformed certfile was provided" or something.
> Same as above: the error message and numeric code come from OpenSSL, not
> from us.

No ideas here. I googled for some OpenSSL API to verify the certificate, which we can even possibly expose in ssl.py, but I couldn't find any. I guess we can't do nothing about this.

> The only idea which comes to mind is try to open() the file before calling load_cert_chain().
> That would automatically also take care of permission errors, etc..
> Not very clean, but... :-\

It's vulnerable to various issues such as race conditions (for example,
you open() the file while it still exists but it doesn't exist anymore
when OpenSSL opens it again).

A clean way to do this would be to use lower-level APIs such as
PEM_read_X509(), so that we can pass our own FILE* to OpenSSL. But it is
also much more code to write.

That said, have you checked the system errno at this point? Perhaps it
gives us enough information (if it hasn't been cleared by
OpenSSL... :/).

> No ideas here. I googled for some OpenSSL API to verify the
> certificate, which we can even possibly expose in ssl.py, but I
> couldn't find any.

I don't think that would change anything, since the verification APIs
would probably give you the exact same error message.

Fortunately errno was set.
Patch in attachment introduces the following changes:

=== 1 ===

Before:
>>> ssl.wrap_socket(socket.socket(), server_side=1)
>>>

Now:
>>> ssl.wrap_socket(socket.socket(), server_side=1)
ValueError: certfile must be specified for server-side operations


=== 2 ===

Before:
>>> s = ssl.wrap_socket(socket.socket(), server_side=1, certfile='Lib/test/keycert.pem')
>>> s.connect(('blogger.com', 443))
>>>

Now:
>>> s = ssl.wrap_socket(socket.socket(), server_side=1, certfile='Lib/test/keycert.pem')
>>> s.connect(('blogger.com', 443))
ValueError: can't connect in server-side mode


=== 3 ===

Before:
>>> os.path.exists('xxx')
False
>>> ssl.wrap_socket(socket.socket(), certfile='xxx')
ssl.SSLError: [Errno 336445442] _ssl.c:1604: error:140DC002:SSL routines:SSL_CTX_use_certificate_chain_file:system lib

Now:
>>> os.path.exists('xxx')
False
>>> ssl.wrap_socket(socket.socket(), certfile='xxx')
IOError: [Errno 2] No such file or directory


=== 4 ===

Before:
>>> os.path.exists('xxx')
False
>>> ctx = ssl.SSLContext(ssl.PROTOCOL_TLSv1)
>>> ctx.load_verify_locations('xxx')
ssl.SSLError: [Errno 185090050] _ssl.c:1676: error:0B084002:x509 certificate routines:X509_load_cert_crl_file:system lib

Now:
>>> os.path.exists('xxx')
False
>>> ctx = ssl.SSLContext(ssl.PROTOCOL_TLSv1)
>>> ctx.load_verify_locations('xxx')
IOError: [Errno 2] No such file or directory

>>> ctx = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
>>> ctx.wrap_socket(socket.socket(), server_side=1)
<ssl.SSLSocket object, fd=3, family=2, type=1, proto=0>
>>> 

I'm not sure how to raise ValueError("certfile must be specified") here as SSLContext class doesn't store certfile information, at least at Python level. Any hint?

> >>> ctx = ssl.SSLContext(ssl.PROTOCOL_SSLv23)
> >>> ctx.wrap_socket(socket.socket(), server_side=1)
> <ssl.SSLSocket object, fd=3, family=2, type=1, proto=0>
> >>> 
> 
> I'm not sure how to raise ValueError("certfile must be specified")
> here as SSLContext class doesn't store certfile information, at least
> at Python level. Any hint?

Actually, you shouldn't raise the error, since the certfile can be
specified after wrapping the socket.

@Antoine: ok, thanks.

This is now committed in r84352. See also r84351 which raises ValueError if non-zero flag argument is provided for sendall().
http://bugs.python.org/msg115166 was the original message at the top of this discussion which I accidentally removed.

> @Antoine: ok, thanks.
> 
> This is now committed in r84352.

I don't think it's ok to test for the IOError message ("No such file"),
because it comes from the OS and can therefore change from platform to
platform. Instead, you should check the value of the "errno" attribute
on the IOError objects.

Also:

+            except IOError as x:
+                if support.verbose:
+                    sys.stdout.write("\nsocket.error is %s\n" % str(x))

The message should be changed (IOError instead of socket.error).

You're right. Committed in r84355.

Some buildbots have started failing exactly after this commit:

======================================================================
ERROR: test_errors (test.test_ssl.BasicSocketTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/test/test_ssl.py", line 186, in test_errors
    s = ssl.wrap_socket(sock, server_side=True, certfile=CERTFILE)
  File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/ssl.py", line 414, in wrap_socket
    ciphers=ciphers)
  File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/ssl.py", line 137, in __init__
    self.context.load_cert_chain(certfile, keyfile)
ssl.SSLError: [Errno 185057381] _ssl.c:1609: error:0B07C065:x509 certificate routines:X509_STORE_add_cert:cert already in hash table

======================================================================
ERROR: test_protocol_sslv2 (test.test_ssl.ThreadedTests)
Connecting to an SSLv2 server with various client options
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/test/test_ssl.py", line 80, in f
    return func(*args, **kwargs)
  File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/test/test_ssl.py", line 1100, in test_protocol_sslv2
    try_protocol_combo(ssl.PROTOCOL_SSLv2, ssl.PROTOCOL_SSLv2, True)
  File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/test/test_ssl.py", line 969, in try_protocol_combo
    ctx.load_cert_chain(CERTFILE)
ssl.SSLError: [Errno 336445442] _ssl.c:1609: error:140DC002:SSL routines:SSL_CTX_use_certificate_chain_file:system lib

See e.g. http://www.python.org/dev/buildbot/builders/x86%20Ubuntu%203.x/builds/1870

This is probably dependent on the OpenSSL version.

Committed r84400 which should fix the first test failure.
I'll take a look at the buildbots to see how it goes.
As for the second failure I have no idea at the moment.