On Thursday 22 April 2010, Jan Just Keijser wrote: > > The only doubt I have is about error handling; in this case, if the > > allocation of the BIO fails, an error message is logged and nothing is > > done. Is this the right thing to do? > > I don't know if a FATAL error is such a good thing - not being able to > set an env var does not warrant a server crash , I'd say.
Ok, thanks. It's now a warning (with a clearer message as well). > Also, you're not freeing the BIO as far as I can tell. Right, well spotted! The variables are in a block which makes them local, but of course the memory pointed to by bio must be freed. Thanks for the heads-up. Please find attached the revised patch. -- D.
--- openvpn-2.1.1/ssl.c 2010-02-28 22:17:45.000000000 +0000 +++ openvpn-2.1.1-a/ssl.c 2010-04-22 22:33:40.000000000 +0100 @@ -788,9 +788,28 @@ verify_callback (int preverify_ok, X509_ /* export serial number as environmental variable */ { - const int serial = (int) ASN1_INTEGER_get (X509_get_serialNumber (ctx->current_cert)); + BIO *bio = NULL; + char serial[1024] = {0}; + int n; + + if ((bio = BIO_new (BIO_s_mem ())) == NULL) { + msg (M_WARN, "CALLBACK: Cannot create BIO (for tls_serial_%d)", ctx->error_depth); + } + else { + /* "prints" the serial number onto the BIO */ + i2a_ASN1_INTEGER(bio, X509_get_serialNumber (ctx->current_cert)); + n = BIO_read (bio, serial, sizeof (serial)-1); + if (n < 0) { + serial[0] = '\x0'; + } + else { + serial[n] = 0; + } + openvpn_snprintf (envname, sizeof(envname), "tls_serial_%d", ctx->error_depth); - setenv_int (opt->es, envname, serial); + setenv_str (opt->es, envname, serial); + BIO_free(bio); + } } /* export current untrusted IP */