[mod_gnutls-devel] mgs_hook_authz() handling of return values from mgs_cert_verify()?

[Ramkumar Chinchani]

I cannot get the test code to run - various versions of gnutls-cli keep
barfing at me.

Noticed that there is already test code that tests for client verification.

However IINM "t/tests/11_basic_client_verification_fail" only tests the
case where the client doesn't send a certificate at all and not the case
where the certificate sent cannot be verified by the server.

Here is a first draft of this test case which should fail before the patch
in [1] is applied since we expect the test itself to fail. Once the patch
is applied, client should correctly get a HTTP_FORBIDDEN.

[1] http://lists.gnupg.org/pipermail/mod_gnutls-devel/2014-March/000054.html



diff --git a/t/tests/18_basic_client_verification_fail2/apache.conf
b/t/tests/18_basic_client_verification_fail2/apache.conf
new file mode 100644
index 0000000..aec01d9
--- /dev/null
+++ b/t/tests/18_basic_client_verification_fail2/apache.conf
@@ -0,0 +1,13 @@
+Include ${PWD}/../../base_apache.conf
+
+GnuTLSCache dbm cache/gnutls_cache
+
+<VirtualHost ${TEST_IP}:${TEST_PORT}>
+ ServerName ${TEST_HOST}
+ GnuTLSEnable On
+ GnuTLSCertificateFile server/x509.pem
+ GnuTLSKeyFile server/secret.key
+ GnuTLSPriorities NORMAL
+ GnuTLSClientCAFile authority/x509.pem
+ GnuTLSClientVerify require
+</VirtualHost>
diff --git a/t/tests/18_basic_client_verification_fail2/fail.client
b/t/tests/18_basic_client_verification_fail2/fail.client
new file mode 100644
index 0000000..e69de29
diff --git a/t/tests/18_basic_client_verification_fail2/gnutls-cli.args
b/t/tests/18_basic_client_verification_fail2/gnutls-cli.args
new file mode 100644
index 0000000..23f74f1
--- /dev/null
+++ b/t/tests/18_basic_client_verification_fail2/gnutls-cli.args
@@ -0,0 +1,4 @@
+--x509certfile=../../impostor/x509.pem
+--x509keyfile=../../impostor/secret.key
+--x509cafile=../../authority/x509.pem
+--priority=NORMAL
diff --git a/t/tests/18_basic_client_verification_fail2/input
b/t/tests/18_basic_client_verification_fail2/input
new file mode 100644
index 0000000..9ba3394
--- /dev/null
+++ b/t/tests/18_basic_client_verification_fail2/input
@@ -0,0 +1,3 @@
+GET /test.txt HTTP/1.1
+Host: __HOSTNAME__
+




On Fri, Mar 14, 2014 at 2:28 PM, Daniel Kahn Gillmor
<dkg at fifthhorseman.net>wrote:

> On 03/14/2014 02:58 AM, Ramkumar Chinchani wrote:
> > When GnuTLSClientVerify method is set to cartel or msva,
> mgs_cert_verify()
> > correctly returns HTTP_FORBIDDEN when verification fails.
> >
> > However, when GnuTLSClientVerify is set to "require" at server-level and
> > not at directory-level, mgs_hook_authz() doesn't seem to honor the return
> > code properly for this case.
> >
> > Kindly review the following patch.
> >
> > diff --git a/src/gnutls_hooks.c b/src/gnutls_hooks.c
> > index d068ebb..5bfc2b6 100644
> > --- a/src/gnutls_hooks.c
> > +++ b/src/gnutls_hooks.c
> > @@ -878,7 +878,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;
> >          }
> >      }
>
>
> Thanks, Ramkumar!  it sounds like you've got a couple test cases in mind
> for this.  can you add new test cases to avoid this kind of breakage in
> the future?  It seems funny to introduce two test cases to verify a
> 3-line patch, but they should help us avoid seeing this problem crop up
> again in the future.
>
>         --dkg
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/attachments/20140319/92342688/attachment.html>