Fix dlg memory leaks. #862
Conversation
| @@ -525,6 +525,7 @@ XrdCryptosslCipher::XrdCryptosslCipher(int bits, char *pub, | |||
| } | |||
| BIO_free(biop); | |||
| } | |||
| BN_free( bnpub ); | |||
| @@ -1045,6 +1045,7 @@ int XrdCryptosslX509SignProxyReq(XrdCryptoX509 *xcpi, XrdCryptoRSA *kcpi, | |||
| // Notify what we added | |||
| int crit = X509_EXTENSION_get_critical(xpiextdup); | |||
| DEBUG("added extension '"<<s<<"', critical: " << crit); | |||
| X509_EXTENSION_free( xpiextdup ); | |||
There was a problem hiding this comment.
Not sure about this. The extension is owned by the proxy certificate (xpo) ...
There was a problem hiding this comment.
My reasoning here was following:
-
X509_add_ext() is just a wrapper around X509v3_add_ext() (https://www.openssl.org/docs/man1.1.0/crypto/X509_add_ext.html)
-
from X509v3_add_ext() man:
X509v3_add_ext() adds extension ex to stack *x at position loc. If loc is -1 the new extension is added to the end. If *x is NULL a new stack will be allocated. The passed extension ex is duplicated internally so it must be freed after use.
(https://www.openssl.org/docs/man1.1.0/crypto/X509v3_add_ext.html)
- I also checked the source code of X509v3_add_ext() and indeed it does duplicate the extension
Question: maybe in this case it does not make sense to duplicate the object with X509_EXTENSION_dup on our side? (line 1040)
| @@ -1116,12 +1117,16 @@ int XrdCryptosslX509SignProxyReq(XrdCryptoX509 *xcpi, XrdCryptoRSA *kcpi, | |||
| PRINT("problem converting data for extension"); | |||
| return -kErrPX_Error; | |||
| } | |||
| PROXY_CERT_INFO_EXTENSION_free( pci ); | |||
| // Set extension name. | ||
| ASN1_OBJECT *obj = OBJ_txt2obj(gsiProxyCertInfo_OID, 1); | ||
| if (!obj || X509_EXTENSION_set_object(ext, obj) != 1) { | ||
| PRINT("could not set extension name"); | ||
| return -kErrPX_SetAttribute; | ||
| } | ||
| ASN1_OBJECT_free( obj ); |
| @@ -1141,6 +1146,9 @@ int XrdCryptosslX509SignProxyReq(XrdCryptoX509 *xcpi, XrdCryptoRSA *kcpi, | |||
| return -kErrPX_Signing; | |||
| } | |||
|
|
|||
| EVP_PKEY_free( ekpi ); // decrement reference counter | |||
| @@ -1141,6 +1146,9 @@ int XrdCryptosslX509SignProxyReq(XrdCryptoX509 *xcpi, XrdCryptoRSA *kcpi, | |||
| return -kErrPX_Signing; | |||
| } | |||
|
|
|||
| EVP_PKEY_free( ekpi ); // decrement reference counter | |||
| X509_EXTENSION_free( ext ); | |||
There was a problem hiding this comment.
As above, the extension should be owned by the proxy certificate ...
There was a problem hiding this comment.
As above ;-) X509_add_ext() duplicates the object internally ;-)
| @@ -1538,6 +1538,8 @@ XrdSecCredentials *XrdSecProtocolgsi::getCredentials(XrdSecParameters *parm, | |||
| if (bpar->UpdateBucket(bpub,lpub,kXRS_puk) != 0) | |||
| return ErrC(ei,bpar,bmai,0, kGSErrAddBucket, | |||
| XrdSutBuckStr(kXRS_puk),"global",stepstr); | |||
| delete[] bpub; // bpub is being duplicated inside of 'UpdateBucket' | |||
| @@ -3342,6 +3344,8 @@ int XrdSecProtocolgsi::ClientDoPxyreq(XrdSutBuffer *br, XrdSutBuffer **bm, | |||
| emsg = "problems signing the request"; | |||
| return 0; | |||
| } | |||
| delete req; | |||
| @@ -3350,6 +3354,7 @@ int XrdSecProtocolgsi::ClientDoPxyreq(XrdSutBuffer *br, XrdSutBuffer **bm, | |||
| return 0; | |||
| } | |||
| } | |||
| delete npxy; // has been allocated in *X509SignProxyReq | |||
No description provided.