[PATCH] Proxy remote server SSL certificate verification

Aviram Cohen aviram at adallom.com
Thu Aug 22 14:00:55 UTC 2013


Hello!

I have a couple of questions regarding the two last comments:

On Wed, Aug 21, 2013 at 5:30 PM, Maxim Dounin <mdounin at mdounin.ru> wrote:
> Hello!
>
[..]
>> @@ -3748,6 +3786,13 @@ ngx_http_proxy_set_ssl(ngx_conf_t *cf, n
>>          != NGX_OK)
>>      {
>>          return NGX_ERROR;
>> +    }
>> +
>> +    if (ngx_ssl_set_verify_options(plcf->upstream.ssl,
>> +          &plcf->upstream.ssl_certificate, plcf->upstream.ssl_verify_depth)
>> +        != NGX_OK)
>> +    {
>> +        return NGX_ERROR;
>>      }
>
> This is called before options used are correctly set.

Where do you think this call should be performed? Should we add a
postconfiguration callback for
 the proxy module from which this would be called?
(BTW, I'll remove ngx_ssl_set_verify_options() and use
ngx_ssl_trusted_certificate() directly instead, as it is better)

>
> (There is also a style problem here, but it doesn't really matter
> as you'll have to rewrite the code anyway.)
>
> [...]
>
>> @@ -1334,6 +1341,11 @@ ngx_http_upstream_ssl_handshake(ngx_conn
>>
>>          ngx_http_upstream_send_request(r, u);
>>
>> +        c = r->connection;
>> +
>> +fail:
>> +        ngx_http_run_posted_requests(c);
>> +
>>          return;
>>      }
>>
>
> You probably missed my previous comment.  You have a use after
> free problem here.  Try triggering an error in
> ngx_http_upstream_send_request() with NGX_DEBUG_MALLOC defined, it
> should segfault.

You're right, I've missed it... Should we check before
ngx_http_send_request() is called whether or not the
request has a parent request, and accordingly decide later whether to
call ngx_http_run_posted_requests() or not?

>
> [...]
>
> --
> Maxim Dounin
> http://nginx.org/en/donation.html
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel

Best regrads,
Aviram



More information about the nginx-devel mailing list