From e89a316664cceda7fd3e105f876c159042625ef5 Mon Sep 17 00:00:00 2001 From: Michal Rokos Date: Sat, 8 Jun 2002 11:55:34 +0000 Subject: X509Cert: introduced GetX509CertPtr and DupX509CertPtr X509Ext: fixed memory leaking in ExtFactory after GC --- ChangeLog | 9 +++++++++ ossl_pkcs7.c | 25 +++++-------------------- ossl_ssl.c | 14 ++------------ ossl_x509.h | 3 ++- ossl_x509cert.c | 20 ++++++++++++-------- ossl_x509ext.c | 18 +++++++++++++++--- ossl_x509store.c | 11 ++--------- 7 files changed, 47 insertions(+), 53 deletions(-) diff --git a/ChangeLog b/ChangeLog index db1f322..0cf18f2 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,6 +3,15 @@ ChangeLog for ### CHANGE LOG ### +Sat, 8 Jun 2002 13:48:37 +0200 -- Michal Rokos + * x509.h: dropped ossl_x509_get_X509(obj) + * x509.h: introduced GetX509CertPtr(obj), DupX509CertPtr(obj) with the same semantics as explained for *PKeyPtr + * pkcs7.c: use these new funcs + * ssl.c: ditto. + * x509ext.c: ditto. + * x509store.c: ditto. + * x509ext.c: fix memory leaking after GC + Sat, 8 Jun 2002 11:54:13 +0200 -- Michal Rokos * pkey.h: dropped ossl_pkey_get_EVP_PKEY, ossl_pkey_get_private_EVP_PKEY * pkey.h: added GetPKeyPtr, GetPrivPKeyPtr, DupPrivPKeyPtr diff --git a/ossl_pkcs7.c b/ossl_pkcs7.c index 73487ed..b55fd98 100644 --- a/ossl_pkcs7.c +++ b/ossl_pkcs7.c @@ -111,18 +111,15 @@ static VALUE ossl_pkcs7_s_sign(VALUE klass, VALUE key, VALUE cert, VALUE data) StringValue(data); pkey = GetPrivPKeyPtr(key); * NO NEED TO DUP * - x509 = ossl_x509_get_X509(cert); + x509 = GetX509CertPtr(cert); * NO NEED TO DUP * if (!(bio = BIO_new_mem_buf(RSTRING(data)->ptr, RSTRING(data)->len))) { - X509_free(x509); OSSL_Raise(ePKCS7Error, ""); } if (!(pkcs7 = PKCS7_sign(x509, pkey, NULL, bio, 0))) { - X509_free(x509); BIO_free(bio); OSSL_Raise(ePKCS7Error, ""); } - X509_free(x509); BIO_free(bio); WrapPKCS7(cPKC7, obj, pkcs7); @@ -247,18 +244,15 @@ ossl_pkcs7_add_recipient(VALUE self, VALUE cert) GetPKCS7(self, pkcs7); - x509 = ossl_x509_get_X509(cert); + x509 = GetX509CertPtr(cert); /* NO NEED TO DUP */ if (!(ri = PKCS7_RECIP_INFO_new())) { - X509_free(x509); OSSL_Raise(ePKCS7Error, ""); } if (!PKCS7_RECIP_INFO_set(ri, x509)) { - X509_free(x509); PKCS7_RECIP_INFO_free(ri); OSSL_Raise(ePKCS7Error, ""); } - X509_free(x509); if (!PKCS7_add_recipient_info(pkcs7, ri)) { PKCS7_RECIP_INFO_free(ri); @@ -271,18 +265,12 @@ static VALUE ossl_pkcs7_add_certificate(VALUE self, VALUE cert) { PKCS7 *pkcs7; - X509 *x509; GetPKCS7(self, pkcs7); - x509 = ossl_x509_get_X509(cert); - - if (!PKCS7_add_certificate(pkcs7, x509)) { /* DUPs x509 - free it! */ - X509_free(x509); + if (!PKCS7_add_certificate(pkcs7, GetX509CertPtr(cert))) { /* NO NEED TO DUP */ OSSL_Raise(ePKCS7Error, ""); } - X509_free(x509); - return self; } @@ -429,7 +417,7 @@ ossl_pkcs7_data_decode(VALUE self, VALUE key, VALUE cert) OSSL_Check_Type(cert, cX509Cert); pkey = GetPrivPKeyPtr(key); /* NO NEED TO DUP */ - x509 = ossl_x509_get_X509(cert); + x509 = GetX509CertPtr(cert); /* NO NEED TO DUP */ if (!(bio = PKCS7_dataDecode(pkcs7, pkey, NULL, x509))) { X509_free(x509); @@ -496,15 +484,12 @@ ossl_pkcs7si_initialize(VALUE self, VALUE cert, VALUE key, VALUE digest) GetPKCS7si(self, p7si); pkey = GetPrivPKeyPtr(key); /* NO NEED TO DUP */ + x509 = GetX509CertPtr(cert); /* NO NEED TO DUP */ md = ossl_digest_get_EVP_MD(digest); - x509 = ossl_x509_get_X509(cert); if (!(PKCS7_SIGNER_INFO_set(p7si, x509, pkey, md))) { - X509_free(x509); OSSL_Raise(ePKCS7Error, ""); } - X509_free(x509); - return self; } diff --git a/ossl_ssl.c b/ossl_ssl.c index a483871..bb94a72 100644 --- a/ossl_ssl.c +++ b/ossl_ssl.c @@ -179,32 +179,24 @@ ssl_ctx_setup(VALUE self) /* private key may be bundled in certificate file. */ val = ssl_get_cert(self); - cert = NIL_P(val) ? NULL : ossl_x509_get_X509(val); + cert = NIL_P(val) ? NULL : GetX509CertPtr(val); /* NO DUP NEEDED */ val = ssl_get_key(self); key = NIL_P(val) ? NULL : GetPKeyPtr(val); /* NO NEED TO DUP */ if (cert && key) { if (!SSL_CTX_use_certificate(p->ctx, cert)) { /* Adds a ref => Safe to FREE */ - X509_free(cert); OSSL_Raise(eSSLError, "SSL_CTX_use_certificate:"); } if (!SSL_CTX_use_PrivateKey(p->ctx, key)) { /* Adds a ref => Safe to FREE */ - X509_free(cert); OSSL_Raise(eSSLError, "SSL_CTX_use_PrivateKey:"); } if (!SSL_CTX_check_private_key(p->ctx)) { - X509_free(cert); OSSL_Raise(eSSLError, "SSL_CTX_check_private_key:"); } } - /* - * Free cert, key (Used => Safe to FREE || Not used => Not needed) - */ - if (cert) X509_free(cert); - val = ssl_get_ca(self); - ca = NIL_P(val) ? NULL : ossl_x509_get_X509(val); + ca = NIL_P(val) ? NULL : GetX509CertPtr(val); /* NO DUP NEEDED. */ val = ssl_get_ca_file(self); ca_file = NIL_P(val) ? NULL : RSTRING(val)->ptr; val = ssl_get_ca_path(self); @@ -212,10 +204,8 @@ ssl_ctx_setup(VALUE self) if (ca) { if (!SSL_CTX_add_client_CA(p->ctx, ca)) { /* Copies X509_NAME => FREE it. */ - X509_free(ca); OSSL_Raise(eSSLError, ""); } - X509_free(ca); } if ((!SSL_CTX_load_verify_locations(p->ctx, ca_file, ca_path) || !SSL_CTX_set_default_verify_paths(p->ctx))) { diff --git a/ossl_x509.h b/ossl_x509.h index e23e3ee..7a59a75 100644 --- a/ossl_x509.h +++ b/ossl_x509.h @@ -36,7 +36,8 @@ extern VALUE eX509CertError; VALUE ossl_x509_new(X509 *); VALUE ossl_x509_new_from_file(VALUE); -X509 *ossl_x509_get_X509(VALUE); +X509 *GetX509CertPtr(VALUE); +X509 *DupX509CertPtr(VALUE); void Init_ossl_x509cert(void); /* diff --git a/ossl_x509cert.c b/ossl_x509cert.c index e60c23b..492bd65 100644 --- a/ossl_x509cert.c +++ b/ossl_x509cert.c @@ -79,16 +79,20 @@ ossl_x509_new_from_file(VALUE filename) } X509 * -ossl_x509_get_X509(VALUE obj) +GetX509CertPtr(VALUE obj) { - X509 *x509, *new; - + X509 *x509; SafeGetX509(obj, x509); - - if (!(new = X509_dup(x509))) { - OSSL_Raise(eX509CertError, ""); - } - return new; + return x509; +} + +X509 * +DupX509CertPtr(VALUE obj) +{ + X509 *x509; + SafeGetX509(obj, x509); + CRYPTO_add(&x509->references,1,CRYPTO_LOCK_X509); + return x509; } /* diff --git a/ossl_x509ext.c b/ossl_x509ext.c index 5bdccbb..94fe710 100644 --- a/ossl_x509ext.c +++ b/ossl_x509ext.c @@ -28,7 +28,7 @@ } while (0) #define MakeX509ExtFactory(klass, obj, ctx) \ - obj = Data_Make_Struct(klass, X509V3_CTX, 0, CRYPTO_free, ctx) + obj = Data_Make_Struct(klass, X509V3_CTX, 0, ossl_x509extfactory_free, ctx) #define GetX509ExtFactory(obj, ctx) do { \ Data_Get_Struct(obj, X509V3_CTX, ctx); \ if (!ctx) { \ @@ -84,6 +84,18 @@ ossl_x509ext_get_X509_EXTENSION(VALUE obj) /* * Ext factory */ +static void +ossl_x509extfactory_free(X509V3_CTX *ctx) +{ + if (ctx) { + if (ctx->issuer_cert) X509_free(ctx->issuer_cert); + if (ctx->subject_cert) X509_free(ctx->subject_cert); + if (ctx->crl) X509_CRL_free(ctx->crl); + if (ctx->subject_req) X509_REQ_free(ctx->subject_req); + OPENSSL_free(ctx); + } +} + static VALUE ossl_x509extfactory_s_allocate(VALUE klass) { @@ -102,7 +114,7 @@ ossl_x509extfactory_set_issuer_cert(VALUE self, VALUE cert) GetX509ExtFactory(self, ctx); - ctx->issuer_cert = ossl_x509_get_X509(cert); + ctx->issuer_cert = DupX509CertPtr(cert); /* DUP NEEDED */ return cert; } @@ -114,7 +126,7 @@ ossl_x509extfactory_set_subject_cert(VALUE self, VALUE cert) GetX509ExtFactory(self, ctx); - ctx->subject_cert = ossl_x509_get_X509(cert); + ctx->subject_cert = DupX509CertPtr(cert); /* DUP NEEDED */ return cert; } diff --git a/ossl_x509store.c b/ossl_x509store.c index e0714b7..19b9ee2 100644 --- a/ossl_x509store.c +++ b/ossl_x509store.c @@ -200,15 +200,11 @@ ossl_x509store_add_trusted(VALUE self, VALUE cert) GetX509Store(self, storep); - OSSL_Check_Type(cert, cX509Cert); - x509 = ossl_x509_get_X509(cert); + x509 = DupX509CertPtr(cert); /* DUP NEEDED!!! */ if (!X509_STORE_add_cert(storep->store->ctx, x509)) { - X509_free(x509); OSSL_Raise(eX509StoreError, ""); } - X509_free(x509); - return cert; } @@ -318,14 +314,11 @@ static VALUE ossl_x509store_verify(VALUE self, VALUE cert) { ossl_x509store *storep = NULL; - X509 *x509 = NULL; int result = 0; GetX509Store(self, storep); - OSSL_Check_Type(cert, cX509Cert); - x509 = ossl_x509_get_X509(cert); - X509_STORE_CTX_set_cert(storep->store, x509); + X509_STORE_CTX_set_cert(storep->store, GetX509CertPtr(cert)); /* NO DUP NEEDED. */ result = X509_verify_cert(storep->store); /*X509_STORE_CTX_cleanup(storep->store); *clears chain*/ -- cgit v1.2.3