You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ben Reser <be...@reser.org> on 2003/11/27 05:01:24 UTC

[PATCH] Better handling of wrong passwords or no passwords for client certificates.

* subversion/libsvn_ra_dav/session.c
  (client_ssl_callback): Check return value of ne_ssl_clicert_decrypt.
  If the wrong password is given to decrypt then ne_ssl_set_clicert will
  segfault.

  (client_ssl_callback): When returning due to lack of password or
  bad password be sure to still destroy the pool.

Index: subversion/libsvn_ra_dav/session.c
===================================================================
--- subversion/libsvn_ra_dav/session.c  (revision 7859)
+++ subversion/libsvn_ra_dav/session.c  (working copy)
@@ -289,9 +289,16 @@
               char pw[128];
               if (client_ssl_keypw_callback(userdata, pw, 128))
                 {
-                  return; /* no password given */
+                  /* no password given */
+                  apr_pool_destroy(pool);
+                  return; 
                 }
-              ne_ssl_clicert_decrypt(clicert, pw);
+              if (ne_ssl_clicert_decrypt(clicert, pw))
+                {
+                  /* wrong pass probably */
+                  apr_pool_destroy(pool);
+                  return;
+                }
               ne_ssl_set_clicert(sess, clicert);
             }
           else if (clicert != NULL)


-- 
Ben Reser <be...@reser.org>
http://ben.reser.org

"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Better handling of wrong passwords or no passwords for client certificates.

Posted by Ben Reser <be...@reser.org>.
On Sat, Nov 29, 2003 at 08:31:12PM +0000, Philip Martin wrote:
> Thanks!  See r7872, I also made some other changes to tidy up the
> code.
> 
> Questions (not specifically for Ben)
> - do we need to notify the user if the decrypt fails?

I was wondering why there wasn't a retry setup for at least the pw if
not the certificate.  I think it makes a lot of sense to take and allow
clients to specify a retry limit.  They can't really be sure that the
certificate or the password were good.  

The other question is how do you notify the user that the decrypt
failed?  I don't think there's really any good way of telling the client
that and printing to STDERR from the library doesn't seem right either.

So I'd say we should have a retry limit, asking for the cert/pw again is
pretty intuitive that there's something wrong with what you gave before.

-- 
Ben Reser <be...@reser.org>
http://ben.reser.org

"Conscience is the inner voice which warns us somebody may be looking."
- H.L. Mencken

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Better handling of wrong passwords or no passwords for client certificates.

Posted by Philip Martin <ph...@codematters.co.uk>.
Ben Reser <be...@reser.org> writes:

> * subversion/libsvn_ra_dav/session.c
>   (client_ssl_callback): Check return value of ne_ssl_clicert_decrypt.
>   If the wrong password is given to decrypt then ne_ssl_set_clicert will
>   segfault.

Thanks!  See r7872, I also made some other changes to tidy up the
code.

Questions (not specifically for Ben)
- do we need to notify the user if the decrypt fails?
- any reason this file uses apr_pool_create/destroy rather than
  svn_pool_create/destroy?

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org