You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Vlad Georgescu <vg...@gmail.com> on 2006/11/22 13:14:42 UTC

[PATCH] Add support for SASL security layers to ra_svn

Hi,

Some SASL mechanisms (e.g. DIGEST-MD5, GSSAPI, etc.) support security
layers (i.e. encryption).  This patch adds support for such layers,
using the new svn_ra_svn__stream_t interface introduced in r22238. The
presence and level of encryption are negotiated during the
authentication exchange.  On the server side, two new svnserve.conf
options (min-encryption and max-encryption) can be used in this
negotiation.  For example, setting both to 0 would disable encryption,
setting min-encryption to 128 would require at least 128-bit
encryption etc.

[[[
Add support for SASL security layers to ra_svn.

* subversion/libsvn_ra_svn/ra_svn.h
 (svn_ra_svn_conn_st): New member 'encrypted'.

* subvesion/libsvn_ra_svn/marshal.c
 (svn_ra_svn_create_conn): Initialize conn->encrypted.

* subversion/libsvn_ra_svn/sasl_auth.c
 (sasl_baton_t): New typedef.
 (sasl_read_cb, sasl_write_cb, sasl_timeout_cb, sasl_pending_cb): Implement
  the svn_ra_svn__stream_t interface.
 (svn_ra_svn__enable_sasl_encryption): New function. Wraps the existing
  stream with a SASL encrypted stream.
 (svn_ra_svn__do_auth): Make sure sasl_ctx has the same lifetime as the
  connection. Call svn_ra_svn__enable_sasl_encryption.

* subversion/include/private/ra_svn_sasl.h
 (svn_ra_svn__enable_sasl_encryption): New declaration.

* subversion/include/svn_config.h
 (SVN_CONFIG_OPTION_MIN_SSF, SVN_CONFIG_OPTION_MAX_SSF): New options for
  svnserve.conf.

* subversion/libsvn_repos/repos.c
 (create_conf): Add the new options to the default svnserve.conf.

* subversion/svnserve/sasl_auth.c
 (sasl_auth_request): Make sure sasl_ctx has the same lifetime as the
  connection. Read the new svnserve.conf options. Call
  svn_ra_svn__enable_sasl_encryption after the authentication exchange.
]]]

-- 
Vlad

Re: [PATCH] Add support for SASL security layers to ra_svn

Posted by Vlad Georgescu <vg...@gmail.com>.
Vlad Georgescu wrote:
> On 1/4/07, Hyrum K. Wright <hy...@mail.utexas.edu> wrote:
>> FYI, Now that Vlad's a full committer, I'm going to stop tracking this
>> patch.
>>
>> -Hyrum
> 
> Yeah, I'll probably commit it soon, if nobody objects.
> 

Committed in r23155.

-- 
Vlad

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

Re: [PATCH] Add support for SASL security layers to ra_svn

Posted by Vlad Georgescu <vg...@gmail.com>.
On 1/4/07, Hyrum K. Wright <hy...@mail.utexas.edu> wrote:
> FYI, Now that Vlad's a full committer, I'm going to stop tracking this
> patch.
>
> -Hyrum

Yeah, I'll probably commit it soon, if nobody objects.

-- 
Vlad

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

Re: [PATCH] Add support for SASL security layers to ra_svn

Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Garrett Rooney wrote:
> On 12/4/06, Hyrum K. Wright <hy...@mail.utexas.edu> wrote:
>> Vlad Georgescu wrote:
>> > Hi,
>> >
>> > Some SASL mechanisms (e.g. DIGEST-MD5, GSSAPI, etc.) support security
>> > layers (i.e. encryption).  This patch adds support for such layers,
>> > using the new svn_ra_svn__stream_t interface introduced in r22238. The
>> > presence and level of encryption are negotiated during the
>> > authentication exchange.  On the server side, two new svnserve.conf
>> > options (min-encryption and max-encryption) can be used in this
>> > negotiation.  For example, setting both to 0 would disable encryption,
>> > setting min-encryption to 128 would require at least 128-bit
>> > encryption etc.
>>
>> Ping...
>>
>> Has any of the committers looked at Vlad's patch?  If nothing happens
>> with it in the next few days, I'll go ahead and file an issue.
> 
> I've actually got this patch sitting in my local working copy, and
> just need to find some time to do some more testing before checking it
> in.  Been a bit busy lately, but so far it looks good.
> 
> Needless to say, if someone else gets to it first I would absolutely
> not object ;-)

FYI, Now that Vlad's a full committer, I'm going to stop tracking this
patch.

-Hyrum


