[Python-checkins] CVS: python/dist/src/Modules socketmodule.c,1.168,1.169

[Jeremy Hylton]

Update of /cvsroot/python/python/dist/src/Modules
In directory usw-pr-cvs1:/tmp/cvs-serv13517

Modified Files:
	socketmodule.c 
Log Message:
Fix two memory leaks in socket.ssl().

XXX [1] These changes aren't tested very thoroughly, because regrtest
doesn't do any SSL tests.  I've done some trivial tests on my own, but
don't really know how to use the key and cert files.  In one case, an
SSL-level error causes Python to dump core.  I'll get the fixed in the
next round of changes.

XXX [2] The checkin removes the x_attr member of the SSLObject struct.
I'm not sure if this is kosher for backwards compatibility at the
binary level.  Perhaps its safer to keep the member but keep it
assigned to NULL.

And the leaks?

newSSLObject() called PyDict_New(), stored the result in x_attr
without checking it, and later stored NULL in x_attr without doing
anything to the dict.  So the dict always leaks.  There is no further
reference to x_attr, so I just removed it completely.

The error cases in newSSLObject() passed the return value of
PyString_FromString() directly to PyErr_SetObject().
PyErr_SetObject() expects a borrowed reference, so the string leaked.


Index: socketmodule.c
===================================================================
RCS file: /cvsroot/python/python/dist/src/Modules/socketmodule.c,v
retrieving revision 1.168
retrieving revision 1.169
diff -C2 -d -r1.168 -r1.169
*** socketmodule.c	2001/08/29 21:37:09	1.168
--- socketmodule.c	2001/10/10 03:19:39	1.169
***************
*** 492,496 ****
  	PyObject_HEAD
  	PySocketSockObject *Socket;	/* Socket on which we're layered */
- 	PyObject 	*x_attr;	/* Attributes dictionary */
  	SSL_CTX* 	ctx;
  	SSL*     	ssl;
--- 492,495 ----
***************
*** 2500,2551 ****
  {
  	SSLObject *self;
  
  	self = PyObject_New(SSLObject, &SSL_Type); /* Create new object */
  	if (self == NULL){
! 		PyErr_SetObject(SSLErrorObject,
! 				PyString_FromString("newSSLObject error"));
! 		return NULL;
  	}
  	memset(self->server, '\0', sizeof(char) * 256);
  	memset(self->issuer, '\0', sizeof(char) * 256);
  
- 	self->x_attr = PyDict_New();
  	self->ctx = SSL_CTX_new(SSLv23_method()); /* Set up context */
  	if (self->ctx == NULL) {
! 		PyErr_SetObject(SSLErrorObject,
! 				PyString_FromString("SSL_CTX_new error"));
! 		PyObject_Del(self);
! 		return NULL;
  	}
  
! 	if ( (key_file && !cert_file) || (!key_file && cert_file) )
! 	{
! 		PyErr_SetObject(SSLErrorObject,
! 		      PyString_FromString(
! 			"Both the key & certificate files must be specified"));
! 		PyObject_Del(self);
! 		return NULL;
  	}
  
! 	if (key_file && cert_file)
! 	{
  		if (SSL_CTX_use_PrivateKey_file(self->ctx, key_file,
! 						SSL_FILETYPE_PEM) < 1)
! 		{
! 			PyErr_SetObject(SSLErrorObject,
! 				PyString_FromString(
! 				  "SSL_CTX_use_PrivateKey_file error"));
! 			PyObject_Del(self);
! 			return NULL;
  		}
  
  		if (SSL_CTX_use_certificate_chain_file(self->ctx,
! 						       cert_file) < 1)
! 		{
! 			PyErr_SetObject(SSLErrorObject,
! 				PyString_FromString(
! 				  "SSL_CTX_use_certificate_chain_file error"));
! 			PyObject_Del(self);
! 			return NULL;
  		}
  	}
--- 2499,2537 ----
  {
  	SSLObject *self;
+ 	PyObject *error = NULL;
  
  	self = PyObject_New(SSLObject, &SSL_Type); /* Create new object */
  	if (self == NULL){
! 		error =  PyString_FromString("newSSLObject error");
! 		goto fail;
  	}
  	memset(self->server, '\0', sizeof(char) * 256);
  	memset(self->issuer, '\0', sizeof(char) * 256);
  
  	self->ctx = SSL_CTX_new(SSLv23_method()); /* Set up context */
  	if (self->ctx == NULL) {
! 		error = PyString_FromString("SSL_CTX_new error");
! 		goto fail;
  	}
  
! 	if ((key_file && !cert_file) || (!key_file && cert_file)) {
! 		error = PyString_FromString(
! 			"Both the key & certificate files must be specified");
! 		goto fail;
  	}
  
! 	if (key_file && cert_file) {
  		if (SSL_CTX_use_PrivateKey_file(self->ctx, key_file,
! 						SSL_FILETYPE_PEM) < 1) {
! 			error = PyString_FromString(
! 				"SSL_CTX_use_PrivateKey_file error");
! 			goto fail;
  		}
  
  		if (SSL_CTX_use_certificate_chain_file(self->ctx,
! 						       cert_file) < 1) {
! 			error = PyString_FromString(
! 				"SSL_CTX_use_certificate_chain_file error");
! 			goto fail;
  		}
  	}
***************
*** 2557,2566 ****
  	SSL_set_connect_state(self->ssl);
  
  	if ((SSL_connect(self->ssl)) == -1) {
! 		/* Actually negotiate SSL connection */
! 		PyErr_SetObject(SSLErrorObject,
! 				PyString_FromString("SSL_connect error"));
! 		PyObject_Del(self);
! 		return NULL;
  	}
  	self->ssl->debug = 1;
--- 2543,2551 ----
  	SSL_set_connect_state(self->ssl);
  
+ 	/* Actually negotiate SSL connection */
+ 	/* XXX If SSL_connect() returns 0, it's also a failure. */
  	if ((SSL_connect(self->ssl)) == -1) {
! 		error = PyString_FromString("SSL_connect error");
! 		goto fail;
  	}
  	self->ssl->debug = 1;
***************
*** 2572,2579 ****
  				  self->issuer, 256);
  	}
- 	self->x_attr = NULL;
  	self->Socket = Sock;
  	Py_INCREF(self->Socket);
  	return self;
  }
  
--- 2557,2570 ----
  				  self->issuer, 256);
  	}
  	self->Socket = Sock;
  	Py_INCREF(self->Socket);
  	return self;
+  fail:
+ 	if (error) {
+ 		PyErr_SetObject(SSLErrorObject, error);
+ 		Py_DECREF(error);
+ 	}
+ 	Py_DECREF(self);
+ 	return NULL;
  }
  
***************
*** 2630,2634 ****
  	SSL_free(self->ssl);
  	SSL_CTX_free(self->ctx);
- 	Py_XDECREF(self->x_attr);
  	Py_XDECREF(self->Socket);
  	PyObject_Del(self);
--- 2621,2624 ----