[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Allow non-SSL client to connect to SSL enabled svnserve

From: Sigfred Håversen <bsdlist_at_mumak.com>
Date: 2004-11-03 00:53:53 CET

On Wednesday 03 November 2004 00.35, Julian Foad wrote:
> Sigfred Håversen wrote:
> > * subversion/libsvn_ra_svn/marshal.c
> > (writebuf_output_ssl): Do not write buffer if size less than zero.
>
> Less than or equal to zero.
>
> > * subversion/svnserve/serve.c
> > (serve): Permit connection from client lacking SSL capability, even
> > svnserve
>
> "even if svnserve" ...?
>
> > has that capability.
>

Thanks. Changed patch below.

/Sigfred

A client lacking SSL capability can now connect to a svnserve that has that
capability. This should allow compatability with older clients. Fixed a bug
in writebuf_output_ssl where buffers of length zero was written. Other changes
are mostly cosmetic.

* subversion/libsvn_ra_svn/client.c
  (asn1time_to_string): Fix comment.
  (match_hostname): Fix comment.
  (verify_hostname): Fix comments.
  (fill_server_cert_info): Fix comment/indentation.
  (do_ssl_auth): Fix comment.

* subversion/libsvn_ra_svn/marshal.c
  (writebuf_output_ssl): Do not write buffer if size less than or
    equal to zero. Fix indentation.
  (network_biopair_interop): Fix comments.
  (do_ssl_operation): Fix comments/indentation.
  (cleanup_ssl): Fix comment/indentation.
  (svn_ra_svn_ssl_init):Fix comments.

* subversion/svnserve/main.c
  (init_ssl_ctx): Fix comment/indentation.
  (main): Fix indentation.

* subversion/svnserve/serve.c
  (serve): Permit connection from client lacking SSL capability, even if
    svnserve has that capability.

Index: subversion/libsvn_ra_svn/client.c
===================================================================
--- subversion/libsvn_ra_svn/client.c (revision 11694)
+++ subversion/libsvn_ra_svn/client.c (working copy)
@@ -273,9 +273,7 @@
   return SVN_NO_ERROR;
 }
 
-/* Format an ASN1 time to a string.
- */
-
+/* Format an ASN1 time to a string. */
 static svn_boolean_t asn1time_to_string(ASN1_TIME *tm, char *buffer,
                                         apr_size_t len)
 {
@@ -303,8 +301,7 @@
 }
 
 /* Compare peername against hostname. Allow wildcard in leftmost
- * position in peername, and the comparision is case insensitive.
- */
+ * position in peername, and the comparision is case insensitive. */
 static svn_boolean_t match_hostname(const char *peername,
                                     const char *hostname, apr_pool_t *pool)
 {
@@ -330,8 +327,7 @@
 
 /* Verify that the certificates common name matches the hostname.
  * Adapted from verify_callback() in postfixtls patch by Lutz Jaenicke
- * at http://www.aet.tu-cottbus.de/personen/jaenicke/postfix_tls/
- */
+ * at http://www.aet.tu-cottbus.de/personen/jaenicke/postfix_tls/ */
 static svn_boolean_t verify_hostname(ra_svn_session_baton_t *sess,
                                      apr_pool_t *pool,
                                      svn_auth_ssl_server_cert_info_t *cert_info)
@@ -342,15 +338,12 @@
   X509 *peer = SSL_get_peer_certificate(sess->conn->ssl);
   STACK_OF(GENERAL_NAME) *gens;
 
- /*
- * Check out the name certified against the hostname expected.
+ /* Check out the name certified against the hostname expected.
    * Standards are not always clear with respect to the handling of
    * dNSNames. RFC3207 does not specify the handling. We therefore follow
    * the strict rules in RFC2818 (HTTP over TLS), Section 3.1:
    * The Subject Alternative Name/dNSName has precedence over CommonName
- * (CN). If dNSName entries are provided, CN is not checked anymore.
- */
-
+ * (CN). If dNSName entries are provided, CN is not checked anymore. */
   gens = X509_get_ext_d2i(peer, NID_subject_alt_name, 0, 0);
   if (gens)
     {
@@ -374,12 +367,11 @@
 }
 
 /* Fill in the server certificate information, as well as check for some
- * errors in the certificate.
- */
+ * errors in the certificate. */
 static svn_error_t *fill_server_cert_info(ra_svn_session_baton_t *sess,
- apr_pool_t *pool,
- svn_auth_ssl_server_cert_info_t *cert_info,
- apr_uint32_t *cert_failures)
+ apr_pool_t *pool,
+ svn_auth_ssl_server_cert_info_t *cert_info,
+ apr_uint32_t *cert_failures)
 {
   #define CERT_BUFFER_SIZE 256
   const char hexcodes[] = "0123456789ABCDEF";
@@ -501,8 +493,7 @@
   return SVN_NO_ERROR;
 }
 
