openssl / x509 memory leak issues.

initialization had a small leak (static size), verify had none, ocsp_verify had tons.

I hope this was all...
This commit is contained in:
Bernhard Amann 2014-05-19 14:36:36 -07:00
parent aa81825104
commit 604072f762

View file

@ -25,12 +25,14 @@ X509* d2i_X509_(X509** px, const u_char** in, int len)
} }
// construct an error record // construct an error record
RecordVal* x509_error_record(uint64_t num, const char* reason) RecordVal* x509_result_record(uint64_t num, const char* reason, Val* chainVector = 0)
{ {
RecordVal* rrecord = new RecordVal(BifType::Record::X509::Result); RecordVal* rrecord = new RecordVal(BifType::Record::X509::Result);
rrecord->Assign(0, new Val(num, TYPE_INT)); rrecord->Assign(0, new Val(num, TYPE_INT));
rrecord->Assign(1, new StringVal(reason)); rrecord->Assign(1, new StringVal(reason));
if ( chainVector )
rrecord->Assign(2, chainVector);
return rrecord; return rrecord;
} }
@ -59,6 +61,7 @@ X509_STORE* x509_get_root_store(TableVal* root_certs)
} }
X509_STORE_add_cert(ctx, x); X509_STORE_add_cert(ctx, x);
X509_free(x);
} }
delete idxs; delete idxs;
@ -169,16 +172,17 @@ function x509_get_certificate_string%(cert: opaque of x509, pem: bool &default=F
## x509_get_certificate_string x509_verify ## x509_get_certificate_string x509_verify
function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_certs: table_string_of_string, verify_time: time &default=network_time()%): X509::Result function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_certs: table_string_of_string, verify_time: time &default=network_time()%): X509::Result
%{ %{
BIO* bio_err = BIO_new_fp(stderr, BIO_NOCLOSE); RecordVal* rval = 0;
X509_STORE* ctx = x509_get_root_store(root_certs->AsTableVal()); X509_STORE* ctx = x509_get_root_store(root_certs->AsTableVal());
if ( ! ctx ) if ( ! ctx )
return x509_error_record(-1, "Problem initializing root store"); return x509_result_record(-1, "Problem initializing root store");
VectorVal *certs_vec = certs->AsVectorVal(); VectorVal *certs_vec = certs->AsVectorVal();
if ( certs_vec->Size() < 1 ) if ( certs_vec->Size() < 1 )
{ {
reporter->Error("No certificates given in vector"); reporter->Error("No certificates given in vector");
return x509_error_record(-1, "no certificates"); return x509_result_record(-1, "no certificates");
} }
// host certificate // host certificate
@ -187,7 +191,7 @@ function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_c
if ( ! sv ) if ( ! sv )
{ {
builtin_error("undefined value in certificate vector"); builtin_error("undefined value in certificate vector");
return x509_error_record(-1, "undefined value in certificate vector"); return x509_result_record(-1, "undefined value in certificate vector");
} }
file_analysis::X509Val* cert_handle = (file_analysis::X509Val*) sv; file_analysis::X509Val* cert_handle = (file_analysis::X509Val*) sv;
@ -196,27 +200,46 @@ function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_c
if ( ! cert ) if ( ! cert )
{ {
builtin_error(fmt("No certificate in opaque")); builtin_error(fmt("No certificate in opaque"));
return x509_error_record(-1, "No certificate in opaque"); return x509_result_record(-1, "No certificate in opaque");
} }
const unsigned char* start = ocsp_reply->Bytes(); const unsigned char* start = ocsp_reply->Bytes();
OCSP_RESPONSE *resp = d2i_OCSP_RESPONSE(NULL, &start, ocsp_reply->Len());
if ( ! resp )
return x509_error_record(-1, "Could not parse OCSP response");
int status = OCSP_response_status(resp);
if ( status != OCSP_RESPONSE_STATUS_SUCCESSFUL )
return x509_error_record(-2, OCSP_response_status_str(status));
OCSP_BASICRESP *basic = OCSP_response_get1_basic(resp);
if ( ! basic )
return x509_error_record(-1, "Could not parse OCSP response");
STACK_OF(X509)* untrusted_certs = x509_get_untrusted_stack(certs_vec); STACK_OF(X509)* untrusted_certs = x509_get_untrusted_stack(certs_vec);
if ( ! untrusted_certs ) if ( ! untrusted_certs )
return x509_error_record(-1, "Problem initializing list of untrusted certificates"); return x509_result_record(-1, "Problem initializing list of untrusted certificates");
// from here, always goto cleanup. Initialize all other required variables...
time_t vtime = (time_t) verify_time;
OCSP_BASICRESP *basic = 0;
OCSP_SINGLERESP *single = 0;
X509_STORE_CTX *csc = 0;
OCSP_CERTID *certid = 0;
int status = -1;
int out = -1;
int result = -1;
X509* issuer_certificate = 0;
OCSP_RESPONSE *resp = d2i_OCSP_RESPONSE(NULL, &start, ocsp_reply->Len());
if ( ! resp )
{
rval = x509_result_record(-1, "Could not parse OCSP response");
goto x509_ocsp_cleanup;
}
status = OCSP_response_status(resp);
if ( status != OCSP_RESPONSE_STATUS_SUCCESSFUL )
{
rval = x509_result_record(-2, OCSP_response_status_str(status));
goto x509_ocsp_cleanup;
}
basic = OCSP_response_get1_basic(resp);
if ( ! basic )
{
rval = x509_result_record(-1, "Could not parse OCSP response");
goto x509_ocsp_cleanup;
}
// the following code took me _forever_ to get right. // the following code took me _forever_ to get right.
// The OCSP_basic_verify command takes a list of certificates. However (which is not immediately // The OCSP_basic_verify command takes a list of certificates. However (which is not immediately
@ -225,10 +248,10 @@ function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_c
// inject the certificates in the certificate list of the OCSP reply, they actually are used during // inject the certificates in the certificate list of the OCSP reply, they actually are used during
// the lookup. // the lookup.
// Yay. // Yay.
X509* issuer_certificate = 0; issuer_certificate = 0;
for ( int i = 0; i < sk_X509_num(untrusted_certs); i++) for ( int i = 0; i < sk_X509_num(untrusted_certs); i++)
{ {
sk_X509_push(basic->certs, sk_X509_value(untrusted_certs, i)); sk_X509_push(basic->certs, X509_dup(sk_X509_value(untrusted_certs, i)));
if ( X509_NAME_cmp(X509_get_issuer_name(cert), X509_get_subject_name(sk_X509_value(untrusted_certs, i))) ) if ( X509_NAME_cmp(X509_get_issuer_name(cert), X509_get_subject_name(sk_X509_value(untrusted_certs, i))) )
issuer_certificate = sk_X509_value(untrusted_certs, i); issuer_certificate = sk_X509_value(untrusted_certs, i);
@ -236,28 +259,27 @@ function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_c
// Because we actually want to be able to give nice error messages that show why we were // Because we actually want to be able to give nice error messages that show why we were
// not able to verify the OCSP response - do our own verification logic first. // not able to verify the OCSP response - do our own verification logic first.
X509_STORE_CTX csc; csc = X509_STORE_CTX_new();
X509_STORE_CTX_init(&csc, ctx, sk_X509_value(basic->certs, 0), basic->certs); X509_STORE_CTX_init(csc, ctx, sk_X509_value(basic->certs, 0), basic->certs);
X509_STORE_CTX_set_time(&csc, 0, (time_t) verify_time); X509_STORE_CTX_set_time(csc, 0, (time_t) verify_time);
X509_STORE_CTX_set_purpose(&csc, X509_PURPOSE_OCSP_HELPER); X509_STORE_CTX_set_purpose(csc, X509_PURPOSE_OCSP_HELPER);
int result = X509_verify_cert(&csc); result = X509_verify_cert(csc);
if ( result != 1 ) if ( result != 1 )
{ {
const char *reason = X509_verify_cert_error_string(csc.error); const char *reason = X509_verify_cert_error_string((*csc).error);
X509_STORE_CTX_cleanup(&csc); rval = x509_result_record(result, X509_verify_cert_error_string((*csc).error));
sk_X509_free(untrusted_certs); goto x509_ocsp_cleanup;
return x509_error_record(result, X509_verify_cert_error_string(csc.error));
} }
int out = OCSP_basic_verify(basic, NULL, ctx, 0); out = OCSP_basic_verify(basic, NULL, ctx, 0);
if ( result < 1 ) if ( result < 1 )
{ {
X509_STORE_CTX_cleanup(&csc); rval = x509_result_record(out, ERR_error_string(ERR_get_error(),NULL));
sk_X509_free(untrusted_certs); goto x509_ocsp_cleanup;
return x509_error_record(out, ERR_error_string(ERR_get_error(),NULL));
} }
// ok, now we verified the OCSP response. This means that we have a valid chain tying it // ok, now we verified the OCSP response. This means that we have a valid chain tying it
// to a root that we trust and that the signature also hopefully is valid. This does not yet // to a root that we trust and that the signature also hopefully is valid. This does not yet
// mean that the ocsp response actually matches the certificate the server send us or that // mean that the ocsp response actually matches the certificate the server send us or that
@ -266,7 +288,6 @@ function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_c
// let's start this out by checking that the response is actually for the certificate we want // let's start this out by checking that the response is actually for the certificate we want
// to validate and not for something completely unrelated that the server is trying to trick us // to validate and not for something completely unrelated that the server is trying to trick us
// into accepting. // into accepting.
OCSP_CERTID *certid;
if ( issuer_certificate ) if ( issuer_certificate )
certid = OCSP_cert_to_id(NULL, cert, issuer_certificate); certid = OCSP_cert_to_id(NULL, cert, issuer_certificate);
@ -274,52 +295,80 @@ function x509_ocsp_verify%(certs: x509_opaque_vector, ocsp_reply: string, root_c
{ {
// issuer not in list sent by server, check store // issuer not in list sent by server, check store
X509_OBJECT obj; X509_OBJECT obj;
int lookup = X509_STORE_get_by_subject(&csc, X509_LU_X509, X509_get_subject_name(cert), &obj); int lookup = X509_STORE_get_by_subject(csc, X509_LU_X509, X509_get_subject_name(cert), &obj);
if ( lookup <= 0) if ( lookup <= 0)
{ {
sk_X509_free(untrusted_certs); rval = x509_result_record(lookup, "Could not find issuer of host certificate");
X509_STORE_CTX_cleanup(&csc); goto x509_ocsp_cleanup;
return x509_error_record(lookup, "Could not find issuer of host certificate");
} }
certid = OCSP_cert_to_id(NULL, cert, obj.data.x509); certid = OCSP_cert_to_id(NULL, cert, obj.data.x509);
} }
sk_X509_free(untrusted_certs);
X509_STORE_CTX_cleanup(&csc);
if ( ! certid ) if ( ! certid )
return x509_error_record(-1, "Certificate ID construction failed"); {
rval = x509_result_record(-1, "Certificate ID construction failed");
goto x509_ocsp_cleanup;
}
// for now, assume we have one reply... // for now, assume we have one reply...
OCSP_SINGLERESP *single = sk_OCSP_SINGLERESP_value(basic->tbsResponseData->responses, 0); single = sk_OCSP_SINGLERESP_value(basic->tbsResponseData->responses, 0);
if ( ! single ) if ( ! single )
return x509_error_record(-1, "Could not lookup OCSP response information"); {
rval = x509_result_record(-1, "Could not lookup OCSP response information");
goto x509_ocsp_cleanup;
}
if ( ! OCSP_id_cmp(certid, single->certId) ) if ( ! OCSP_id_cmp(certid, single->certId) )
return x509_error_record(-1, "OCSP reply is not for host certificate"); return x509_result_record(-1, "OCSP reply is not for host certificate");
// next - check freshness of proof... // next - check freshness of proof...
if ( ! ASN1_GENERALIZEDTIME_check(single->thisUpdate) || ! ASN1_GENERALIZEDTIME_check(single->nextUpdate) ) if ( ! ASN1_GENERALIZEDTIME_check(single->thisUpdate) || ! ASN1_GENERALIZEDTIME_check(single->nextUpdate) )
return x509_error_record(-1, "OCSP reply contains invalid dates"); {
rval = x509_result_record(-1, "OCSP reply contains invalid dates");
goto x509_ocsp_cleanup;
}
// now - nearly done. Check freshness and status code. // now - nearly done. Check freshness and status code.
// There is a function to check the freshness of the ocsp reply in the ocsp code of OpenSSL. But - it only // There is a function to check the freshness of the ocsp reply in the ocsp code of OpenSSL. But - it only
// supports comparing it against the current time, not against arbitrary times. Hence it is kind of unusable // supports comparing it against the current time, not against arbitrary times. Hence it is kind of unusable
// for us... // for us...
// Well, we will do it manually. // Well, we will do it manually.
time_t vtime = (time_t) verify_time;
if ( X509_cmp_time(single->thisUpdate, &vtime) > 0 ) if ( X509_cmp_time(single->thisUpdate, &vtime) > 0 )
return x509_error_record(-1, "OCSP reply specifies time in future"); rval = x509_result_record(-1, "OCSP reply specifies time in future");
else if ( X509_cmp_time(single->nextUpdate, &vtime) < 0 )
rval = x509_result_record(-1, "OCSP reply expired");
else if ( single->certStatus->type != V_OCSP_CERTSTATUS_GOOD )
rval = x509_result_record(-1, OCSP_cert_status_str(single->certStatus->type));
if ( X509_cmp_time(single->nextUpdate, &vtime) < 0 ) // if we have no error so far, we are done.
return x509_error_record(-1, "OCSP reply expired"); if ( !rval )
rval = x509_result_record(1, OCSP_cert_status_str(single->certStatus->type));
if ( single->certStatus->type != V_OCSP_CERTSTATUS_GOOD ) x509_ocsp_cleanup:
return x509_error_record(-1, OCSP_cert_status_str(single->certStatus->type));
return x509_error_record(1, OCSP_cert_status_str(single->certStatus->type)); if ( untrusted_certs )
sk_X509_free(untrusted_certs);
if ( resp )
OCSP_RESPONSE_free(resp);
if ( basic )
OCSP_BASICRESP_free(basic);
if ( csc )
{
X509_STORE_CTX_cleanup(csc);
X509_STORE_CTX_free(csc);
}
if ( certid )
OCSP_CERTID_free(certid);
return rval;
%} %}
## Verifies a certificate. ## Verifies a certificate.
@ -342,13 +391,14 @@ function x509_verify%(certs: x509_opaque_vector, root_certs: table_string_of_str
%{ %{
X509_STORE* ctx = x509_get_root_store(root_certs->AsTableVal()); X509_STORE* ctx = x509_get_root_store(root_certs->AsTableVal());
if ( ! ctx ) if ( ! ctx )
return x509_error_record(-1, "Problem initializing root store"); return x509_result_record(-1, "Problem initializing root store");
VectorVal *certs_vec = certs->AsVectorVal(); VectorVal *certs_vec = certs->AsVectorVal();
if ( ! certs_vec || certs_vec->Size() < 1 ) if ( ! certs_vec || certs_vec->Size() < 1 )
{ {
reporter->Error("No certificates given in vector"); reporter->Error("No certificates given in vector");
return x509_error_record(-1, "no certificates"); return x509_result_record(-1, "no certificates");
} }
// host certificate // host certificate
@ -357,21 +407,20 @@ function x509_verify%(certs: x509_opaque_vector, root_certs: table_string_of_str
if ( !sv ) if ( !sv )
{ {
builtin_error("undefined value in certificate vector"); builtin_error("undefined value in certificate vector");
return x509_error_record(-1, "undefined value in certificate vector"); return x509_result_record(-1, "undefined value in certificate vector");
} }
file_analysis::X509Val* cert_handle = (file_analysis::X509Val*) sv; file_analysis::X509Val* cert_handle = (file_analysis::X509Val*) sv;
X509* cert = cert_handle->GetCertificate(); X509* cert = cert_handle->GetCertificate();
if ( ! cert ) if ( ! cert )
{ {
builtin_error(fmt("No certificate in opaque")); builtin_error(fmt("No certificate in opaque"));
return x509_error_record(-1, "No certificate in opaque"); return x509_result_record(-1, "No certificate in opaque");
} }
STACK_OF(X509)* untrusted_certs = x509_get_untrusted_stack(certs_vec); STACK_OF(X509)* untrusted_certs = x509_get_untrusted_stack(certs_vec);
if ( ! untrusted_certs ) if ( ! untrusted_certs )
return x509_error_record(-1, "Problem initializing list of untrusted certificates"); return x509_result_record(-1, "Problem initializing list of untrusted certificates");
X509_STORE_CTX csc; X509_STORE_CTX csc;
X509_STORE_CTX_init(&csc, ctx, cert, untrusted_certs); X509_STORE_CTX_init(&csc, ctx, cert, untrusted_certs);
@ -406,11 +455,8 @@ function x509_verify%(certs: x509_opaque_vector, root_certs: table_string_of_str
else else
{ {
reporter->InternalWarning("OpenSSL returned null certificate"); reporter->InternalWarning("OpenSSL returned null certificate");
sk_X509_pop_free(chain, X509_free);
for ( int j = i + 1; i < num_certs; ++j ) goto x509_verify_chainerror;
X509_free(sk_X509_value(chain, j));
break;
} }
} }
@ -423,13 +469,7 @@ x509_verify_chainerror:
sk_X509_free(untrusted_certs); sk_X509_free(untrusted_certs);
RecordVal* rrecord = new RecordVal(BifType::Record::X509::Result); RecordVal* rrecord = x509_result_record(csc.error, X509_verify_cert_error_string(csc.error), chainVector);
rrecord->Assign(0, new Val((uint64) csc.error, TYPE_INT));
rrecord->Assign(1, new StringVal(X509_verify_cert_error_string(csc.error)));
if ( chainVector )
rrecord->Assign(2, chainVector);
return rrecord; return rrecord;
%} %}