[PATCH] mail_{ssl, auth_http}_module: add support for SSL client certificates

[Sven Peter]

Hi,

On Jan 13, 2014, at 1:09 PM, Filipe Da Silva <fdasilvayy at gmail.com> wrote:

> Hi.
> 
> Some remarks about your patch .
> 
> 2014/1/13  <nginx-devel-request at nginx.org>:
>> 
>> 
>> diff -r 4aa64f695031 -r 8744640301ae src/mail/ngx_mail_auth_http_module.c
>> --- a/src/mail/ngx_mail_auth_http_module.c      Sat Jan 04 03:32:22 2014 +0400
>> +++ b/src/mail/ngx_mail_auth_http_module.c      Mon Jan 13 11:02:55 2014 +0100
>> @@ -1144,6 +1144,11 @@
>>     ngx_buf_t                 *b;
>>     ngx_str_t                  login, passwd;
>>     ngx_mail_core_srv_conf_t  *cscf;
>> +    ngx_str_t ssl_client_verify = {0, NULL};
>> +    ngx_str_t ssl_client_raw_s_dn = {0, NULL};
>> +    ngx_str_t ssl_client_raw_i_dn = {0, NULL};
>> +    ngx_str_t ssl_client_s_dn = {0, NULL};
>> +    ngx_str_t ssl_client_i_dn = {0, NULL};
> 
> This kind of initialization is not part in the nginx coding style.

Ah, sorry. I'll fix that!
How do I handle the case when nginx is configured without ssl support (i.e. NGX_MAIL_SSL is not defined).?
Just place a #ifdef around the declarations and the other new code below?

> 
>> 
>>     if (ngx_mail_auth_http_escape(pool, &s->login, &login) != NGX_OK) {
>>         return NULL;
>> @@ -1153,6 +1158,29 @@
>>         return NULL;
>>     }
>> 
>> +    // ssl_client_verify doesn't need to be escaped since it comes from nginx itself
>> +#if (NGX_MAIL_SSL)
>> +    ngx_ssl_get_client_verify(s->connection, pool, &ssl_client_verify);
>> +    ngx_ssl_get_subject_dn(s->connection, pool, &ssl_client_s_dn);
>> +    ngx_ssl_get_subject_dn(s->connection, pool, &ssl_client_i_dn);
> 
> Twice call to ngx_ssl_get_subject_dn : Copy-paste issue ?
> 

Yes, it's a copy-paste issue. I didn't notice because I only verify the subject in my setup.
The second call should be ngx_ssl_get_issuer_dn of course.


> ...
> 
> Regards,
> FDS
> 
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel

How do I proceed from here? Re-submit the fixed patch as a reply to this thread?


Thanks,


Sven