Re: [PATCH] Add support for SASL security layers to ra_svn

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 12/4/06, Hyrum K. Wright <hy...@mail.utexas.edu> wrote:
> Vlad Georgescu wrote:
> > Hi,
> >
> > Some SASL mechanisms (e.g. DIGEST-MD5, GSSAPI, etc.) support security
> > layers (i.e. encryption).  This patch adds support for such layers,
> > using the new svn_ra_svn__stream_t interface introduced in r22238. The
> > presence and level of encryption are negotiated during the
> > authentication exchange.  On the server side, two new svnserve.conf
> > options (min-encryption and max-encryption) can be used in this
> > negotiation.  For example, setting both to 0 would disable encryption,
> > setting min-encryption to 128 would require at least 128-bit
> > encryption etc.
>
> Ping...
>
> Has any of the committers looked at Vlad's patch?  If nothing happens
> with it in the next few days, I'll go ahead and file an issue.

I've actually got this patch sitting in my local working copy, and
just need to find some time to do some more testing before checking it
in.  Been a bit busy lately, but so far it looks good.

Needless to say, if someone else gets to it first I would absolutely
not object ;-)

-garrett

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

Re: [PATCH] Add support for SASL security layers to ra_svn

Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Vlad Georgescu wrote:
> Hi,
> 
> Some SASL mechanisms (e.g. DIGEST-MD5, GSSAPI, etc.) support security
> layers (i.e. encryption).  This patch adds support for such layers,
> using the new svn_ra_svn__stream_t interface introduced in r22238. The
> presence and level of encryption are negotiated during the
> authentication exchange.  On the server side, two new svnserve.conf
> options (min-encryption and max-encryption) can be used in this
> negotiation.  For example, setting both to 0 would disable encryption,
> setting min-encryption to 128 would require at least 128-bit
> encryption etc.

Ping...

Has any of the committers looked at Vlad's patch?  If nothing happens
with it in the next few days, I'll go ahead and file an issue.

-Hyrum

