[PATCH] SSLCryptoDevCtl support

[Serge Hallyn <se...@us.ibm.com>]

Hi,

This patch obsoletes that which I sent on Feb 17.  It is simply a port
of Geoff Thorpe's patch, submitted on July 12, 2001, to apache 2.0.48. 
According to list archives, there was never a response to this patch.

Using this patch,

  1. users of openssl versions prior to 0.9.8 (which has not yet been
released) can utilize dynamic engines.  This means that vendors can
provide dynamic crypto engines to customers without requiring
recompilation of openssl and apache.  For instance, our sample ssl.conf
contained the lines:

SSLCryptoDevice dynamic
SSLCryptoDeviceCtrl SO_PATH:/usr/local/lib/hw_ibmca.so
SSLCryptoDeviceCtrl ID:ibmca
SSLCryptoDeviceCtrl LIST_ADD:1
SSLCryptoDeviceCtrl LOAD

  2. These directives can be used to send engine-specific commands to an
openssl engine.  This remains useful after openssl 0.9.8 becomes
standard.

Comments are much appreciated.

thanks,
-serge
-- 
=======================================================
Serge Hallyn
Security Software Engineer, IBM Linux Technology Center
serue@us.ibm.com

Re: [PATCH] SSLCryptoDevCtl support

[Geoff Thorpe <ge...@geoffthorpe.net>]

Hi Serge,

Thanks for porting this. In my defense, I knew that firsttime hack stuff 
would never wash with Ralf (for mod_ssl) or the ASF (for apache2), but I 
simply wanted to quickly wash my hands of that whole weirdness of double 
config processing (IMHO brokenness may be tolerated, but must never be 
embraced :-).

Cheers,
Geoff

On February 23, 2004 03:02 pm, Serge Hallyn wrote:
> thanks for your reply.  Attached is a new patch against HEAD taking
> your comments into consideration.
>
> Note that the static int firsttime hack can go away as soon as the
> repeated config reads in main() are consolidated.

-- 
Geoff Thorpe
geoff@geoffthorpe.net
http://www.geoffthorpe.net/

Re: [PATCH] SSLCryptoDevCtl support

[Serge Hallyn <se...@us.ibm.com>]

Hi,

thanks for your reply.  Attached is a new patch against HEAD taking your
comments into consideration.

Note that the static int firsttime hack can go away as soon as the
repeated config reads in main() are consolidated.

thanks,
-serge

On Thu, 2004-02-19 at 10:04, Joe Orton wrote:
...
> I think that stuff be avoided by using the:
> 
>     if (ssl_config_global_isfixed(mc)) {
>         return NULL;
>     }
> 
> trick like the other ssl_cmd_* functions?  There are code formatting
> issues through the rest of the patch, otherwise looks OK - a patch
> against HEAD would be ideal of course...
> 
> > +const char *ssl_cmd_SSLCryptoDeviceCtrl(cmd_parms *cmd,
> > +					void *dcfg,
> > +					const char *arg,
> > +					const char *prepost)
> > +{
> ...
> > +    colon = strchr(arg, ':');
> 
> should be ap_strchr_c() to avoid losing const-ness.
> 
> > +static int internal_ctrl_cb(void *r, const char *k, const char *v)
> 
> could do with some meaningful parameter names :)
> 
> Regards,
> 
> joe
-- 
=======================================================
Serge Hallyn
Security Software Engineer, IBM Linux Technology Center
serue@us.ibm.com

Re: [PATCH] SSLCryptoDevCtl support

[Joe Orton <jo...@redhat.com>]

On Thu, Feb 19, 2004 at 09:41:55AM -0600, Serge Hallyn wrote:
...
> +
> +/* Our static variables get reset between first and second pass ... so this
> + * variable is mixed in key-value strings and incremented. On the second pass,
> + * the key-values will match so will replace, rather than supplement, the table
> + * entries. (All this to prevent getting two copies of everything?!) */
> +static int braindead = 0;

I think that stuff be avoided by using the:

    if (ssl_config_global_isfixed(mc)) {
        return NULL;
    }

trick like the other ssl_cmd_* functions?  There are code formatting
issues through the rest of the patch, otherwise looks OK - a patch
against HEAD would be ideal of course...

> +const char *ssl_cmd_SSLCryptoDeviceCtrl(cmd_parms *cmd,
> +					void *dcfg,
> +					const char *arg,
> +					const char *prepost)
> +{
...
> +    colon = strchr(arg, ':');

should be ap_strchr_c() to avoid losing const-ness.

> +static int internal_ctrl_cb(void *r, const char *k, const char *v)

could do with some meaningful parameter names :)

Regards,

joe