-/* Authenticate the server certificate.
- */
+/* Authenticate the server certificate. */
 static svn_error_t *do_ssl_auth(ra_svn_session_baton_t *sess,
                                 apr_pool_t *pool)
 {
Index: subversion/libsvn_ra_svn/marshal.c
===================================================================
--- subversion/libsvn_ra_svn/marshal.c (revision 11694)
+++ subversion/libsvn_ra_svn/marshal.c (working copy)
@@ -168,12 +168,11 @@
                                         apr_pool_t *pool, const char *data,
                                         apr_size_t len)
 {
- svn_error_t *err;
+ svn_error_t *err = SVN_NO_ERROR;
   apr_pool_t *subpool = NULL;
 
- err = do_ssl_operation(conn, NULL, NULL, SSL_write, (char *) data, &len);
- if (err)
- return err;
+ if (len > 0)
+ SVN_ERR(do_ssl_operation(conn, NULL, NULL, SSL_write, (char *) data, &len));
 
   subpool = svn_pool_create(pool);
   if (conn->block_handler != NULL)
@@ -960,15 +959,13 @@
 
 /* --- SSL FUNCTIONS --- */
 
-/*
- * The interface layer between network and BIO-pair. The BIO-pair buffers
+/* The interface layer between network and BIO-pair. The BIO-pair buffers
  * the data to/from the TLS layer. Hence, at any time, there may be data
  * in the buffer that must be written to the network. This writing has
  * highest priority because the handshake might fail otherwise.
  * Only then a read_request can be satisfied.
  * Adapted from network_biopair_interop() in postfixtls patch by Lutz Jaenicke
- * at http://www.aet.tu-cottbus.de/personen/jaenicke/postfix_tls/
- */
+ * at http://www.aet.tu-cottbus.de/personen/jaenicke/postfix_tls/ */
 static svn_error_t *network_biopair_interop(svn_ra_svn_conn_t *conn)
 {
   int want_write;
@@ -988,14 +985,11 @@
           want_write = NETLAYER_BUFFERSIZE;
         from_bio = BIO_read(conn->network_bio, buffer, want_write);
         
- /*
- * Write the complete contents of the buffer. Since TLS performs
- * underlying handshaking, we cannot afford to leave the buffer
- * unflushed, as we could run into a deadlock trap (the peer
- * waiting for a final byte and we already waiting for his reply
- * in read position).
- */
-
+ /* Write the complete contents of the buffer. Since TLS performs
+ * underlying handshaking, we cannot afford to leave the buffer
+ * unflushed, as we could run into a deadlock trap (the peer
+ * waiting for a final byte and we already waiting for his reply
+ * in read position). */
       write_pos = 0;
       do {
           num_write = from_bio - write_pos;
@@ -1028,8 +1022,7 @@
   return SVN_NO_ERROR;
 }
 
-/*
- * Function to perform the handshake for SSL_accept(), SSL_connect(),
+/* Function to perform the handshake for SSL_accept(), SSL_connect(),
  * and SSL_shutdown() and perform the SSL_read(), SSL_write() operations.
  * Call the underlying network_biopair_interop-layer to make sure the
  * write buffer is flushed after every operation (that did not fail with
@@ -1039,8 +1032,7 @@
  * if the operation was successfull.
  *
  * Adapted from do_tls_operation() in postfixtls patch by Lutz Jaenicke
- * at http://www.aet.tu-cottbus.de/personen/jaenicke/postfix_tls/
- */
+ * at http://www.aet.tu-cottbus.de/personen/jaenicke/postfix_tls/ */
 static svn_error_t *do_ssl_operation(svn_ra_svn_conn_t *conn,
                                      int (*hsfunc)(SSL *),
                                      int (*rfunc)(SSL *, void *, int),
@@ -1090,8 +1082,8 @@
   if (ret_status > 0)
     return SVN_NO_ERROR;
   else
- return svn_error_create(SVN_ERR_RA_SVN_SSL_ERROR, NULL,
- _("SSL network problem"));
+ return svn_error_create(SVN_ERR_RA_SVN_SSL_ERROR, NULL,
+ _("SSL network problem"));
 }
 
 /* Releases the resources allocated by SSL. */
@@ -1103,7 +1095,7 @@
     return APR_SUCCESS;;
 
   /* The connection has been setup between client and server,
- so we tell the other side that we are finished. */
+ * so we tell the other side that we are finished. */
   if (conn->use_ssl)
     {
       if (!do_ssl_operation(conn, SSL_shutdown, NULL, NULL, NULL, NULL))
@@ -1117,10 +1109,10 @@
   conn->internal_bio = NULL;
 
   if (conn->network_bio)
- {
- BIO_free(conn->network_bio);
- conn->network_bio = NULL;
- }
+ {
+ BIO_free(conn->network_bio);
+ conn->network_bio = NULL;
+ }
 
   return APR_SUCCESS;
 }
@@ -1138,7 +1130,6 @@
  * +----------< BIO-pair (network_bio)
  * | |
  * socket |
- *
  */
 svn_error_t *svn_ra_svn_ssl_init(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
                                  void *ssl_ctx)
@@ -1147,7 +1138,7 @@
 
   /* Need to release SSL resources when the connection is destroyed.
    * Assumes that the owning SSL_CTX is destroyed after cleanup
- * of SSL.*/
+ * of SSL. */
   apr_pool_cleanup_register(pool, conn, cleanup_ssl, apr_pool_cleanup_null);
 
   conn->ssl = SSL_new(ssl_context);
Index: subversion/svnserve/main.c
===================================================================
--- subversion/svnserve/main.c (revision 11694)
+++ subversion/svnserve/main.c (working copy)
@@ -250,8 +250,7 @@
   SSL_library_init();
   
   /* TODO : Seed the randum number generator (RNG)
- * for those operating systems that does not have /dev/urandom.
- */
+ * for those operating systems that does not have /dev/urandom. */
 
   params->ssl_ctx = SSL_CTX_new(SSLv23_server_method());
   if (params->ssl_ctx == NULL)
@@ -280,7 +279,6 @@
               ssl_key_file, ssl_last_error(pool));
       return FALSE;
     }
