On 23/03/17 04:35, Mody, Darshan (Darshan) wrote:
> Matt,
> 
> But openssl does not release the memory when it has duplicated the EC Key 
> which comes from the application

You mean it doesn't free the return value from the callback?
Unfortunately SSL_set_tmp_ecdh_callback() is undocumented so there is no
"official" description of the memory management semantics of this
function (and like I said it has been removed from later versions of
OpenSSL altogether so it is unlikely to ever get documented). However my
interpretation of the way the code is written is that this is a
deliberate design choice, i.e. it is deliberately left to the the
application to mange this memory. Presumably multiple invocations across
multiple connections could return the same value so it would be
inappropriate for OpenSSL to free it.

Matt



> 
>            /* Duplicate the ECDH structure. */
>             if (ecdhp == NULL) {
>                 SSLerr(SSL_F_SSL3_SEND_SERVER_KEY_EXCHANGE, ERR_R_ECDH_LIB);
>                 goto err;
>             }
>             if (s->cert->ecdh_tmp_auto)
>                 ecdh = ecdhp;
>             else if ((ecdh = EC_KEY_dup(ecdhp)) == NULL) { 
>                 SSLerr(SSL_F_SSL3_SEND_SERVER_KEY_EXCHANGE, ERR_R_ECDH_LIB);
>                 goto err;
>             }
> 
> Thanks
> Darshan
> 
> -----Original Message-----
> From: openssl-dev [mailto:openssl-dev-boun...@openssl.org] On Behalf Of Matt 
> Caswell
> Sent: Tuesday, March 21, 2017 3:28 PM
> To: openssl-dev@openssl.org
> Subject: Re: [openssl-dev] Memory leak in application when we use ECDH
> 
> 
> 
> On 21/03/17 09:46, Matt Caswell wrote:
>>
>> There is a potential leak in this case:
>>
>>             if (s->s3->tmp.ecdh != NULL) {
>>                 SSLerr(SSL_F_SSL3_SEND_SERVER_KEY_EXCHANGE,
>>                        ERR_R_INTERNAL_ERROR);
>>                 goto err;
>>             }
>>
>> But this is a "should not happen" scenario - so there is another bug 
>> if that is happening - and you would see "internal error" messages on 
>> the error stack.
>>
>> Another slight oddity in this code is the double check of ecdhp 
>> against NULL which seems redundant (but otherwise harmless):
>>
>>             if (ecdhp == NULL) {
>>                 al = SSL_AD_HANDSHAKE_FAILURE;
>>                 SSLerr(SSL_F_SSL3_SEND_SERVER_KEY_EXCHANGE,
>>                        SSL_R_MISSING_TMP_ECDH_KEY);
>>                 goto f_err;
>>             }
>>
>>             ...
>>
>>             /* Duplicate the ECDH structure. */
>>             if (ecdhp == NULL) {
>>                 SSLerr(SSL_F_SSL3_SEND_SERVER_KEY_EXCHANGE, ERR_R_ECDH_LIB);
>>                 goto err;
>>             }
> 
> Fix for the above issues (which is unlikely to solve your problem) is here:
> 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_openssl_openssl_pull_3003&d=DwICAg&c=BFpWQw8bsuKpl1SgiZH64Q&r=bsEULbVnjelD7InzgsegHBEbtXzaIDagy9EuEhJrKfQ&m=lmOlT993M2YueHJqZT9cKMDBkwGi-yB56pEUuki2qv8&s=pgqizfrjno8szLWBm_ROxbSePFpUYCO4KboURycC0no&e=
>  
> 
> Matt
> 
> --
> openssl-dev mailing list
> To unsubscribe: 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__mta.openssl.org_mailman_listinfo_openssl-2Ddev&d=DwICAg&c=BFpWQw8bsuKpl1SgiZH64Q&r=bsEULbVnjelD7InzgsegHBEbtXzaIDagy9EuEhJrKfQ&m=lmOlT993M2YueHJqZT9cKMDBkwGi-yB56pEUuki2qv8&s=jaW-ScgHUXwPTGLNdnt6AsNePpsg5n1Inju4e0V6SAs&e=
>  
> 
-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

Reply via email to