Serial number export, fixed
PATCH 1/1 Serial number export, fixed
ssl.c:
correctly manage and export serial numbers of any size (as parsed by OpenSSL) into the environment. Set to empty string in case of errors, as 0 and negative numbers are all possible (although illegal) certificate serial numbers. Use an OpenSSL BIO object to do the job.
openvpn.8:
Added some notes about $tls_serial_{n} format and usage to the existing description.
contrib/OCSP_check.sh:
New barebone script to demonstrate how to use $tls_serial_{n} to perform simple OCSP queries using OpenSSL command line "openssl ocsp".
--- contrib/OCSP_check.sh | 67 +++++++++++++++++++++++++++++++++++++++++++++++++ openvpn.8 | 7 ++++- ssl.c | 26 +++++++++++++++++-- 3 files changed, 96 insertions(+), 4 deletions(-) create mode 100755 contrib/OCSP_check.sh
contrib/OCSP_check.sh new file +67 -0
@@ -0,0 +1,67 @@ +#!/bin/sh + +# Sample script to perform OCSP queries with OpenSSL +# given a certificate serial number. + +# If you run your own CA, you can set up a very simple +# OCSP server using the -port option to "openssl ocsp". + +# Full documentation and examples: +# http://www.openssl.org/docs/apps/ocsp.html + + +# Edit the following values to suit your needs +# *** NO SPACES IN PATHS ANYWHERE *** + +# OCSP responder URL (mandatory) +ocsp_url="http://some.ocsp.server/" +#ocsp_url="https://some.secure.ocsp.server/" + +# Path to issuer certificate (mandatory) +issuer="-issuer /path/to/CAcert.crt" + +# use a nonce in the query, set to "-no_nonce" to not use it +nonce="-nonce" + +# Verify the response? +verify="-CAfile /path/to/CAcert.crt" +# you can also use -CApath +# If you do not want to verify, set the following +#verify="-noverify" + +# Depth in the certificate chain where the cert to verify is. +# Set to -1 to run the verification at every level (NOTE that +# in that case you need a more complex script as the various +# parameters for the query will likely be different at each level) +# "0" is the usual value here, where the client certificate is +check_depth=0 + +cur_depth=$1 # this is the *CURRENT* depth +common_name=$2 # in case you need it + +# begin +if [ $check_depth -eq -1 ] || [ $cur_depth -eq $check_depth ]; then + eval serial="\$tls_serial_${cur_depth}" + + # Check that the serial is not empty + if [ -n "$serial" ]; then + + # This is only an example; you are encouraged to run this command (without + # redirections) manually against your or your CA's OCSP server to see how + # it responds, and adapt accordingly. + # Sample output: + # + # Response verify OK + # 0x428740A5: good + # This Update: Apr 24 19:38:49 2010 GMT + # Next Update: May 2 14:23:42 2010 GMT + + openssl ocsp $issuer \ + $nonce \ + $verify \ + -url "$ocsp_url" \ + -serial "0x${serial}" >/dev/null 2>&1 + else + exit 1 + fi +fi
openvpn.8 +6 -1
@@ -5321,7 +5321,12 @@ where is the verification level. Only set for TLS connections. Set prior to execution of .B --tls-verify -script. +script. This is in the form of a hex string like "37AB46E0", which is +suitable for doing serial-based OCSP queries (with OpenSSL, you have +to prepend "0x" to the string). If something goes wrong while reading +the value from the certificate it will be an empty string, so your +code should check that. +See the contrib/OCSP_check.sh script for an example. .\"********************************************************* .TP .B tun_mtu
ssl.c +23 -3
@@ -788,9 +788,29 @@ verify_callback (int preverify_ok, X509_STORE_CTX * ctx)/* export serial number as environmental variable */ { -const int serial = (int )ASN1_INTEGER_get (X509_get_serialNumber (ctx->current_cert)); -openvpn_snprintf (envname,sizeof (envname),"tls_serial_%d" , ctx->error_depth); -setenv_int (opt->es, envname, serial); +BIO *bio =NULL ; +char serial[100 ]; +int n; + +CLEAR (serial); +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 and read it back */ +if ( (i2a_ASN1_INTEGER (bio,X509_get_serialNumber (ctx->current_cert)) >=0 ) && + (( n =BIO_read (bio, serial,sizeof (serial)-1 ) ) >=0 ) ) { + serial[n] =0 ; + } +else { +msg (M_WARN ,"CALLBACK: Error reading/writing BIO (for tls_serial_%d)" , ctx->error_depth); + serial[0 ] =0 ;/* empty string */ + } + +openvpn_snprintf (envname,sizeof (envname),"tls_serial_%d" , ctx->error_depth); +setenv_str (opt->es, envname, serial); +BIO_free (bio); + } }/* export current untrusted IP */
Signature · 1 line
1.6.5.6
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 11 lines · Davide Brini
ssl.c:
correctly manage and export serial numbers of any size (as parsed by OpenSSL) into the environment. Set to empty string in case of errors, as 0 and negative numbers are all possible (although illegal) certificate serial numbers. Use an OpenSSL BIO object to do the job.openvpn.8:
Added some notes about $tls_serial_{n} format and usage to the existing description.
Quoted 6 lines
contrib/OCSP_check.sh:
New barebone script to demonstrate how to use $tls_serial_{n} to perform simple OCSP queries using OpenSSL command line "openssl ocsp".Signed-off-by Davide Brini <[email protected]...>
Hi Davide,
Thanks for sending an updated patch! This is looking a lot better. However, I have a few comments to this patch as well. But this one is getting closer to be accepted :)
Btw! Very good idea by introducing the OCSP_check.sh example! And even a proper git patch! I like that :)
Quoted 21 lines
--- contrib/OCSP_check.sh | 67 +++++++++++++++++++++++++++++++++++++++++++++++++ openvpn.8 | 7 ++++- ssl.c | 26 +++++++++++++++++-- 3 files changed, 96 insertions(+), 4 deletions(-) create mode 100755 contrib/OCSP_check.shcontrib/OCSP_check.sh new file +8
-0@@ -0,0 +1,67 @@ +#!/bin/sh + +# Sample script to perform OCSP queries with OpenSSL +# given a certificate serial number. + +# If you run your own CA, you can set up a very simple +# OCSP server using the -port option to "openssl ocsp". +
contrib/OCSP_check.sh line 18
Quoted 10 lines
contrib/OCSP_check.sh new file +10
-0+# Full documentation and examples: +# http://www.openssl.org/docs/apps/ocsp.html + + +# Edit the following values to suit your needs +# *** NO SPACES IN PATHS ANYWHERE *** + +# OCSP responder URL (mandatory) +ocsp_url="http://some.ocsp.server/" +#ocsp_url="https://some.secure.ocsp.server/"
Wouldn't it be better to use a more valid URL? Or at least uncomment both of them ... and have a check that this variable is set? If unset, exit with error.
Quoted 81 lines
contrib/OCSP_check.sh new file +49
-0+ +# Path to issuer certificate (mandatory) +issuer="-issuer /path/to/CAcert.crt" + +# use a nonce in the query, set to "-no_nonce" to not use it +nonce="-nonce" + +# Verify the response? +verify="-CAfile /path/to/CAcert.crt" +# you can also use -CApath +# If you do not want to verify, set the following +#verify="-noverify" + +# Depth in the certificate chain where the cert to verify is. +# Set to -1 to run the verification at every level (NOTE that +# in that case you need a more complex script as the various +# parameters for the query will likely be different at each level) +# "0" is the usual value here, where the client certificate is +check_depth=0 + +cur_depth=$1 # this is the *CURRENT* depth +common_name=$2 # in case you need it + +# begin +if [ $check_depth -eq -1 ] || [ $cur_depth -eq $check_depth ]; then + eval serial="\$tls_serial_${cur_depth}" + + # Check that the serial is not empty + if [ -n "$serial" ]; then + + # This is only an example; you are encouraged to run this command (without + # redirections) manually against your or your CA's OCSP server to see how + # it responds, and adapt accordingly. + # Sample output: + # + # Response verify OK + # 0x428740A5: good + # This Update: Apr 24 19:38:49 2010 GMT + # Next Update: May 2 14:23:42 2010 GMT + + openssl ocsp $issuer \ + $nonce \ + $verify \ + -url "$ocsp_url" \ + -serial "0x${serial}" >/dev/null 2>&1 + else + exit 1 + fi +fiopenvpn.8 +6
-1@@ -5321,7 +5321,12 @@ where is the verification level. Only set for TLS connections. Set prior to execution of .B --tls-verify -script. +script. This is in the form of a hex string like "37AB46E0", which is +suitable for doing serial-based OCSP queries (with OpenSSL, you have +to prepend "0x" to the string). If something goes wrong while reading +the value from the certificate it will be an empty string, so your +code should check that. +See the contrib/OCSP_check.sh script for an example. .\"********************************************************* .TP .B tun_mtussl.c +3
-3@@ -788,9 +788,29 @@ verify_callback (int preverify_ok, X509_STORE_CTX * ctx)/* export serial number as environmental variable */ { -const int serial = (int )ASN1_INTEGER_get (X509_get_serialNumber (ctx->current_cert)); -openvpn_snprintf (envname,sizeof (envname),"tls_serial_%d" , ctx->error_depth); -setenv_int (opt->es, envname, serial); +BIO *bio =NULL ; +char serial[100 ]; +int n;
ssl.c verify_callback (int preverify_ok, X509_STORE_CTX * ctx) · line 803
Quoted 10 lines
ssl.c +10
-0+ +CLEAR (serial); +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 and read it back */ +if ( (i2a_ASN1_INTEGER (bio,X509_get_serialNumber (ctx->current_cert)) >=0 ) && + (( n =BIO_read (bio, serial,sizeof (serial)-1 ) ) >=0 ) ) { + serial[n] =0 ;
This should really not be needed when you've CLEAR()'ed serial earlier on. You should trust that BIO_read() do not exceed it's limits or place other garbage after the value.
ssl.c verify_callback (int preverify_ok, X509_STORE_CTX * ctx) · line 807
Quoted 4 lines
ssl.c +4
-0+ } +else { +msg (M_WARN ,"CALLBACK: Error reading/writing BIO (for tls_serial_%d)" , ctx->error_depth); + serial[0 ] =0 ;/* empty string */
Hmmm ... My first thought was that this should not be needed. On second thought, I realised it really might be needed - as BIO_read() *might* have put some data here before it failed. So if that can happen, some data which has been parsed badly into memory is still available via the 'serial' string.
On the other hand, this might not be that critical, as setenv_str() is pretty safe and will obey the first byte in the string. So a mem-info-leak is more difficult to manage, as this variable is only available inside this function.
But consider to replace this "empty string" with yet another CLEAR(), that's a more defensive approach. This part of the code should hopefulyl not be called much, unless something is really wrong - which means that we can afford to spend a little extra time clearing this region. Or another approach is to only use setenv_str() using the 'serial' string only on success ... and otherwise setenv_str(opt->es, envname, "\0") (might even be that NULL would work as well).
I'm not convinced about any particular solution here now ... but I'm open for other thoughts about this as well.
ssl.c verify_callback (int preverify_ok, X509_STORE_CTX * ctx) · line 816
Quoted 9 lines
ssl.c +6
-0+ } + +openvpn_snprintf (envname,sizeof (envname),"tls_serial_%d" , ctx->error_depth); +setenv_str (opt->es, envname, serial); +BIO_free (bio); + } }/* export current untrusted IP */
One last comment is on the coding style. This patch breaks the coding style (yeah, I admit it - even I've been arrested on this one quite lately). Please correct that as well ... the current coding style (like it or not) is based on:
if (boolean expression)
{
do_something();
}
else
{
do_something_else();
}To keep the code as clean and nicely as possible, we should try to follow the current coding style. If you don't like the current standard, lets discuss that first :)
Thanks again, Davide ... I hope I'm not pushing you too hard, but this is a good improvement from the last patch, so this progress is making me happy! :)
kind regards,
David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkvUzKIACgkQDC186MBRfrqkgACgqZmCi6Sh0++tVmLCrQKRsZS0 OrsAoIz1CndbkfFCNRO4HPYxW3NHgUkC
=z2nY
-----END PGP SIGNATURE-----
Quoted 2 lines · On Monday 26 Apr 2010 00:13:39 David Sommerseth wrote:
Btw! Very good idea by introducing the
OCSP_check.shexample! And even a proper git patch! I like that :)
Thanks!
Quoted 5 lines
contrib/OCSP_check.sh line 18
Quoted 3 lines
contrib/OCSP_check.sh new file +3
-0+# OCSP responder URL (mandatory) +ocsp_url="http://some.ocsp.server/" +#ocsp_url="https://some.secure.ocsp.server/"Wouldn't it be better to use a more valid URL?
And what could it be? I can put in the OCSP URL of some well-known CA, but then if the user is using their own server, that would be meaningless anyway. But I'm open to suggestions.
Quoted 1 line
Or at least uncomment both of them
Uncommenting both of them makes no sense, as the second assignment overrides the first, so it would be like just uncommenting the second one. They are meant to be mutually incompatible.
Quoted 1 line
... and have a check that this variable is set? If unset, exit with error.
Ah well, as I said the script is meant to be a barebone skeleton to demonstrate basic usage. That is by no means the only thing that lacks proper error checking in the script, so if we go down that route there's a lot to be changed.
The users are supposed to adapt it to their needs and make it more robust.
Quoted 9 lines
ssl.c verify_callback (int preverify_ok, X509_STORE_CTX * ctx) · line 803
Quoted 5 lines
(quoted diff) +2
-0+ /* "prints" the serial number onto the BIO and read it back */ + if ( (i2a_ASN1_INTEGER(bio, X509_get_serialNumber(
ctx->current_cert)) >= 0) &&ssl.c +2
-0+ (( n = BIO_read (bio, serial, sizeof (serial)-1) ) >= 0) ) { + serial[n] =0 ;This should really not be needed when you've CLEAR()'ed serial earlier on. You should trust that
BIO_read()do not exceed it's limits or place other garbage after the value.
Is it really a good idea? You correctly speak of defensiveness later, so why give avay some safety just to avoid an assignment? What harm does it make to leave it in? Doesn't look like an expensive operation to me.
I'd say actually, for maximum safety, we should set to 0 all the bytes from the n'th all the way to the end of "serial".
Quoted 13 lines
Quoted 6 lines
ssl.c +3
-0+ } +else { + msg (M_WARN, "CALLBACK: Error reading/writing BIO (fortls_serial_%d)",
ctx->error_depth);
+ serial[0] = 0; /* empty
string */Hmmm ... My first thought was that this should not be needed. On second thought, I realised it really might be needed - as
BIO_read()*might* have put some data here before it failed. So if that can happen, some data which has been parsed badly into memory is still available via the 'serial' string.
Quoted 7 lines
On the other hand, this might not be that critical, as
setenv_str()is pretty safe and will obey the first byte in the string. So a mem-info-leak is more difficult to manage, as this variable is only available inside this function.But consider to replace this "empty string" with yet another
CLEAR(), that's a more defensive approach.
Agreed, will do that.
Quoted 5 lines
This part of the code should hopefulyl not be called much, unless something is really wrong - which means that we can afford to spend a little extra time clearing this region. Or another approach is to only use
setenv_str()using the 'serial' string only on success ... and otherwise setenv_str(opt-Quoted 1 line
es, envname, "\0") (might even be that NULL would work as well).
How would that differ from what I'm doing now? It will still end up being an empty string, but with the downside that we would have two separate calls to setenv_str() to perform what is a single logical action (ie set the environment variable).
Quoted 2 lines
I'm not convinced about any particular solution here now ... but I'm open for other thoughts about this as well.
I thought about another option: leaving the environment variable unset (ie, do NOT call setenv_str() at all if there are errors) rather than set it to an empty string.
The problem with this is that in a shell script is a bit cumbersome to differentitate between an /empty/ variable and an /unset/ variable, and this is because the difference does not usually matter. It definitely doesn't in our case.
Another option I thought of is to set the variable to something that could never be a valid serial number, eg "XXXXXXXXX" or something like that. Ugly as hell, if you ask me. And would /still/ require that the user of the variable check what's in it, so checking that it's not empty or checking that it's not "XXXXXXXXX" does not make much difference (and the former allows for more idiomatic code).
Bottom line: I'd still go with the empty variable solution.
Quoted 14 lines
One last comment is on the coding style. This patch breaks the coding style (yeah, I admit it - even I've been arrested on this one quite lately). Please correct that as well ... the current coding style (like it or not) is based on:
if (boolean expression) { do_something(); } else { do_something_else(); }
Quoted 3 lines
To keep the code as clean and nicely as possible, we should try to follow the current coding style. If you don't like the current standard, lets discuss that first :)
I don't have any problem with whatever standard is in place, so I can surely reformat the code to the preferred style.
But if I have to say it all, I would say that it doesn't look like that style is applied consistently throughout the code (eg see verify_cert_eku() in ssl.c) although, now that you mention it, I must say that the vast majority of the code does use it, so it makes sense to stick to it.
Quoted 3 lines
Thanks again, Davide ... I hope I'm not pushing you too hard, but this is a good improvement from the last patch, so this progress is making me happy! :)
No problem, thanks for the constructive criticism! If you agree, I will implement che changes you suggest in the C code and resend the patch.
Signature · 1 line
D.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 1 line · Davide Brini
On Monday 26 Apr 2010 00:13:39 David Sommerseth wrote:
[...snip...]
Quoted 16 lines
Quoted 5 lines
contrib/OCSP_check.sh line 18
Quoted 3 lines
contrib/OCSP_check.sh new file +3
-0+# OCSP responder URL (mandatory) +ocsp_url="http://some.ocsp.server/" +#ocsp_url="https://some.secure.ocsp.server/"Wouldn't it be better to use a more valid URL?
And what could it be? I can put in the OCSP URL of some well-known CA, but then if the user is using their own server, that would be meaningless anyway. But I'm open to suggestions.
Quoted 1 line
Or at least uncomment both of them
Uncommenting both of them makes no sense, as the second assignment overrides the first, so it would be like just uncommenting the second one. They are meant to be mutually incompatible.
Quoted 7 lines
Quoted 1 line
... and have a check that this variable is set? If unset, exit with error.
Ah well, as I said the script is meant to be a barebone skeleton to demonstrate basic usage. That is by no means the only thing that lacks proper error checking in the script, so if we go down that route there's a lot to be changed.
The users are supposed to adapt it to their needs and make it more robust.
Agreed, but from experience with many users ... it's a lot of users who just take a script and try it out without even looking at the script itself. So if the script could fail gracefully giving a hint like "you've not done as I told you to", some support issues will be avoided.
I've even experienced users where I've told them what to change in some code before running it on the phone, written it in the mail with the example code *and* having comments in the code - and still they failed to modify what they were required to change, calling back and complaining about it. Luckily enough, there is not too many of that kind - but they do exist. But there are others who actually do try code without using it, see the error - and then fixing it. So an error message would be appropriate if the code is not modified to a "proper" value - whatever "proper" would be.
Quoted 10 lines
Quoted 9 lines
ssl.c verify_callback (int preverify_ok, X509_STORE_CTX * ctx) · line 803
Quoted 5 lines
(quoted diff) +2
-0+ /* "prints" the serial number onto the BIO and read it back */ + if ( (i2a_ASN1_INTEGER(bio, X509_get_serialNumber(
ctx->current_cert)) >= 0) &&ssl.c +2
-0+ (( n = BIO_read (bio, serial, sizeof (serial)-1) ) >= 0) ) { + serial[n] =0 ;This should really not be needed when you've CLEAR()'ed serial earlier on. You should trust that
BIO_read()do not exceed it's limits or place other garbage after the value.
Quoted 5 lines
Is it really a good idea? You correctly speak of defensiveness later, so why give avay some safety just to avoid an assignment? What harm does it make to leave it in? Doesn't look like an expensive operation to me.
I'd say actually, for maximum safety, we should set to 0 all the bytes from the n'th all the way to the end of "serial".
Well, you have f.ex. 100 bytes, which are CLEAR()'ed. Then BIO_read() puts 50 bytes into this memory space, and returns 50. Then you set the end of this section to a value this memory already have via the first CLEAR() operation. It's completely redundant. It's almost like calculating 2+2 and then validate the answer by doing (1+1)+(1+1).
If BIO_read() returns 50 and writes 60 bytes, that's another issue - and to be frankly, that's a bug in OpenSSL. We shouldn't need to be overly paranoid and validate that OpenSSL does it's job correctly. We can't protect OpenSSL from doing stupidities, and such a check will really not give any improvements - as it might even hide issues which are in OpenSSL. I'm of the opinion it's better to get those obviously bugs (also in "third-party code") surfaced as quick as possible.
Or else you could pull this a lot further and insist on a proper use of printf(), like:
int len = strlen(str)
if( len != printf(str) ) {
exit(1); /* printf didn't print the complete string */
}Do we do that nowadays? It's for sure the most defensive way of programming, but it's really expected that printf() does it's job. But as I said, this is pulling it to the extremes.
So my point is that we should take care of the data and code which *we* are in charge of. If we use a function call from a "third-party code" which we don't control, we should trust that when the function returns a value it claims is a non-error situation, we process it as a non-error situation. If that turns out to give the wrong result, we can provide helpful information getting that external code fixed sooner.
Quoted 27 lines
Quoted 13 lines
Quoted 6 lines
ssl.c +3
-0+ } +else { + msg (M_WARN, "CALLBACK: Error reading/writing BIO (fortls_serial_%d)",
ctx->error_depth);
+ serial[0] = 0; /* empty
string */Hmmm ... My first thought was that this should not be needed. On second thought, I realised it really might be needed - as
BIO_read()*might* have put some data here before it failed. So if that can happen, some data which has been parsed badly into memory is still available via the 'serial' string.Quoted 7 lines
On the other hand, this might not be that critical, as
setenv_str()is pretty safe and will obey the first byte in the string. So a mem-info-leak is more difficult to manage, as this variable is only available inside this function.But consider to replace this "empty string" with yet another
CLEAR(), that's a more defensive approach.Agreed, will do that.
Quoted 4 lines
This part of the code should hopefulyl not be called much, unless something is really wrong - which means that we can afford to spend a little extra time clearing this region. Or another approach is to only use
setenv_str()using the 'serial' string only on success ... and otherwise setenv_str(opt-
Quoted 6 lines
Quoted 1 line
Quoted 1 line
es, envname, "\0") (might even be that NULL would work as well).
How would that differ from what I'm doing now? It will still end up being an empty string, but with the downside that we would have two separate calls to
setenv_str()to perform what is a single logical action (ie set the environment variable).
The difference is that you do reference data which *might* be corrupt, if serial contains parts of data - despite your '\0' on the first byte.
And then you do send this variable further to functions which purpose
is to only give an empty string.
So by setting an empty string explicit, you are very clear in your code that in this an error situation and that you don't want anything else than an empty string in this case. And you don't need to do CLEAR() on serial in this situation. That variable will in this case just be thrown away when the function returns.
Quoted 10 lines
Quoted 2 lines
I'm not convinced about any particular solution here now ... but I'm open for other thoughts about this as well.
I thought about another option: leaving the environment variable unset (ie, do NOT call
setenv_str()at all if there are errors) rather than set it to an empty string.
The problem with this is that in a shell script is a bit cumbersome to differentitate between an /empty/ variable and an /unset/ variable, and this is because the difference does not usually matter. It definitely doesn't in our case.
I thought about this as well, and thought that this *might* break other implementations which expects to find the a tls_serial_X variable. For shell scripts, you have the issues you mentioned - it's not really possible to identify empty vs not-set variable. But you also have plug-ins written in C, which would use a different approach for looking for environment variables.
In fact, taking the environment variable completely away in this case will break the current expected behaviour. I'm not so confident that we should do that. People who already are using C plug-ins might experience incompatibilities then.
Quoted 6 lines
Another option I thought of is to set the variable to something that could never be a valid serial number, eg "XXXXXXXXX" or something like that. Ugly as hell, if you ask me. And would /still/ require that the user of the variable check what's in it, so checking that it's not empty or checking that it's not "XXXXXXXXX" does not make much difference (and the former allows for more idiomatic code).
A false value would not necessarily solve it better. So I agree with you, that an empty string is cleaner and better. And for C plug-ins, it's easier and more convenient to validate an empty string vs a false value.
Quoted 20 lines
Bottom line: I'd still go with the empty variable solution.
Quoted 14 lines
One last comment is on the coding style. This patch breaks the coding style (yeah, I admit it - even I've been arrested on this one quite lately). Please correct that as well ... the current coding style (like it or not) is based on:
if (boolean expression) { do_something(); } else { do_something_else(); }Quoted 3 lines
To keep the code as clean and nicely as possible, we should try to follow the current coding style. If you don't like the current standard, lets discuss that first :)
Quoted 7 lines
I don't have any problem with whatever standard is in place, so I can surely reformat the code to the preferred style.
But if I have to say it all, I would say that it doesn't look like that style is applied consistently throughout the code (eg see
verify_cert_eku()inssl.c) although, now that you mention it, I must say that the vast majority of the code does use it, so it makes sense to stick to it.
I'll have a look into verify_cert_eku() and double check that. And see whom to blame for that ;-) But thanks for trying to stay with the current coding style :)
We should probably have our own "lindent" and "check-patch" scripts as the Linux kernel have, which could be used before sending patches. That could be helpful for everyone.
Quoted 6 lines
Quoted 3 lines
Thanks again, Davide ... I hope I'm not pushing you too hard, but this is a good improvement from the last patch, so this progress is making me happy! :)
No problem, thanks for the constructive criticism! If you agree, I will implement che changes you suggest in the C code and resend the patch.
Sounds good! :)
kind regards,
David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkvVZR4ACgkQDC186MBRfrq0/ACghBrf26h0w4QGh17OxPi3fDWc ypAAn24lmNlVlZ1eznzD6K6jt81rIO3y
=oYJO
-----END PGP SIGNATURE-----
Quoted 10 lines · On Monday 26 Apr 2010 11:04:16 David Sommerseth wrote:
Quoted 9 lines
Quoted 2 lines
... and have a check that this variable is set? If unset, exit with error.
Ah well, as I said the script is meant to be a barebone skeleton to demonstrate basic usage. That is by no means the only thing that lacks proper error checking in the script, so if we go down that route there's a lot to be changed.
The users are supposed to adapt it to their needs and make it more robust.
Quoted 4 lines
Agreed, but from experience with many users ... it's a lot of users who just take a script and try it out without even looking at the script itself. So if the script could fail gracefully giving a hint like "you've not done as I told you to", some support issues will be avoided.
Ok, that makes sense. I didn't look at it this way, but then I perfectly know what you mean, so I'll change it to exit if it detects that something is not set properly.
Quoted 11 lines
Quoted 5 lines
Is it really a good idea? You correctly speak of defensiveness later, so why give avay some safety just to avoid an assignment? What harm does it make to leave it in? Doesn't look like an expensive operation to me. I'd say actually, for maximum safety, we should set to 0 all the bytes from the n'th all the way to the end of "serial".
Well, you have f.ex. 100 bytes, which are CLEAR()'ed. Then
BIO_read()puts 50 bytes into this memory space, and returns 50. Then you set the end of this section to a value this memory already have via the firstCLEAR()operation. It's completely redundant. It's almost like calculating 2+2 and then validate the answer by doing (1+1)+(1+1).
I'd say that almost everything you do when programming defensively is writing apparently "redundant" code, which should never be executed, and all that because you don't trust the rest of the code (especially external calls).
Generally speaking, you can't be sure that i2a_ASN1_INTEGER() or BIO_read() do not mess with the part of the string that is beyond the actual data they write/read. though in this case we can look at their code, and see that they indeed don't.
Quoted 7 lines
If
BIO_read()returns 50 and writes 60 bytes, that's another issue - and to be frankly, that's a bug in OpenSSL. We shouldn't need to be overly paranoid and validate that OpenSSL does it's job correctly. We can't protect OpenSSL from doing stupidities, and such a check will really not give any improvements - as it might even hide issues which are in OpenSSL. I'm of the opinion it's better to get those obviously bugs (also in "third-party code") surfaced as quick as possible.
Ok, so the thing is "we trust OpenSSL". That's fine.
Quoted 12 lines
Quoted 11 lines
Quoted 6 lines
This part of the code should hopefulyl not be called much, unless something is really wrong - which means that we can afford to spend a little extra time clearing this region. Or another approach is to only use
setenv_str()using the 'serial' string only on success ... and otherwisesetenv_str(opt->es, envname, "\0")(might even be that NULL would work as well).How would that differ from what I'm doing now? It will still end up being an empty string, but with the downside that we would have two separate calls to
setenv_str()to perform what is a single logical action (ie set the environment variable).
Quoted 4 lines
The difference is that you do reference data which *might* be corrupt, if serial contains parts of data - despite your '\0' on the first byte.
And then you do send this variable further to functions which purpose
is to only give an empty string.
Not if we CLEAR() it as you suggested, which I'm going to implement.
Quoted 5 lines
So by setting an empty string explicit, you are very clear in your code that in this an error situation and that you don't want anything else than an empty string in this case. And you don't need to do
CLEAR()on serial in this situation. That variable will in this case just be thrown away when the function returns.
See above. I don't like having duplicated calls to setenv_str(). I prefer it to be called just once in a single place, with a sensible value (that is, the actual serial number, or the CLEAR()'ed string if there were errors).
Quoted 8 lines
Quoted 7 lines
I thought about another option: leaving the environment variable unset (ie, do NOT call
setenv_str()at all if there are errors) rather than set it to an empty string.
The problem with this is that in a shell script is a bit cumbersome to differentitate between an /empty/ variable and an /unset/ variable, and this is because the difference does not usually matter. It definitely doesn't in our case.
Quoted 4 lines
I thought about this as well, and thought that this *might* break other implementations which expects to find the a
tls_serial_Xvariable. For shell scripts, you have the issues you mentioned - it's not really possible to identify empty vs not-set variable.
It is indeed possible, but in this specific case it doesn't make any difference since both conditions must be treated the same way (ie, not valid).
Quoted 3 lines
So I agree with you, that an empty string is cleaner and better. And for C plug-ins, it's easier and more convenient to validate an empty string vs a false value.
Good! finally we agree on something :)
Quoted 3 lines
We should probably have our own "lindent" and "check-patch" scripts as the Linux kernel have, which could be used before sending patches. That could be helpful for everyone.
Agreed.
I have another (unrelated) question. The GIT master branch lacks the configure script, so how do you go about building it for testing? I vaguely seem to remember something about running autoscan plus other tricks, is that the case with OpenVPN or are the instructions different? Also, whatever the instructions are, I would suggest they be added to the official "developer information" page.
Thanks!
Signature · 1 line
D.
[Openvpn-devel] Building from git tree (was: Re: [PATCH] Serial number export, fixed)
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 1 line · Davide Brini
On Monday 26 Apr 2010 11:04:16 David Sommerseth wrote:
[...snip...]
Quoted 4 lines
I have another (unrelated) question. The GIT master branch lacks the configure script, so how do you go about building it for testing? I vaguely seem to remember something about running autoscan plus other tricks, is that the case with OpenVPN or are the instructions different?
First of all, I would probably recommend working against the bugfix2.1 branch in this case. You're actually fixing a bug. And then it's going to be easier to apply your patch to the main tree afterwards. I see I need to "soften" up the developers page a little bit on this point. The whole openvpn-testing.git tree is a moving target, and I'm adjusting the routines to make it as smooth and easy for as many as possible.
For the building part, you just need to run 'autoreconf -i' (add -v to be more verbose). That should be all.
It would also be a good idea to always run 'make distcheck' when you're feeling confident with the patch. You also have 'make check', but that only does a compilation against your "local" source code. Running distcheck will copy all the source files which is defined to be a part of the distribution to a new directory and compile the source from there, thus making sure you're not missing any files. That's especially important when adding new files to the repository. Most probably you'll need to update Makefile.am to get those files included.
Quoted 3 lines
Also, whatever the
instructions are, I would suggest they be added to the official "developer information" page.
It actually is documented, but under the "Documentation for testers" ... the old wiki got it here:
<http://www.secure-computing.net/wiki/index.php/OpenVPN/Documentation_for_testers>
I'll make sure that documentation is referenced somehow when the new community server gets fixed again. All this documentation should be available here primarily.
kind regards,
David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkvVd4MACgkQDC186MBRfrpVIQCgiMJD+hP2coratHwGPhVXXpxO jRMAnRE+UEmxGeqRAiOFaXkiBfPqotVI
=RU5T
-----END PGP SIGNATURE-----
Quoted 8 lines · Davide Brini
Quoted 2 lines · On Monday 26 Apr 2010 11:04:16 David Sommerseth wrote:
Agreed, but from experience with many users ... it's a lot of users
who
Quoted 1 line
just take a script and try it out without even looking at the
script
Quoted 2 lines
itself. So if the script could fail gracefully giving a hint like "you've not done as I told you to", some support issues will be
Quoted 7 lines
avoided.
Ok, that makes sense. I didn't look at it this way, but then I perfectly know
what you mean, so I'll change it to exit if it detects that something is not
set properly.
FWIW, and I don't know what the code does so I can't say whether this would be useful, but if you go to http://example.com
or http://www.example.com (but not http://foo.example.com) you get a bare-bones web page explaining that you're using a reserved domain.
Karl <[email protected]...>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
Quoted 8 lines · On Monday 26 Apr 2010 15:50:56 Karl O. Pinc wrote:
Quoted 7 lines
Quoted 3 lines
itself. So if the script could fail gracefully giving a hint like "you've not done as I told you to", some support issues will be avoided.
Ok, that makes sense. I didn't look at it this way, but then I perfectly know what you mean, so I'll change it to exit if it detects that something is not set properly.
Quoted 5 lines
FWIW, and I don't know what the code does so I can't say whether this would be useful, but if you go to http://example.com
or http://www.example.com (but not http://foo.example.com) you get a bare-bones web page explaining that you're using a reserved domain.
Right, I'll do something like this then:
# Edit the following values to suit your needs
# OCSP responder URL (mandatory)
# YOU MUST UNCOMMENT ONE OF THESE AND SET IT TO A VALID SERVER #ocsp_url="http://ocsp.example.com/" #ocsp_url="https://ocsp.secure.example.com/"
So the user will need to actively uncomment one of those, /and/ also set it to a valid URL. Leaving them commented makes the script fail because of sanity checks I added later. Uncommenting but leaving it set to those values will make the subsequent OCSP validation fail (obviously).
In both cases, OpenVPN's --tls-check (which is where the script is supposed to be called) will fail in turn, aborting the client connection. That should be enough of a heads-up for the lazy or distracted user.
(I still think it would be nice to have some sort of channel to send errors to OpenVPN's main log from the children scripts or programs, so users could inspect it.)
Thanks!
Signature · 1 line
D.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 30 lines · Davide Brini
Quoted 8 lines · On Monday 26 Apr 2010 15:50:56 Karl O. Pinc wrote:
Quoted 7 lines
Quoted 3 lines
itself. So if the script could fail gracefully giving a hint like "you've not done as I told you to", some support issues will be avoided.
Ok, that makes sense. I didn't look at it this way, but then I perfectly know what you mean, so I'll change it to exit if it detects that something is not set properly.
Quoted 5 lines
FWIW, and I don't know what the code does so I can't say whether this would be useful, but if you go to http://example.com
or http://www.example.com (but not http://foo.example.com) you get a bare-bones web page explaining that you're using a reserved domain.Right, I'll do something like this then:
# Edit the following values to suit your needs
# OCSP responder URL (mandatory)
# YOU MUST UNCOMMENT ONE OF THESE AND SET IT TO A VALID SERVER #ocsp_url="http://ocsp.example.com/" #ocsp_url="https://ocsp.secure.example.com/"So the user will need to actively uncomment one of those, /and/ also set it to a valid URL. Leaving them commented makes the script fail because of sanity checks I added later. Uncommenting but leaving it set to those values will make the subsequent OCSP validation fail (obviously).
Quoted 8 lines
In both cases, OpenVPN's --tls-check (which is where the script is supposed to be called) will fail in turn, aborting the client connection. That should be enough of a heads-up for the lazy or distracted user.
(I still think it would be nice to have some sort of channel to send errors to OpenVPN's main log from the children scripts or programs, so users could inspect it.)
This begins to look very good!
Just for the errors ... they are not captured if you write to stderr or stdout from the script and put into the openvpn log files? I haven't tried it, but you might know this better from experience.
kind regards,
David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkvVrvYACgkQDC186MBRfrrKqgCfW0tPgr2Aw/DgvQEJUFZLQegG 0v4AnRmr/++u7YC4403ktq7oZqMIWbe5
=Sxey
-----END PGP SIGNATURE-----
Quoted 9 lines · On Monday 26 Apr 2010 16:19:20 David Sommerseth wrote:
Quoted 3 lines
(I still think it would be nice to have some sort of channel to send errors to OpenVPN's main log from the children scripts or programs, so users could inspect it.)
This begins to look very good!
Just for the errors ... they are not captured if you write to stderr or stdout from the script and put into the openvpn log files? I haven't tried it, but you might know this better from experience.
They are captured (which is of course better than nothing), but they're not timestamped etc.
To give you an idea, you get something like this (lines spaced for clarity):
---------------------------------------------------------------------
Mon Apr 26 16:27:44 2010 TLS: Initial packet from 10.0.0.1:1194, sid=473982ac 00c08d68
Error: OCSP server URL not defined!
Did you forget to customize the variables in the script?
Mon Apr 26 16:27:44 2010 VERIFY SCRIPT ERROR: depth=0, /C=GB/O=Test client/
...
---------------------------------------------------------------------
Note the "bare" messages output by the script to stderr (if you output to stdout, they look the same in OpenVPN's log).
One would probably expect something like this instead:
---------------------------------------------------------------------
Mon Apr 26 16:27:44 2010 TLS: Initial packet from 10.0.0.1:1194, sid=473982ac 00c08d68
Mon Apr 26 16:27:44 2010 OCSP_check.sh: Error: OCSP server URL not defined!
Mon Apr 26 16:27:44 2010 OCSP_check.sh: Did you forget to customize the variables in the script?
Mon Apr 26 16:27:44 2010 VERIFY SCRIPT ERROR: depth=0, /C=GB/O=Test client/
...
---------------------------------------------------------------------
In this specific instance, it may probably be hacked somehow using logger(1) with the right parameters in the script (or maybe not - I haven't tried), but it would be nice if there was a general framework offered by the parent OpenVPN so logs from the children would effectively be part of the parent process's message flow, and treated as such (so if OpenVPN is using special or customized log settings, the messages from the children would automatically be processed in the same way).
Signature · 1 line
D.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 48 lines · Davide Brini
Quoted 9 lines · On Monday 26 Apr 2010 16:19:20 David Sommerseth wrote:
Quoted 3 lines
(I still think it would be nice to have some sort of channel to send errors to OpenVPN's main log from the children scripts or programs, so users could inspect it.)
This begins to look very good!
Just for the errors ... they are not captured if you write to stderr or stdout from the script and put into the openvpn log files? I haven't tried it, but you might know this better from experience.
They are captured (which is of course better than nothing), but they're not timestamped etc.
To give you an idea, you get something like this (lines spaced for clarity):---------------------------------------------------------------------
Mon Apr 26 16:27:44 2010 TLS: Initial packet from 10.0.0.1:1194, sid=473982ac00c08d68Error: OCSP server URL not defined!
Did you forget to customize the variables in the script?Mon Apr 26 16:27:44 2010 VERIFY SCRIPT ERROR: depth=0, /C=GB/O=Test client/
...
---------------------------------------------------------------------Note the "bare" messages output by the script to stderr (if you output to stdout, they look the same in OpenVPN's log).
One would probably expect something like this instead:
---------------------------------------------------------------------
Mon Apr 26 16:27:44 2010 TLS: Initial packet from 10.0.0.1:1194, sid=473982ac00c08d68Mon Apr 26 16:27:44 2010
OCSP_check.sh: Error: OCSP server URL not defined!Mon Apr 26 16:27:44 2010
OCSP_check.sh: Did you forget to customize the variables in the script?Mon Apr 26 16:27:44 2010 VERIFY SCRIPT ERROR: depth=0, /C=GB/O=Test client/
...
---------------------------------------------------------------------
Quoted 8 lines
In this specific instance, it may probably be hacked somehow using logger(1) with the right parameters in the script (or maybe not - I haven't tried), but it would be nice if there was a general framework offered by the parent OpenVPN so logs from the children would effectively be part of the parent process's message flow, and treated as such (so if OpenVPN is using special or customized log settings, the messages from the children would automatically be processed in the same way).
Thanks for the update! Yes, I agree, this is something we should try to improve somehow. I'll make sure that goes into a "todo list" when the community server is up-n-running again.
kind regards,
David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkvVu6AACgkQDC186MBRfrr++QCgko0Y+sTEb+86R/nyFOpsiawu f+MAoKQRGi7TP8p+B95cydMiO8OaZC9l
=oVom
-----END PGP SIGNATURE-----
PATCH 1/1 Serial number export, better
In reply to David Sommerseth contrib/OCSP_check/OCSP_check.sh:
New barebone script to demonstrate how to use $tls_serial_{n} to perform simple OCSP queries using OpenSSL command line "openssl ocsp". Minimal sanity checks to fail if user tries to use it without customizing.
openvpn.8:
Added some notes about $tls_serial_{n} format and usage to the existing description.
ssl.c:
correctly manage and export serial numbers of any size (as parsed by OpenSSL) into the environment. Set to empty string in case of errors, as 0 and negative numbers are all possible (although illegal) certificate serial numbers. Use an OpenSSL BIO object to do the job. Conforms to coding style guidelines.
See the discussion at
http://article.gmane.org/gmane.network.openvpn.devel/3588
for more details.
--- contrib/OCSP_check/OCSP_check.sh | 89 ++++++++++++++++++++++++++++++++++++++ openvpn.8 | 7 +++- ssl.c | 27 ++++++++++- 3 files changed, 119 insertions(+), 4 deletions(-) create mode 100644 contrib/OCSP_check/OCSP_check.sh
contrib/OCSP_check/OCSP_check.sh new file +89 -0
@@ -0,0 +1,89 @@ +#!/bin/sh + +# Sample script to perform OCSP queries with OpenSSL +# given a certificate serial number. + +# If you run your own CA, you can set up a very simple +# OCSP server using the -port option to "openssl ocsp". + +# Full documentation and examples: +# http://www.openssl.org/docs/apps/ocsp.html + + +# Edit the following values to suit your needs + +# OCSP responder URL (mandatory) +# YOU MUST UNCOMMENT ONE OF THESE AND SET IT TO A VALID SERVER +#ocsp_url="http://ocsp.example.com/" +#ocsp_url="https://ocsp.secure.example.com/" + +# Path to issuer certificate (mandatory) +# YOU MUST SET THIS TO THE PATH TO THE CA CERTIFICATE +issuer="/path/to/CAcert.crt" + +# use a nonce in the query, set to "-no_nonce" to not use it +nonce="-nonce" + +# Verify the response +# YOU MUST SET THIS TO THE PATH TO THE RESPONSE VERIFICATION CERT +verify="/path/to/CAcert.crt" + +# Depth in the certificate chain where the cert to verify is. +# Set to -1 to run the verification at every level (NOTE that +# in that case you need a more complex script as the various +# parameters for the query will likely be different at each level) +# "0" is the usual value here, where the client certificate is +check_depth=0 + +cur_depth=$1 # this is the *CURRENT* depth +common_name=$2 # CN in case you need it + +# minimal sanity checks + +err=0 +if [ -z "$issuer" ] || [ ! -e "$issuer" ]; then + echo "Error: issuer certificate undefined or not found!" >&2 + err=1 +fi + +if [ -z "$verify" ] || [ ! -e "$verify" ]; then + echo "Error: verification certificate undefined or not found!" >&2 + err=1 +fi + +if [ -z "$ocsp_url" ]; then + echo "Error: OCSP server URL not defined!" >&2 + err=1 +fi + +if [ $err -eq 1 ]; then + echo "Did you forget to customize the variables in the script?" >&2 + exit 1 +fi + +# begin +if [ $check_depth -eq -1 ] || [ $cur_depth -eq $check_depth ]; then + eval serial="\$tls_serial_${cur_depth}" + + # Check that the serial is not empty + if [ -n "$serial" ]; then + + # This is only an example; you are encouraged to run this command (without + # redirections) manually against your or your CA's OCSP server to see how + # it responds, and adapt accordingly. + # Sample output: + # + # Response verify OK + # 0x428740A5: good + # This Update: Apr 24 19:38:49 2010 GMT + # Next Update: May 2 14:23:42 2010 GMT + + openssl ocsp -issuer "$issuer" \ + "$nonce" \ + -CAfile "$verify" \ + -url "$ocsp_url" \ + -serial "0x${serial}" >/dev/null 2>&1 + else + exit 1 + fi +fi
openvpn.8 +6 -1
@@ -5321,7 +5321,12 @@ where is the verification level. Only set for TLS connections. Set prior to execution of .B --tls-verify -script. +script. This is in the form of a hex string like "37AB46E0", which is +suitable for doing serial-based OCSP queries (with OpenSSL, you have +to prepend "0x" to the string). If something goes wrong while reading +the value from the certificate it will be an empty string, so your +code should check that. +See the contrib/OCSP_check/OCSP_check.sh script for an example. .\"********************************************************* .TP .B tun_mtu
ssl.c +24 -3
@@ -788,9 +788,30 @@ verify_callback (int preverify_ok, X509_STORE_CTX * ctx)/* export serial number as environmental variable */ { -const int serial = (int )ASN1_INTEGER_get (X509_get_serialNumber (ctx->current_cert)); -openvpn_snprintf (envname,sizeof (envname),"tls_serial_%d" , ctx->error_depth); -setenv_int (opt->es, envname, serial); +BIO *bio =NULL ; +char serial[100 ]; +int n1, n2; + +CLEAR (serial); +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 and read it back */ +if ( ! ( ( (n1 =i2a_ASN1_INTEGER (bio,X509_get_serialNumber (ctx->current_cert))) >=0 ) && + ( (n2 =BIO_read (bio, serial,sizeof (serial)-1 )) >=0 ) && + ( n1 == n2 ) ) ) + { +msg (M_WARN ,"CALLBACK: Error reading/writing BIO (for tls_serial_%d)" , ctx->error_depth); +CLEAR (serial);/* empty string */ + } + +openvpn_snprintf (envname,sizeof (envname),"tls_serial_%d" , ctx->error_depth); +setenv_str (opt->es, envname, serial); +BIO_free (bio); + } }/* export current untrusted IP */
Signature · 1 line
1.6.5.6
Re: [Openvpn-devel] [PATCH] Serial number export, better
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 23 lines · Davide Brini
contrib/OCSP_check/OCSP_check.sh:
New barebone script to demonstrate how to use $tls_serial_{n} to perform simple OCSP queries using OpenSSL command line "openssl ocsp". Minimal sanity checks to fail if user tries to use it without customizing.openvpn.8:
Added some notes about $tls_serial_{n} format and usage to the existing description.
ssl.c:
correctly manage and export serial numbers of any size (as parsed by OpenSSL) into the environment. Set to empty string in case of errors, as 0 and negative numbers are all possible (although illegal) certificate serial numbers. Use an OpenSSL BIO object to do the job. Conforms to coding style guidelines.See the discussion at
http://article.gmane.org/gmane.network.openvpn.devel/3588
for more details.
Quoted 8 lines
Signed-off-by Davide Brini <[email protected]...>--- contrib/OCSP_check/OCSP_check.sh | 89 ++++++++++++++++++++++++++++++++++++++ openvpn.8 | 7 +++- ssl.c | 27 ++++++++++- 3 files changed, 119 insertions(+), 4 deletions(-) create mode 100644 contrib/OCSP_check/OCSP_check.sh
ACK! This is looking good! I've put it into my work queue and will try to get time sometime this week to get it into the bugfix2.1 branch.
Thanks a lot for your hard work on this one!
kind regards,
David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkvW/KwACgkQDC186MBRfrqUNACfRKjQww+GT1Pf3whbN5a8xr04 2hEAn2p0z1jg9nWYfg7oadIEFWkk5tgD
=juOA
-----END PGP SIGNATURE-----
Re: [Openvpn-devel] [PATCH] Serial number export, better
In reply to Davide Brini-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Quoted 23 lines · Davide Brini
contrib/OCSP_check/OCSP_check.sh:
New barebone script to demonstrate how to use $tls_serial_{n} to perform simple OCSP queries using OpenSSL command line "openssl ocsp". Minimal sanity checks to fail if user tries to use it without customizing.openvpn.8:
Added some notes about $tls_serial_{n} format and usage to the existing description.
ssl.c:
correctly manage and export serial numbers of any size (as parsed by OpenSSL) into the environment. Set to empty string in case of errors, as 0 and negative numbers are all possible (although illegal) certificate serial numbers. Use an OpenSSL BIO object to do the job. Conforms to coding style guidelines.See the discussion at
http://article.gmane.org/gmane.network.openvpn.devel/3588
for more details.
Quoted 8 lines
Signed-off-by Davide Brini <[email protected]...>--- contrib/OCSP_check/OCSP_check.sh | 89 ++++++++++++++++++++++++++++++++++++++ openvpn.8 | 7 +++- ssl.c | 27 ++++++++++- 3 files changed, 119 insertions(+), 4 deletions(-) create mode 100644 contrib/OCSP_check/OCSP_check.sh
This patch is now applied to the bugfix2.1 branch and merged into allmerged. An updated tree is now available.
Commit fa47f0a36c2aeda972a94c93f8f83246306812a0
kind regards,
David Sommerseth
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iEYEARECAAYFAkvXXhEACgkQDC186MBRfrqp6QCfSroMqJRTJ3xdZIHab2YMBxhE TyoAoIgoym231GFcjDNhk9+Z5+WdMltx
=OtJi
-----END PGP SIGNATURE-----
Quoted 8 lines · Davide Brini
Quoted 5 lines · On Monday 26 Apr 2010 00:13:39 David Sommerseth wrote:
contrib/OCSP_check.sh line 18
Quoted 3 lines
contrib/OCSP_check.sh new file +3
-0+# OCSP responder URL (mandatory) +ocsp_url="http://some.ocsp.server/" +#ocsp_url="https://some.secure.ocsp.server/"Wouldn't it be better to use a more valid URL?
Quoted 6 lines
And what could it be? I can put in the OCSP URL of some well-known CA,
but
then if the user is using their own server, that would be meaningless anyway.
But I'm open to suggestions.
I would suggest example.com, which would be compliant with RFC 2606 "Reserved Top Level DNS Names".
Karl <[email protected]...>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
Quoted 2 lines · Davide Brini
The users are supposed to adapt it to their needs and make it more robust.
Hah! Users will not.
//Peter