> [[[
> Add support for SASL security layers to ra_svn.
> 
> * subversion/libsvn_ra_svn/ra_svn.h
> (svn_ra_svn_conn_st): New member 'encrypted'.
> 
> * subvesion/libsvn_ra_svn/marshal.c
> (svn_ra_svn_create_conn): Initialize conn->encrypted.
> 
> * subversion/libsvn_ra_svn/sasl_auth.c
> (sasl_baton_t): New typedef.
> (sasl_read_cb, sasl_write_cb, sasl_timeout_cb, sasl_pending_cb): Implement
>  the svn_ra_svn__stream_t interface.
> (svn_ra_svn__enable_sasl_encryption): New function. Wraps the existing
>  stream with a SASL encrypted stream.
> (svn_ra_svn__do_auth): Make sure sasl_ctx has the same lifetime as the
>  connection. Call svn_ra_svn__enable_sasl_encryption.
> 
> * subversion/include/private/ra_svn_sasl.h
> (svn_ra_svn__enable_sasl_encryption): New declaration.
> 
> * subversion/include/svn_config.h
> (SVN_CONFIG_OPTION_MIN_SSF, SVN_CONFIG_OPTION_MAX_SSF): New options for
>  svnserve.conf.
> 
> * subversion/libsvn_repos/repos.c
> (create_conf): Add the new options to the default svnserve.conf.
> 
> * subversion/svnserve/sasl_auth.c
> (sasl_auth_request): Make sure sasl_ctx has the same lifetime as the
>  connection. Read the new svnserve.conf options. Call
>  svn_ra_svn__enable_sasl_encryption after the authentication exchange.
> ]]]
> 
> 
> ------------------------------------------------------------------------
> 
> Index: subversion/include/private/ra_svn_sasl.h
> ===================================================================
> --- subversion/include/private/ra_svn_sasl.h	(revision 22383)
> +++ subversion/include/private/ra_svn_sasl.h	(working copy)
> @@ -50,6 +50,13 @@
>                                         const char **remote_addrport,
>                                         svn_ra_svn_conn_t *conn,
>                                         apr_pool_t *pool);
> +
> +/* If a security layer was negotiated during the authentication exchange,
> +   create an encrypted stream for conn. */
> +svn_error_t *svn_ra_svn__enable_sasl_encryption(svn_ra_svn_conn_t *conn,
> +                                                sasl_conn_t *sasl_ctx,
> +                                                apr_pool_t *pool);
> +
>  #ifdef __cplusplus
>  }
>  #endif /* __cplusplus */
> Index: subversion/include/svn_config.h
> ===================================================================
> --- subversion/include/svn_config.h	(revision 22383)
> +++ subversion/include/svn_config.h	(working copy)
> @@ -107,6 +107,8 @@
>  #define SVN_CONFIG_OPTION_AUTHZ_DB                  "authz-db"
>  #define SVN_CONFIG_SECTION_SASL                 "sasl"
>  #define SVN_CONFIG_OPTION_USE_SASL                  "use-sasl"
> +#define SVN_CONFIG_OPTION_MIN_SSF                   "min-encryption"
> +#define SVN_CONFIG_OPTION_MAX_SSF                   "max-encryption"
>  
>  /* For repository password database */
>  #define SVN_CONFIG_SECTION_USERS                "users"
> Index: subversion/libsvn_ra_svn/marshal.c
> ===================================================================
> --- subversion/libsvn_ra_svn/marshal.c	(revision 22383)
> +++ subversion/libsvn_ra_svn/marshal.c	(working copy)
> @@ -52,6 +52,7 @@
>    assert((sock && !in_file && !out_file) || (!sock && in_file && out_file));
>  #ifdef SVN_HAVE_SASL
>    conn->sock = sock;
> +  conn->encrypted = FALSE;
>  #endif
>    conn->session = NULL;
>    conn->read_ptr = conn->read_buf;
> Index: subversion/libsvn_ra_svn/ra_svn.h
> ===================================================================
> --- subversion/libsvn_ra_svn/ra_svn.h	(revision 22383)
> +++ subversion/libsvn_ra_svn/ra_svn.h	(working copy)
> @@ -68,6 +68,7 @@
>       interface, SASL still needs direct access to the underlying socket
>       for stuff like IP addresses and port numbers. */
>    apr_socket_t *sock;
> +  svn_boolean_t encrypted;
>  #endif
>    char read_buf[SVN_RA_SVN__READBUF_SIZE];
>    char *read_ptr;
> Index: subversion/libsvn_ra_svn/sasl_auth.c
> ===================================================================
> --- subversion/libsvn_ra_svn/sasl_auth.c	(revision 22383)
> +++ subversion/libsvn_ra_svn/sasl_auth.c	(working copy)
> @@ -443,6 +443,186 @@
>    return SVN_NO_ERROR;
>  }
>  
> +/* Baton for a SASL encrypted svn_ra_svn__stream_t. */
> +typedef struct sasl_baton {
> +  svn_ra_svn__stream_t *stream; /* Inherited stream. */
> +  sasl_conn_t *ctx;             /* The SASL context for this connection. */
> +  unsigned int maxsize;         /* The maximum amount of data we can encode. */
> +  const char *read_buf;         /* The buffer returned by sasl_decode. */
> +  unsigned int read_len;        /* Its current length. */
> +  const char *write_buf;        /* The buffer returned by sasl_encode. */
> +  unsigned int write_len;       /* Its length. */
> +} sasl_baton_t;
> +
> +/* Functions to implement a SASL encrypted svn_ra_svn__stream_t. */
> +
> +/* Implements svn_read_fn_t. */
> +static svn_error_t *sasl_read_cb(void *baton, char *buffer, apr_size_t *len)
> +{
> +  sasl_baton_t *sasl_baton = baton;
> +  int result;
> +  /* A copy of *len, used by the wrapped stream. */
> +  unsigned int len2 = *len;
> +
> +  /* sasl_decode might need more data than a single read can provide,
> +     hence the need to put a loop around the decoding. */
> +  while (! sasl_baton->read_buf || sasl_baton->read_len == 0)
> +    {
> +      SVN_ERR(svn_ra_svn__stream_read(sasl_baton->stream, buffer, &len2));
> +      if (len2 == 0)
> +        {
> +          *len = 0;
> +          return SVN_NO_ERROR;
> +        }
> +      result = sasl_decode(sasl_baton->ctx, buffer, len2, 
> +                           &sasl_baton->read_buf, 
> +                           &sasl_baton->read_len);
> +      if (result != SASL_OK)
> +        return svn_error_create(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
> +                                sasl_errdetail(sasl_baton->ctx));
> +    }
> +
> +  /* The buffer returned by sasl_decode might be larger than what the 
> +     caller wants.  If this is the case, we only copy back *len bytes now
> +     (the rest will be returned by subsequent calls to this function).
> +     If not, we just copy back the whole thing. */
> +  if (*len >= sasl_baton->read_len)
> +    {
> +      memcpy(buffer, sasl_baton->read_buf, sasl_baton->read_len);
> +      *len = sasl_baton->read_len;
> +      sasl_baton->read_buf = NULL;
> +      sasl_baton->read_len = 0;
> +    }
> +  else
> +    {
> +      memcpy(buffer, sasl_baton->read_buf, *len);
> +      sasl_baton->read_len -= *len;
> +      sasl_baton->read_buf += *len;
> +    }
> +
> +  return SVN_NO_ERROR;
> +}
> +
> +/* Implements svn_write_fn_t. */
> +static svn_error_t *
> +sasl_write_cb(void *baton, const char *buffer, apr_size_t *len)
> +{
> +  sasl_baton_t *sasl_baton = baton;
> +  int result;
> +
> +  if (! sasl_baton->write_buf || sasl_baton->write_len == 0)
> +    {
> +      /* Make sure we don't write too much. */
> +      *len = (*len > sasl_baton->maxsize) ? sasl_baton->maxsize : *len;
> +      result = sasl_encode(sasl_baton->ctx, buffer, *len, 
> +                           &sasl_baton->write_buf, 
> +                           &sasl_baton->write_len);
> +
> +      if (result != SASL_OK)
> +        return svn_error_create(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
> +                                sasl_errdetail(sasl_baton->ctx));
> +    }
> +
> +  do 
> +    {
> +      unsigned int tmplen = sasl_baton->write_len;
> +      SVN_ERR(svn_ra_svn__stream_write(sasl_baton->stream, 
> +                                       sasl_baton->write_buf, 
> +                                       &tmplen));
> +      if (tmplen == 0)
> +      {
> +        /* The output buffer and its length will be preserved in sasl_baton
> +           and will be written out during the next call to this function 
> +           (which will have the same arguments). */
> +        *len = 0;
> +        return SVN_NO_ERROR;
> +      }
> +      sasl_baton->write_len -= tmplen;
> +      sasl_baton->write_buf += tmplen;
> +    } 
> +  while (sasl_baton->write_len > 0);
> +
> +  sasl_baton->write_buf = NULL;
> +  sasl_baton->write_len = 0;
> +
> +  return SVN_NO_ERROR;
> +}
> +
> +/* Implements ra_svn_timeout_fn_t. */
> +static void sasl_timeout_cb(void *baton, apr_interval_time_t interval)
> +{
> +  sasl_baton_t *sasl_baton = baton;
> +  svn_ra_svn__stream_timeout(sasl_baton->stream, interval);
> +}
> +
> +/* Implements ra_svn_pending_fn_t. */
> +static svn_boolean_t sasl_pending_cb(void *baton)
> +{
> +  sasl_baton_t *sasl_baton = baton;
> +  return svn_ra_svn__stream_pending(sasl_baton->stream);
> +}
> +
> +svn_error_t *svn_ra_svn__enable_sasl_encryption(svn_ra_svn_conn_t *conn,
> +                                                sasl_conn_t *sasl_ctx,
> +                                                apr_pool_t *pool)
> +{
> +  sasl_baton_t *sasl_baton;
> +  const sasl_ssf_t *ssfp;
> +  int result;
> +  const void *maxsize;
> +
> +  if (! conn->encrypted)
> +    {
> +      /* Get the strength of the security layer. */
> +      result = sasl_getprop(sasl_ctx, SASL_SSF, (void*) &ssfp);
> +      if (result != SASL_OK)
> +        return svn_error_create(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
> +                                sasl_errdetail(sasl_ctx));
> +
> +      if (*ssfp > 0)
> +        {
> +          /* Flush the connection, as we're about to replace its stream. */
> +          SVN_ERR(svn_ra_svn_flush(conn, pool));
> +
> +          /* Create and initialize the stream baton. */
> +          sasl_baton = apr_pcalloc(conn->pool, sizeof(*sasl_baton));
> +          sasl_baton->ctx = sasl_ctx;
> +
> +          /* Find out the maximum input size for sasl_encode. */
> +          result = sasl_getprop(sasl_ctx, SASL_MAXOUTBUF, &maxsize);
> +          if (result != SASL_OK)
> +            return svn_error_create(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
> +                                    sasl_errdetail(sasl_ctx));
> +          sasl_baton->maxsize = *((unsigned int *) maxsize);
> +
> +          /* If there is any data left in the read buffer at this point,
> +             we need to decrypt it. */
> +          if (conn->read_end > conn->read_ptr)
> +            {
> +              result = sasl_decode(sasl_ctx, conn->read_ptr,
> +                                   conn->read_end - conn->read_ptr,
> +                                   &sasl_baton->read_buf,
> +                                   &sasl_baton->read_len);
> +              if (result != SASL_OK)
> +                return svn_error_create(SVN_ERR_RA_NOT_AUTHORIZED, NULL,
> +                                        sasl_errdetail(sasl_ctx));
> +              conn->read_end = conn->read_ptr;
> +            }
> +
> +          /* Wrap the existing stream. */
> +          sasl_baton->stream = conn->stream;
> +
> +          conn->stream = svn_ra_svn__stream_create(sasl_baton, sasl_read_cb,
> +                                                   sasl_write_cb, 
> +                                                   sasl_timeout_cb,
> +                                                   sasl_pending_cb, conn->pool);
> +          /* Yay, we have a security layer! */
> +          conn->encrypted = TRUE;
> +        }
> +    }
> +  return SVN_NO_ERROR;
> +}
> +
>  svn_error_t *svn_ra_svn__get_addresses(const char **local_addrport, 
>                                         const char **remote_addrport,
>                                         svn_ra_svn_conn_t *conn,
> @@ -556,7 +736,7 @@
>  
>        SVN_ERR(new_sasl_ctx(&sasl_ctx, sess->is_tunneled,
>                             hostname, local_addrport, remote_addrport,
> -                           pool));
> +                           sess->conn->pool));
>        SVN_ERR(try_auth(sess,
>                         sasl_ctx,
>                         creds,
> @@ -578,6 +758,8 @@
>    while (!success);
>    svn_pool_destroy(subpool);
>  
> +  SVN_ERR(svn_ra_svn__enable_sasl_encryption(sess->conn, sasl_ctx, pool));
> +
>    SVN_ERR(svn_auth_save_credentials(iterstate, pool));
>  
>    return SVN_NO_ERROR;
> Index: subversion/libsvn_repos/repos.c
> ===================================================================
> --- subversion/libsvn_repos/repos.c	(revision 22383)
> +++ subversion/libsvn_repos/repos.c	(working copy)
> @@ -1460,6 +1460,20 @@
>        "### library for authentication. Default is false."
>        APR_EOL_STR
>        "# use-sasl = true"
> +      APR_EOL_STR
> +      "### These options specify the desired strength of the security layer"
> +      APR_EOL_STR
> +      "### that you want SASL to provide. 0 means no encryption, 1 means"
> +      APR_EOL_STR
> +      "### integrity-checking only, values larger than 1 are correlated"
> +      APR_EOL_STR
> +      "### to the effective key length for encryption (e.g. 128 means 128-bit"
> +      APR_EOL_STR
> +      "### encryption). The values below are the defaults."
> +      APR_EOL_STR
> +      "# min-encryption = 0"
> +      APR_EOL_STR
> +      "# max-encryption = 256"
>  #endif
>        APR_EOL_STR;
>  
> Index: subversion/svnserve/sasl_auth.c
> ===================================================================
> --- subversion/svnserve/sasl_auth.c	(revision 22383)
> +++ subversion/svnserve/sasl_auth.c	(working copy)
> @@ -229,7 +229,7 @@
>    apr_pool_t *subpool;
>    apr_status_t apr_err;
>    const char *localaddrport = NULL, *remoteaddrport = NULL;
> -  const char *mechlist;
> +  const char *mechlist, *val;
>    char hostname[APRMAXHOSTLEN + 1];
>    sasl_security_properties_t secprops;
>    svn_boolean_t success, no_anonymous;
> @@ -261,7 +261,7 @@
>      }
>  
>    /* Make sure the context is always destroyed. */
> -  apr_pool_cleanup_register(pool, sasl_ctx, sasl_dispose_cb,
> +  apr_pool_cleanup_register(b->pool, sasl_ctx, sasl_dispose_cb,
>                              apr_pool_cleanup_null);
>  
>    /* Initialize security properties. */
> @@ -275,6 +275,18 @@
>    if (no_anonymous)
>      secprops.security_flags |= SASL_SEC_NOANONYMOUS;
>  
> +  svn_config_get(b->cfg, &val, 
> +                 SVN_CONFIG_SECTION_SASL,
> +                 SVN_CONFIG_OPTION_MIN_SSF,
> +                 "0");
> +  secprops.min_ssf = atoi(val);
> +
> +  svn_config_get(b->cfg, &val,
> +                 SVN_CONFIG_SECTION_SASL,
> +                 SVN_CONFIG_OPTION_MAX_SSF,
> +                 "256");
> +  secprops.max_ssf = atoi(val);
> +
>    /* Set security properties. */
>    result = sasl_setprop(sasl_ctx, SASL_SEC_PROPS, &secprops);
>    if (result != SASL_OK)
> @@ -316,6 +328,8 @@
>    while (!success);
>    svn_pool_destroy(subpool);
>  
> +  SVN_ERR(svn_ra_svn__enable_sasl_encryption(conn, sasl_ctx, pool));
> +
>    if (no_anonymous)
>      {
>        char *p;