Remove conditionals around crypto based free functions on relayd(8)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Remove conditionals around crypto based free functions on relayd(8)

Ricardo Mestre-2
Hi,

This removes the conditionals around crypto based free functions for relayd(8),
where appropriate. Sanity checked was already performed by tb@ briefly.

I was also preparing a similar diff for smtpd(8) last week, but deraadt@
already committed it as part of a larger diff.

OK?

Index: relay.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.219
diff -u -p -u -r1.219 relay.c
--- relay.c 2 Feb 2017 08:24:16 -0000 1.219
+++ relay.c 19 May 2017 10:04:15 -0000
@@ -1712,8 +1712,7 @@ relay_close(struct rsession *con, const
  SSL_shutdown(con->se_in.ssl);
  SSL_free(con->se_in.ssl);
  }
- if (con->se_in.tlscert != NULL)
- X509_free(con->se_in.tlscert);
+ X509_free(con->se_in.tlscert);
  if (con->se_in.s != -1) {
  close(con->se_in.s);
  if (con->se_out.s == -1) {
@@ -1738,8 +1737,7 @@ relay_close(struct rsession *con, const
  SSL_shutdown(con->se_out.ssl);
  SSL_free(con->se_out.ssl);
  }
- if (con->se_out.tlscert != NULL)
- X509_free(con->se_out.tlscert);
+ X509_free(con->se_out.tlscert);
  if (con->se_out.s != -1) {
  close(con->se_out.s);
 
@@ -2327,8 +2325,7 @@ relay_tls_connect(int fd, short event, v
     rlay->rl_tls_cacertx509);
  } else
  con->se_in.tlscert = NULL;
- if (servercert != NULL)
- X509_free(servercert);
+ X509_free(servercert);
  if (con->se_in.tlscert == NULL)
  relay_close(con, "could not create certificate");
  else
Index: relayd.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
retrieving revision 1.166
diff -u -p -u -r1.166 relayd.c
--- relayd.c 6 May 2017 19:44:53 -0000 1.166
+++ relayd.c 19 May 2017 10:04:15 -0000
@@ -534,8 +534,7 @@ purge_table(struct relayd *env, struct t
  free(host);
  }
  free(table->sendbuf);
- if (table->conf.flags & F_TLS)
- SSL_CTX_free(table->ssl_ctx);
+ SSL_CTX_free(table->ssl_ctx);
 
  if (head != NULL)
  TAILQ_REMOVE(head, table, entry);
@@ -578,22 +577,14 @@ purge_relay(struct relayd *env, struct r
  purge_key(&rlay->rl_tls_ca, rlay->rl_conf.tls_ca_len);
  purge_key(&rlay->rl_tls_cakey, rlay->rl_conf.tls_cakey_len);
 
- if (rlay->rl_tls_x509 != NULL) {
- X509_free(rlay->rl_tls_x509);
- rlay->rl_tls_x509 = NULL;
- }
- if (rlay->rl_tls_pkey != NULL) {
- EVP_PKEY_free(rlay->rl_tls_pkey);
- rlay->rl_tls_pkey = NULL;
- }
- if (rlay->rl_tls_cacertx509 != NULL) {
- X509_free(rlay->rl_tls_cacertx509);
- rlay->rl_tls_cacertx509 = NULL;
- }
- if (rlay->rl_tls_capkey != NULL) {
- EVP_PKEY_free(rlay->rl_tls_capkey);
- rlay->rl_tls_capkey = NULL;
- }
+ X509_free(rlay->rl_tls_x509);
+ rlay->rl_tls_x509 = NULL;
+ EVP_PKEY_free(rlay->rl_tls_pkey);
+ rlay->rl_tls_pkey = NULL;
+ X509_free(rlay->rl_tls_cacertx509);
+ rlay->rl_tls_cacertx509 = NULL;
+ EVP_PKEY_free(rlay->rl_tls_capkey);
+ rlay->rl_tls_capkey = NULL;
 
  SSL_CTX_free(rlay->rl_ssl_ctx);
 
Index: ssl.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/ssl.c,v
retrieving revision 1.31
diff -u -p -u -r1.31 ssl.c
--- ssl.c 9 Jan 2017 14:49:21 -0000 1.31
+++ ssl.c 19 May 2017 10:04:15 -0000
@@ -348,10 +348,9 @@ ssl_load_key(struct relayd *env, const c
  ssl_error(__func__, name);
 
  free(buf);
- if (bio != NULL)
- BIO_free_all(bio);
- if (key != NULL)
- EVP_PKEY_free(key);
+ BIO_free_all(bio);
+ EVP_PKEY_free(key);
+
  return (NULL);
 }
 
@@ -443,14 +442,10 @@ ssl_load_pkey(const void *data, size_t d
  return (1);
 
  fail:
- if (rsa != NULL)
- RSA_free(rsa);
- if (in != NULL)
- BIO_free(in);
- if (pkey != NULL)
- EVP_PKEY_free(pkey);
- if (x509 != NULL)
- X509_free(x509);
+ RSA_free(rsa);
+ BIO_free(in);
+ EVP_PKEY_free(pkey);
+ X509_free(x509);
  free(exdata);
 
  return (0);
@@ -479,12 +474,12 @@ ssl_ctx_fake_private_key(SSL_CTX *ctx, c
 
  if (pkeyptr != NULL)
  *pkeyptr = pkey;
- else if (pkey != NULL)
+ else
  EVP_PKEY_free(pkey);
 
  if (x509ptr != NULL)
  *x509ptr = x509;
- else if (x509 != NULL)
+ else
  X509_free(x509);
 
  return (ret);

Reply | Threaded
Open this post in threaded view
|

Re: Remove conditionals around crypto based free functions on relayd(8)

Theo Buehler
On Fri, May 19, 2017 at 11:18:00AM +0100, Ricardo Mestre wrote:
> Hi,
>
> This removes the conditionals around crypto based free functions for relayd(8),
> where appropriate. Sanity checked was already performed by tb@ briefly.
>
> I was also preparing a similar diff for smtpd(8) last week, but deraadt@
> already committed it as part of a larger diff.
>
> OK?

As I told you in private, this will interfere with claudio's libtls
patch. It's surely no fun for him to rebase his huge diff (or to resolve
the resulting conflicts), so I think this should wait.

Reply | Threaded
Open this post in threaded view
|

Re: Remove conditionals around crypto based free functions on relayd(8)

Ricardo Mestre-2
You're absolutely right, even if approved I won't commit until claudio@
rebases it with libtls.

On 12:53 Fri 19 May     , Theo Buehler wrote:

> On Fri, May 19, 2017 at 11:18:00AM +0100, Ricardo Mestre wrote:
> > Hi,
> >
> > This removes the conditionals around crypto based free functions for relayd(8),
> > where appropriate. Sanity checked was already performed by tb@ briefly.
> >
> > I was also preparing a similar diff for smtpd(8) last week, but deraadt@
> > already committed it as part of a larger diff.
> >
> > OK?
>
> As I told you in private, this will interfere with claudio's libtls
> patch. It's surely no fun for him to rebase his huge diff (or to resolve
> the resulting conflicts), so I think this should wait.
>

Reply | Threaded
Open this post in threaded view
|

Re: Remove conditionals around crypto based free functions on relayd(8)

Ricardo Mestre-2
In reply to this post by Ricardo Mestre-2
Hi,

After being prodded by benno@ please find below a new diff for relayd(8) due to
claudio@'s migration to libtls:

Comments? OK?

Index: relay.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.221
diff -u -p -u -r1.221 relay.c
--- relay.c 28 May 2017 10:39:15 -0000 1.221
+++ relay.c 19 Jun 2017 22:30:56 -0000
@@ -2130,10 +2130,8 @@ relay_tls_ctx_create(struct relay *rlay)
  purge_key(&rlay->rl_tls_cert, rlay->rl_conf.tls_cert_len);
  purge_key(&rlay->rl_tls_cacert, rlay->rl_conf.tls_cacert_len);
 
- if (rlay->rl_tls_client_cfg == NULL)
- tls_config_free(tls_client_cfg);
- if (rlay->rl_tls_cfg == NULL)
- tls_config_free(tls_cfg);
+ tls_config_free(tls_client_cfg);
+ tls_config_free(tls_cfg);
 
  return (0);
  err:
@@ -2212,8 +2210,7 @@ relay_tls_transaction(struct rsession *c
  errstr = "could not accept the TLS connection";
  goto err;
  }
- if (cre->tlscert != NULL)
- tls_free(tls_server);
+ tls_free(tls_server);
  flag = EV_READ;
  } else {
  cre->tls = tls_client();
Index: relayd.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
retrieving revision 1.169
diff -u -p -u -r1.169 relayd.c
--- relayd.c 31 May 2017 04:14:34 -0000 1.169
+++ relayd.c 19 Jun 2017 22:30:56 -0000
@@ -576,18 +576,12 @@ purge_relay(struct relayd *env, struct r
  purge_key(&rlay->rl_tls_ca, rlay->rl_conf.tls_ca_len);
  purge_key(&rlay->rl_tls_cakey, rlay->rl_conf.tls_cakey_len);
 
- if (rlay->rl_tls_pkey != NULL) {
- EVP_PKEY_free(rlay->rl_tls_pkey);
- rlay->rl_tls_pkey = NULL;
- }
- if (rlay->rl_tls_cacertx509 != NULL) {
- X509_free(rlay->rl_tls_cacertx509);
- rlay->rl_tls_cacertx509 = NULL;
- }
- if (rlay->rl_tls_capkey != NULL) {
- EVP_PKEY_free(rlay->rl_tls_capkey);
- rlay->rl_tls_capkey = NULL;
- }
+ EVP_PKEY_free(rlay->rl_tls_pkey);
+ rlay->rl_tls_pkey = NULL;
+ X509_free(rlay->rl_tls_cacertx509);
+ rlay->rl_tls_cacertx509 = NULL;
+ EVP_PKEY_free(rlay->rl_tls_capkey);
+ rlay->rl_tls_capkey = NULL;
 
  tls_free(rlay->rl_tls_ctx);
  tls_config_free(rlay->rl_tls_cfg);
Index: ssl.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/ssl.c,v
retrieving revision 1.33
diff -u -p -u -r1.33 ssl.c
--- ssl.c 28 May 2017 10:39:15 -0000 1.33
+++ ssl.c 19 Jun 2017 22:30:56 -0000
@@ -109,10 +109,8 @@ ssl_load_key(struct relayd *env, const c
 
  fail:
  free(buf);
- if (bio != NULL)
- BIO_free_all(bio);
- if (key != NULL)
- EVP_PKEY_free(key);
+ BIO_free_all(bio);
+ EVP_PKEY_free(key);
  return (NULL);
 }
 
@@ -195,12 +193,9 @@ ssl_update_certificate(const uint8_t *ol
 
 done:
  free(foo);
- if (in)
- BIO_free(in);
- if (out)
- BIO_free(out);
- if (cert)
- X509_free(cert);
+ BIO_free(in);
+ BIO_free(out);
+ X509_free(cert);
  return (newcert);
 }
 
@@ -252,14 +247,10 @@ ssl_load_pkey(void *data, char *buf, off
  return (1);
 
  fail:
- if (rsa != NULL)
- RSA_free(rsa);
- if (in != NULL)
- BIO_free(in);
- if (pkey != NULL)
- EVP_PKEY_free(pkey);
- if (x509 != NULL)
- X509_free(x509);
+ RSA_free(rsa);
+ BIO_free(in);
+ EVP_PKEY_free(pkey);
+ X509_free(x509);
  free(exdata);
 
  return (0);
@@ -318,10 +309,8 @@ ssl_ctx_fake_private_key(char *buf, off_
 fail:
  BIO_free(in);
 
- if (pkey != NULL)
- EVP_PKEY_free(pkey);
- if (x509 != NULL)
- X509_free(x509);
+ EVP_PKEY_free(pkey);
+ X509_free(x509);
 
  return (ret);
 }