Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak because of using openssl encryption #323

Closed
vschreiner opened this issue Oct 26, 2015 · 3 comments
Closed

Memory leak because of using openssl encryption #323

vschreiner opened this issue Oct 26, 2015 · 3 comments
Labels
Milestone

Comments

@vschreiner
Copy link

@vschreiner vschreiner commented Oct 26, 2015

I experienced two memory leaks when I am using the rabbitmq-c with encrypted connection. I am using libssl. I was able to fix the leaks that occured in hostname_matches_subject_alt_name and amqp_ssl_socket_verify_hostname in amqp_openssl.c.

I changed the methods to look like follows:

static int hostname_matches_subject_alt_name(const char *hostname, X509 *cert)
{
  int found_any_entries = 0;
  int found_match;
  GENERAL_NAME *namePart = NULL;
  STACK_OF(GENERAL_NAME) *san =
    (STACK_OF(GENERAL_NAME)*) X509_get_ext_d2i(cert, NID_subject_alt_name, NULL, NULL);

  while (sk_GENERAL_NAME_num(san) > 0)
  {
    namePart = sk_GENERAL_NAME_pop(san);

    if (namePart->type == GEN_DNS) {
      found_any_entries = 1;
      found_match = match(namePart->d.uniformResourceIdentifier, hostname);
      if (found_match)
        return 1;
    }
  }

  return (found_any_entries ? 0 : -1);
}

and

static int
amqp_ssl_socket_verify_hostname(void *base, const char *host)
{
  struct amqp_ssl_socket_t *self = (struct amqp_ssl_socket_t *)base;
  int status = 0;
  X509 *cert=NULL;
  int res;
  cert = SSL_get_peer_certificate(self->ssl);
  if (!cert) {
    goto error;
  }
  res = hostname_matches_subject_alt_name(host, cert);
  if (res != 1) {
    res = hostname_matches_subject_common_name(host, cert);
    if (!res)
      goto error;
  }
exit:
  X509_free(cert);
  return status;
error:
  if(cert!=NULL)
      X509_free(cert);
  status = -1;
  goto exit;
}

After I recompiled librabbitmq-c and rerun the application using valgrind the memory leaks were gone.

kind regards

Volker Schreiner

AMAN Media GmbH
https://www.aman.de

alanxz added a commit that referenced this issue Oct 27, 2015
The cert object should be X509_free'd after use, it leaks otherwise.

Thanks Volker Schreiner for reporting this.

Fixes #323
@alanxz alanxz added the t:bug label Oct 27, 2015
@alanxz

This comment has been minimized.

Copy link
Owner

@alanxz alanxz commented Oct 27, 2015

I've figured out your changes in amqp_ssl_socket_verify_hostname.

What did you change about hostname_matches_subject_alt_name? The function looks identical to the current code.

alanxz added a commit that referenced this issue Oct 27, 2015
The cert object should be X509_free'd after use, it leaks otherwise.

Thanks Volker Schreiner for reporting this.

Fixes #323
@vschreiner

This comment has been minimized.

Copy link
Author

@vschreiner vschreiner commented Oct 27, 2015

I added the

X509_free(cert);

and

if(cert!=NULL)
X509_free(cert);

lines which were missing in the 0.7.1 release.

AMAN Media GmbH
https://www.aman.de

@alanxz alanxz closed this in #324 Oct 27, 2015
@alanxz

This comment has been minimized.

Copy link
Owner

@alanxz alanxz commented Oct 27, 2015

Fix is now in place. Thanks for reporting it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.