-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 28/04/10 07:17, Brian Raderman wrote:
> Hello,
> 
> This patch adds support for using certificates stored in the Mac OSX Keychain
> to authenticate with the OpenVPN server.  This works with certificates
stored
> on the computer as well as certificates on hardware tokens that
support Apple's
> tokend interface.  It has been tested with an Aladdin eToken on
> Mac OSX Leopard and with software only certificates on
> Mac OSX Leopard and Snow Leopard.  Aladdin has not yet released
> drivers for Snow Leopard.  The patch is very similar to, and also
> based on, the Windows Crypto API certificate functionality that
> currently exists in OpenVPN.

> I did not know how to adjust the automake/autoconf configuration
> files, so I have included the XCode project I used to build and test
> openvpn in this patch.  All new files were added in an "osx" subdirectory.
> 
> Please let me know if you have any questions or if there are any changes
> (I suspect there will be many) that I need to complete before this patch
> can be accepted.
> 

Hi Brian,

Thanks a lot for your patch!  This is indeed a neat feature we want to
really consider for inclusion!  But there are of course a few things I
spotted which I'd like to see being improved.

I am not familiar with OSX details at all, but I hope someone else with
better OSX understanding can review those parts more thoroughly.  My
comments will be of the more generic type.

* usage of printf()
Please use the msg() function instead.  Don't print things to console
directly via {f,}printf() directly.  There are plenty of places in the
code where you can see msg() being used for a lot of logging.

* malloc()
You use malloc() several places.  If possible try to make use of the
gc_arena API instead.  This API is also used a lot of places in the
code, so you'll most likely find some good examples there as well.

* static void dummy1() function
In osx/keychain.c you define a rather silly function, which is not used
anywhere.  Please take this one out.

* xcode files and migration to autotools
Even though Xcode is probably a very good tool on OSX, it's not what we
can base OpenVPN on.  I would like to see this migrated over to
autotools.  It's not as hard and difficult as you might think
immediately, but there are several traps on the way.  Some traps more
visible than others.

First of all, it's a pretty good autotools tutorial [1] which is worth
going through to understand the concept and how to get started.

Then you'll need to modify configure.ac and Makefile.am.  And that's
basically it.  Have a look for SOCKS in configure.ac to get an idea.
Further, you'll also need to add the source files of our osx/ C and H
files to openvpn_SOURCES.  But! These files should probably be depending
on if this OSX feature is enabled or not.  There's probably several ways
how to solve this, and I'm not sure what's the best solution right now.
 But digging into automake docs[2] might be the proper place to start.

When you think you're done, ./configure works and it seems to compile
flawlessly by running 'make', try runnning 'make distcheck' to be sure
it will package itself properly, that the test compilation and test run
works as expected as well.


That's basically all I had to comment so far.  But thanks again for your
patch!  Looking forward to hear from you again with further updates as well.


kind regards,

David Sommerseth


[1] <http://www.lrde.epita.fr/~adl/autotools.html>
[2] <http://sources.redhat.com/automake/automake.html>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iEYEARECAAYFAkvX23gACgkQDC186MBRfroWXgCfeZUA1pFrSsu55MMk3gg9TIcJ
M0EAoKO/OYi2v5YMNDankep5vqMEX7hC
=sXAB
-----END PGP SIGNATURE-----

Reply via email to