classification
Title: Missing SSLSocket.sendmsg() wrapper allows programs to send unencrypted data by mistake
Type: security Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: ncoghlan Nosy List: baikie, ncoghlan, pitrou, python-dev
Priority: high Keywords: patch

Created on 2011-08-24 19:11 by baikie, last changed 2011-08-27 14:02 by ncoghlan. This issue is now closed.

Files
File name Uploaded Description Edit
ssl_sendrecvmsg_notimplemented.diff baikie, 2011-08-24 19:11 Make SSLSocket.sendmsg/recvmsg/recvmsg_into() raise NotImplementedError review
ssl_sendrecvmsg_notimplemented-2.diff baikie, 2011-08-25 21:10 review
Messages (5)
msg142900 - (view) Author: David Watson (baikie) Date: 2011-08-24 19:11
Changeset fd10d042b41d removed the wrappers on ssl.SSLSocket for 
the new socket.send/recvmsg() methods (since I forgot to check 
for the existence of the underlying methods - see issue #6560), 
but this leaves SSLSocket with send/recvmsg() methods inherited 
from the underlying socket type; thus SSLSocket.sendmsg() will 
insert the given data into the stream without encrypting it (or 
wrapping it in SSL in any way). 
 
This immediately screws up the SSL connection, resulting in 
receive errors at both ends ("SSL3_GET_RECORD:wrong version 
number" and the like), but the data is clearly visible in a 
packet capture, so it's too late if it was actually something 
secret. 
 
Correspondingly, recvmsg() and recvmsg_into() return the 
encrypted data, and screw up the connection by removing it from 
the SSL stream. 
 
Of course, these methods don't make sense over SSL anyway, but if 
the programmer naively assumes they do, then ideally they should 
not expose any secret information. 
 
Attaching a patch implementing Antoine Pitrou's suggestion that 
the methods should simply raise NotImplementedError.  I don't 
know if these versions should also be added only if present on 
the underlying socket - they're Not Implemented either way :-)
msg142966 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-08-25 12:43
Adding an explanation message to the NotImplementedError would be more helpful. Otherwise, good catch.
msg142993 - (view) Author: David Watson (baikie) Date: 2011-08-25 21:10
On Thu 25 Aug 2011, Antoine Pitrou wrote:
> Adding an explanation message to the NotImplementedError would be more helpful. Otherwise, good catch.

OK, I've copied the messages from the ValueErrors the other
methods raise.
msg143000 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2011-08-26 04:54
As Antoine said, good catch. I'll be able to incorporate this in the next couple of days.
msg143066 - (view) Author: Roundup Robot (python-dev) Date: 2011-08-27 14:00
New changeset b06f011a3529 by Nick Coghlan in branch 'default':
Fix #12835: prevent use of the unencrypted sendmsg/recvmsg APIs on SSL wrapped sockets (Patch by David Watson)
http://hg.python.org/cpython/rev/b06f011a3529
History
Date User Action Args
2011-08-27 14:02:57ncoghlansetstatus: open -> closed
resolution: fixed
stage: resolved
2011-08-27 14:00:38python-devsetnosy: + python-dev
messages: + msg143066
2011-08-26 04:54:23ncoghlansetmessages: + msg143000
2011-08-25 22:27:33pitrousetassignee: ncoghlan
type: behavior -> security
2011-08-25 21:10:27baikiesetfiles: + ssl_sendrecvmsg_notimplemented-2.diff

messages: + msg142993
2011-08-25 12:43:36pitrousetmessages: + msg142966
2011-08-25 12:33:59neologixsetpriority: normal -> high
nosy: + ncoghlan, pitrou
type: behavior
2011-08-24 19:11:33baikiecreate