[mod_gnutls-devel] `GnuTLSClientVerify` behavior incorrect?

[Ramkumar Chinchani]

FYI.

This has been fixed in gnutls upstream

commit 4aebdbe7d424f2a1705f3751c5fef6f2e5f4e616
Author: Nikos Mavrogiannopoulos <nmav at gnutls.org>
Date:   Thu Aug 29 14:06:20 2013 +0300

    Corrected gnutls_certificate_server_set_request().

-  unsigned send_cert_req:1;
+  unsigned send_cert_req;





On Tue, Mar 4, 2014 at 8:03 AM, Ramkumar Chinchani <
ramkumar.chinchani at gmail.com> wrote:

> Next version of the patch. This is now tested and works as advertised.
>
> The main change is in the gnutls_certificate_server_set_request() call.
>
> void
> gnutls_certificate_server_set_request (gnutls_session_t session,
>                                        gnutls_certificate_request_t req)
> {
>   session->internals.send_cert_req = req;
> }
>
> But,
>
> unsigned send_cert_req:1;
>
> So this is really boolean.
>
>
>
> diff --git a/src/gnutls_hooks.c b/src/gnutls_hooks.c
> index d068ebb..ebf0a27 100644
> --- a/src/gnutls_hooks.c
> +++ b/src/gnutls_hooks.c
> @@ -142,7 +142,7 @@ static int
> mgs_select_virtual_server_cb(gnutls_session_t session) {
>          ctxt->sc = tsc;
>         }
>
> -    gnutls_certificate_server_set_request(session,
> ctxt->sc->client_verify_mode);
> +    gnutls_certificate_server_set_request(session,
> !!ctxt->sc->client_verify_mode);
>
>      /* Set Anon credentials */
>      gnutls_credentials_set(session, GNUTLS_CRD_CERTIFICATE,
> ctxt->sc->certs);
>
> @@ -849,7 +849,8 @@ int mgs_hook_authz(request_rec * r) {
>          ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
>                  "GnuTLS: Directory set to Ignore Client Certificate!");
>      } else {
> -        if (ctxt->sc->client_verify_mode < dc->client_verify_mode) {
> +        if (dc->client_verify_mode >= 0 &&
> +                ctxt->sc->client_verify_mode < dc->client_verify_mode) {
>              ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
>                      "GnuTLS: Attempting to rehandshake with peer. %d %d",
>                      ctxt->sc->client_verify_mode,
> @@ -862,7 +863,7 @@ int mgs_hook_authz(request_rec * r) {
>                  return rv;
>
>              gnutls_certificate_server_set_request
> -                    (ctxt->session, dc->client_verify_mode);
> +                    (ctxt->session, !!dc->client_verify_mode);
>
>              if (mgs_rehandshake(ctxt) != 0) {
>                  return HTTP_FORBIDDEN;
>
> @@ -878,7 +879,8 @@ int mgs_hook_authz(request_rec * r) {
>          rv = mgs_cert_verify(r, ctxt);
>          if (rv != DECLINED &&
>                  (rv != HTTP_FORBIDDEN ||
> -                dc->client_verify_mode == GNUTLS_CERT_REQUIRE)) {
> +                dc->client_verify_mode == GNUTLS_CERT_REQUIRE ||
> +                ctxt->sc->client_verify_mode == GNUTLS_CERT_REQUIRE)) {
>              return rv;
>          }
>      }
>
>
>
> On Mon, Mar 3, 2014 at 6:03 AM, Ramkumar Chinchani <
> ramkumar.chinchani at gmail.com> wrote:
>
>> This is the complete patch.
>> Basically two changes,
>> 1) dc->client_verify_mode = -1 unless there is a explicit directory
>> directive
>> 2) msgs_cert_verify() does correctly come with HTTP_FORBIDDEN but if
>> client cert is "required" it must be honored
>>
>>
>> diff --git a/src/gnutls_hooks.c b/src/gnutls_hooks.c
>> index d068ebb..2cc8af1 100644
>>
>> --- a/src/gnutls_hooks.c
>> +++ b/src/gnutls_hooks.c
>> @@ -849,7 +849,8 @@ int mgs_hook_authz(request_rec * r) {
>>          ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
>>                  "GnuTLS: Directory set to Ignore Client Certificate!");
>>      } else {
>> -        if (ctxt->sc->client_verify_mode < dc->client_verify_mode) {
>> +        if (dc->client_verify_mode >= 0 &&
>> +                ctxt->sc->client_verify_mode < dc->client_verify_mode) {
>>              ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
>>                      "GnuTLS: Attempting to rehandshake with peer. %d %d",
>>                      ctxt->sc->client_verify_mode,
>> @@ -878,7 +879,8 @@ int mgs_hook_authz(request_rec * r) {
>>          rv = mgs_cert_verify(r, ctxt);
>>          if (rv != DECLINED &&
>>                  (rv != HTTP_FORBIDDEN ||
>> -                dc->client_verify_mode == GNUTLS_CERT_REQUIRE)) {
>> +                dc->client_verify_mode == GNUTLS_CERT_REQUIRE ||
>> +                ctxt->sc->client_verify_mode == GNUTLS_CERT_REQUIRE)) {
>>              return rv;
>>          }
>>      }
>>
>>
>>
>> On Tue, Feb 25, 2014 at 9:48 PM, Ramkumar Chinchani <
>> ramkumar.chinchani at gmail.com> wrote:
>>
>>> As per current config model, it is possible that  `GnuTLSClientVerify`
>>> is defined in server or vhost but not under directory, in which case this
>>> is probably what is desired?
>>>
>>> Kindly review.
>>>
>>>
>>> diff --git a/src/gnutls_hooks.c b/src/gnutls_hooks.c
>>> index d068ebb..335ae3f 100644
>>> --- a/src/gnutls_hooks.c
>>> +++ b/src/gnutls_hooks.c
>>> @@ -849,7 +849,8 @@ int mgs_hook_authz(request_rec * r) {
>>>          ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
>>>                  "GnuTLS: Directory set to Ignore Client Certificate!");
>>>      } else {
>>> -        if (ctxt->sc->client_verify_mode < dc->client_verify_mode) {
>>> +        if (dc->client_verify_mode >= 0 &&
>>> +             ctxt->sc->client_verify_mode < dc->client_verify_mode) {
>>>              ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
>>>                      "GnuTLS: Attempting to rehandshake with peer. %d
>>> %d",
>>>                      ctxt->sc->client_verify_mode,
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/attachments/20140304/d7a0ee8f/attachment-0001.html>