Hi,

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

> Hi.
>
> Some remarks about your patch .
>
> 2014/1/13 <nginx-devel-request@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@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

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel