Re: 003_ftp.patch, cert ref count

2014-04-12 Thread Jérémie Courrèges-Anglas
Mike Small sma...@panix.com writes:

 Was looking at
 http://ftp.openbsd.org/pub/OpenBSD/patches/5.5/common/003_ftp.patch.sig
 this last chunk...

 + if (ssl_verify) {
 + X509  *cert;
 +
 +   cert = SSL_get_peer_certificate(ssl);
 +  if (cert == NULL) {
 +  fprintf(ttyout, %s: no server 
 certificate\n,
 +  
 getprogname());
 + 
   goto cleanup_url_get;
 + 
   }
 +
 +   if (ssl_check_hostname(cert, host) != 0) {
 +  fprintf(ttyout, 
 %s: host `%s' not present in
 + 
 server certificate\n,
 + 
   getprogname(), host);
 + 
goto cleanup_url_get;
 + 
  }
 +
 +   X509_free(cert);
 }


 If that second check fails and you goto cleanup_url_get you skip
 X509_free(cert). Wouldn't that screw up the reference count?

Good catch.

 Or does
 that not matter after SSL_Shutdown and SSL_Free are called?

It does not matter because if the second check fails, we're going to
exit the process anyway, so a memory leak does not have an impact; I'm
more concerned by the ssl_ctx allocation, for which I already have
a diff.

Thanks,
-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



003_ftp.patch, cert ref count

2014-04-11 Thread Mike Small
Was looking at
http://ftp.openbsd.org/pub/OpenBSD/patches/5.5/common/003_ftp.patch.sig
this last chunk...

+ if (ssl_verify) {
+ X509  *cert;
+
+   cert = SSL_get_peer_certificate(ssl);
+  if (cert == NULL) {
+  fprintf(ttyout, %s: no server 
certificate\n,
+  
getprogname());
+   
goto cleanup_url_get;
+   
}
+
+   if (ssl_check_hostname(cert, host) != 0) {
+  fprintf(ttyout, %s: 
host `%s' not present in
+   
  server certificate\n,
+   
getprogname(), host);
+   
 goto cleanup_url_get;
+   
   }
+
+   X509_free(cert);
}


If that second check fails and you goto cleanup_url_get you skip
X509_free(cert). Wouldn't that screw up the reference count? Or does
that not matter after SSL_Shutdown and SSL_Free are called?