-
           
   if (!SSL_CTX_check_private_key(params->ssl_ctx))
     {
@@ -428,7 +426,8 @@
         case SVNSERVE_OPT_CERT_FILE:
           SVN_INT_ERR(svn_utf_cstring_to_utf8(&ssl_cert_file, arg, pool));
           ssl_cert_file = svn_path_internal_style(ssl_cert_file, pool);
- SVN_INT_ERR(svn_path_get_absolute(&ssl_cert_file, ssl_cert_file, pool));
+ SVN_INT_ERR(svn_path_get_absolute(&ssl_cert_file, ssl_cert_file,
+ pool));
           break;
 
         case SVNSERVE_OPT_KEY_FILE:
Index: subversion/svnserve/serve.c
===================================================================
--- subversion/svnserve/serve.c (revision 11694)
+++ subversion/svnserve/serve.c (working copy)
@@ -1383,12 +1383,9 @@
                                      &caplist, &client_url));
       SVN_ERR(svn_ra_svn_set_capabilities(conn, caplist));
 
- if (params->ssl_layer)
+ if (params->ssl_layer
+ && svn_ra_svn_has_capability(conn, SVN_RA_SVN_CAP_SSL))
         {
- if (!svn_ra_svn_has_capability(conn, SVN_RA_SVN_CAP_SSL))
- return svn_error_create(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
- _("Client must have SSL capability"));
-
           /* Flush write buffer before SSL handshake. */
           svn_ra_svn_flush(conn, pool);
 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Nov 3 00:54:18 2004

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.