Attachment 23674 Details for Bug 60610

[patch] get_logs.patch

get_logs.patch (text/plain), 17.60 KB, created by Marcus Meissner on 2004-09-20 15:55:29 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcus Meissner

Created: 2004-09-20 15:55:29 UTC

Size: 17.60 KB

patch

obsolete

>Security fix for 1.0:  implement svn_repos__history2() and
>svn_repos__get_logs2() for private use by mod_dav_svn.  Both new
>functions use a read_authz callback.  The former uses the callback
>when walking backwards through history.  The latter uses the callback
>to validate changed-paths when returning a revision:
>
>   - If a revision has no readable changed-paths, push only the revnum
>     at the log reciever -- nothing else.
>     
>   - If a revision has a mixture of readable/unreadable changed-paths,
>     then omit the log message and the unreadable paths.
>
>(Note: this patch assumes you've applied the 'common' patch first.)
>
>* libsvn_repos/repos.h
>  (svn_repos__get_logs2): new declaration, takes an authz_func/baton.
>  (svn_repos__history2):  new declaration, takes an authz_func/baton.
>
>* libsvn_repos/rev_hunt.c
>  (svn_repos_history):  become dumb wrapper around svn_repos__history2().
>  (svn_repos__history2): validate the initial peg-object's readability.
>       also validate every history rev/path pair.  if an unreadable
>       pair is discovered, just stop tracing history and return.
>
>* libsvn_repos/log.c
>  (svn_repos_get_logs): become dumb wrapper around svn_repos__get_logs2().
>  (detect_changed): use authz_read func on every changed-path.  omit
>       unreadable paths from returned hash.  Return either no error or
>       one of two specific errorcodes.
>  (svn_repos_get_logs2): pass the authz_func to svn_repos_history2().
>       if authz_func is present, always call detect_changed().  if
>       detect_changed() returns 'unreadable' error, then only send
>       revnum, nothing else.  if it returns 'partially readable'
>       error, then omit logmsg.
>
>* mod_dav_svn/log.c
>  (repos.h): #include this "private" header in a creepy way.
>  (dav_svn__log_report): build an authz baton.  invoke svn_repos__get_logs2()
>      with the authz func.
>
>
>Index: subversion/mod_dav_svn/log.c
>===================================================================
>--- subversion/mod_dav_svn/log.c	(revision 10978)
>+++ subversion/mod_dav_svn/log.c	(working copy)
>@@ -32,6 +32,8 @@
> 
> #include "dav_svn.h"
> 
>+/* Private header included just for 1.0.7 get_logs() security fix. */
>+#include "../libsvn_repos/repos.h"
> 
> struct log_receiver_baton
> {
>@@ -208,6 +210,7 @@
>   dav_error *derr = NULL;
>   apr_xml_elem *child;
>   struct log_receiver_baton lrb;
>+  dav_svn_authz_read_baton arb;
>   const dav_svn_repos *repos = resource->info->repos;
>   const char *target = NULL;
>   int ns;
>@@ -279,6 +282,11 @@
>       /* else unknown element; skip it */
>     }
> 
>+  /* Build authz read baton */
>+  arb.r = resource->info->r;
>+  arb.repos = resource->info->repos;
>+
>+  /* Build log receiver baton */
>   lrb.bb = apr_brigade_create(resource->pool,  /* not the subpool! */
>                               output->c->bucket_alloc);
>   lrb.output = output;
>@@ -290,15 +298,17 @@
>      flag in our log_receiver_baton structure). */
> 
>   /* Send zero or more log items. */
>-  serr = svn_repos_get_logs(repos->repos,
>-                            paths,
>-                            start,
>-                            end,
>-                            discover_changed_paths,
>-                            strict_node_history,
>-                            log_receiver,
>-                            &lrb,
>-                            resource->pool);
>+  serr = svn_repos__get_logs2(repos->repos,
>+                              paths,
>+                              start,
>+                              end,
>+                              discover_changed_paths,
>+                              strict_node_history,
>+                              dav_svn_authz_read,
>+                              &arb,
>+                              log_receiver,
>+                              &lrb,
>+                              resource->pool);
>   if (serr)
>     {
>       derr = dav_svn_convert_err(serr, HTTP_BAD_REQUEST, serr->message,
>Index: subversion/libsvn_repos/log.c
>===================================================================
>--- subversion/libsvn_repos/log.c	(revision 10978)
>+++ subversion/libsvn_repos/log.c	(working copy)
>@@ -40,16 +40,34 @@
>  * the value is (void *) 'U', 'A', 'D', or 'R', for modified, added,
>  * deleted, or replaced, respectively.
>  * 
>+ * If optional AUTHZ_READ_FUNC is non-NULL, then use it (with
>+ * AUTHZ_READ_BATON and FS) to check whether each changed-path (and
>+ * copyfrom_path) is readable:
>+ *
>+ *     - If some paths are readable and some are not, then silently
>+ *     omit the unreadable paths from the CHANGED hash, and return
>+ *     SVN_ERR_ENTRY_NOT_FOUND.
>+ *
>+ *     - If absolutely every changed-path (and copyfrom_path) is
>+ *     unreadable, then return an empty CHANGED hash and
>+ *     APR_EGENERAL.  (This is to distinguish a revision
>+ *     which truly has no changed paths from a revision in which all
>+ *     paths are unreadable.)
>  */
> static svn_error_t *
> detect_changed (apr_hash_t **changed,
>                 svn_fs_root_t *root,
>+                svn_fs_t *fs,
>+                svn_repos_authz_func_t authz_read_func,
>+                void *authz_read_baton,
>                 apr_pool_t *pool)
> {
>   apr_hash_t *changes;
>   apr_hash_index_t *hi;
>   apr_pool_t *subpool = svn_pool_create (pool);
>-  
>+  svn_boolean_t found_readable = FALSE;
>+  svn_boolean_t found_unreadable = FALSE;
>+
>   *changed = apr_hash_make (pool);
>   SVN_ERR (svn_fs_paths_changed (&changes, root, pool));
>   for (hi = apr_hash_first (pool, changes); hi; hi = apr_hash_next (hi))
>@@ -68,6 +86,23 @@
>       path = (const char *) key;
>       change = val;
> 
>+      /* Skip path if unreadable. */
>+      if (authz_read_func)
>+        {
>+          svn_boolean_t readable;
>+          SVN_ERR (authz_read_func (&readable,
>+                                    root, path,
>+                                    authz_read_baton, subpool));
>+          if (! readable)
>+            {
>+              found_unreadable = TRUE;
>+              continue;
>+            }
>+        }
>+
>+      /* At least one changed-path was readable. */
>+      found_readable = TRUE;
>+
>       switch (change->change_kind)
>         {
>         case svn_fs_path_change_reset:
>@@ -101,10 +136,29 @@
> 
>           SVN_ERR (svn_fs_copied_from (&copyfrom_rev, &copyfrom_path,
>                                        root, path, subpool));
>+
>           if (copyfrom_path && SVN_IS_VALID_REVNUM (copyfrom_rev))
>             {
>-              item->copyfrom_path = apr_pstrdup (pool, copyfrom_path);
>-              item->copyfrom_rev = copyfrom_rev;
>+              svn_boolean_t readable = TRUE;
>+
>+              if (authz_read_func)
>+                {
>+                  svn_fs_root_t *copyfrom_root;
>+                  
>+                  SVN_ERR (svn_fs_revision_root (&copyfrom_root, fs,
>+                                                 copyfrom_rev, subpool));
>+                  SVN_ERR (authz_read_func (&readable,
>+                                            copyfrom_root, copyfrom_path,
>+                                            authz_read_baton, subpool));
>+                  if (! readable)
>+                    found_unreadable = TRUE;
>+                }
>+
>+              if (readable)
>+                {
>+                  item->copyfrom_path = apr_pstrdup (pool, copyfrom_path);
>+                  item->copyfrom_rev = copyfrom_rev;
>+                }
>             }
>         }
>       apr_hash_set (*changed, apr_pstrdup (pool, path), 
>@@ -112,6 +166,18 @@
>     }
> 
>   svn_pool_destroy (subpool);
>+
>+  if (! found_readable)
>+    /* Every changed-path was unreadable. */
>+    return svn_error_create (APR_EGENERAL,
>+                             NULL, NULL);
>+
>+  if (found_unreadable)
>+    /* At least one changed-path was unreadable. */
>+    return svn_error_create (SVN_ERR_ENTRY_NOT_FOUND,
>+                             NULL, NULL);
>+
>+  /* Every changed-path was readable. */
>   return SVN_NO_ERROR;
> }
> 
>@@ -132,15 +198,17 @@
> 
> 
> svn_error_t *
>-svn_repos_get_logs (svn_repos_t *repos,
>-                    const apr_array_header_t *paths,
>-                    svn_revnum_t start,
>-                    svn_revnum_t end,
>-                    svn_boolean_t discover_changed_paths,
>-                    svn_boolean_t strict_node_history,
>-                    svn_log_message_receiver_t receiver,
>-                    void *receiver_baton,
>-                    apr_pool_t *pool)
>+svn_repos__get_logs2 (svn_repos_t *repos,
>+                      const apr_array_header_t *paths,
>+                      svn_revnum_t start,
>+                      svn_revnum_t end,
>+                      svn_boolean_t discover_changed_paths,
>+                      svn_boolean_t strict_node_history,
>+                      svn_repos_authz_func_t authz_read_func,
>+                      void *authz_read_baton,
>+                      svn_log_message_receiver_t receiver,
>+                      void *receiver_baton,
>+                      apr_pool_t *pool)
> {
>   svn_revnum_t this_rev, head = SVN_INVALID_REVNUM;
>   apr_pool_t *subpool = svn_pool_create (pool);
>@@ -191,10 +259,12 @@
>           /* Get the changed revisions for this path. */
>           const char *this_path = APR_ARRAY_IDX (paths, 0, const char *);
>           revs = apr_array_make (pool, 64, sizeof (svn_revnum_t));
>-          SVN_ERR (svn_repos_history (fs, this_path, history_to_revs_array, 
>-                                      revs, start, end, 
>-                                      strict_node_history ? FALSE : TRUE, 
>-                                      pool));
>+          SVN_ERR (svn_repos__history2 (fs, this_path,
>+                                        history_to_revs_array, revs,
>+                                        authz_read_func, authz_read_baton,
>+                                        start, end, 
>+                                        strict_node_history ? FALSE : TRUE, 
>+                                        pool));
>         }
>       else
>         {
>@@ -212,10 +282,14 @@
> 
>               /* Get the changed revisions for this path, and add them to
>                  the hash (this will eliminate duplicates). */
>-              SVN_ERR (svn_repos_history (fs, this_path, history_to_revs_array,
>-                                          changed_revs, start, end, 
>-                                          strict_node_history ? FALSE : TRUE, 
>-                                          pool));
>+              SVN_ERR (svn_repos__history2 (fs, this_path,
>+                                            history_to_revs_array,
>+                                            changed_revs,
>+                                            authz_read_func, authz_read_baton,
>+                                            start, end, 
>+                                            strict_node_history
>+                                                       ? FALSE : TRUE, 
>+                                            pool));
>               for (j = 0; j < changed_revs->nelts; j++)
>                 {
>                   /* We're re-using the memory allocated for the array
>@@ -288,11 +362,45 @@
>          them (i.e., "svn log -v" means `discover_changed_paths' will
>          be non-zero here).  */
> 
>-      if ((this_rev > 0) && discover_changed_paths)
>+
>+      if ((this_rev > 0)        
>+          && (authz_read_func || discover_changed_paths))
>         {
>           svn_fs_root_t *newroot;
>+          svn_error_t *patherr;
>+
>           SVN_ERR (svn_fs_revision_root (&newroot, fs, this_rev, subpool));
>-          SVN_ERR (detect_changed (&changed_paths, newroot, subpool));
>+          patherr = detect_changed (&changed_paths,
>+                                    newroot, fs,
>+                                    authz_read_func, authz_read_baton,
>+                                    subpool);
>+
>+          if (patherr
>+              && patherr->apr_err == APR_EGENERAL)
>+            {
>+              /* All changed-paths are unreadable, so clear all fields. */
>+              svn_error_clear (patherr);              
>+              changed_paths = NULL;
>+              author = NULL;
>+              date = NULL;
>+              message = NULL;
>+            }
>+          else if (patherr
>+                   && patherr->apr_err == SVN_ERR_ENTRY_NOT_FOUND)
>+            {
>+              /* At least one changed-path was unreadable, so omit the
>+                 log message.  (The unreadable paths are already
>+                 missing from the hash.) */
>+              svn_error_clear (patherr);
>+              message = NULL;
>+            }
>+          else if (patherr)
>+            return patherr;
>+
>+          /* It may be the case that an authz func was passed in, but
>+             the user still doesn't want to see any changed-paths. */
>+          if (! discover_changed_paths)
>+            changed_paths = NULL;
>         }
> 
>       SVN_ERR ((*receiver) (receiver_baton,
>@@ -310,3 +418,22 @@
> 
>   return SVN_NO_ERROR;
> }
>+
>+
>+
>+svn_error_t *
>+svn_repos_get_logs (svn_repos_t *repos,
>+                    const apr_array_header_t *paths,
>+                    svn_revnum_t start,
>+                    svn_revnum_t end,
>+                    svn_boolean_t discover_changed_paths,
>+                    svn_boolean_t strict_node_history,
>+                    svn_log_message_receiver_t receiver,
>+                    void *receiver_baton,
>+                    apr_pool_t *pool)
>+{
>+  return svn_repos__get_logs2 (repos, paths, start, end,
>+                               discover_changed_paths, strict_node_history,
>+                               NULL, NULL, /* no authz stuff */
>+                               receiver, receiver_baton, pool);
>+}
>Index: subversion/libsvn_repos/rev_hunt.c
>===================================================================
>--- subversion/libsvn_repos/rev_hunt.c	(revision 10978)
>+++ subversion/libsvn_repos/rev_hunt.c	(working copy)
>@@ -170,6 +170,7 @@
> }
> 
> 
>+
> svn_error_t *
> svn_repos_history (svn_fs_t *fs,
>                    const char *path,
>@@ -180,6 +181,25 @@
>                    svn_boolean_t cross_copies,
>                    apr_pool_t *pool)
> {
>+  return svn_repos__history2 (fs, path, history_func, history_baton,
>+                              NULL, NULL,
>+                              start, end, cross_copies, pool);
>+}
>+
>+
>+
>+svn_error_t *
>+svn_repos__history2 (svn_fs_t *fs,
>+                     const char *path,
>+                     svn_repos_history_func_t history_func,
>+                     void *history_baton,
>+                     svn_repos_authz_func_t authz_read_func,
>+                     void *authz_read_baton,
>+                     svn_revnum_t start,
>+                     svn_revnum_t end,
>+                     svn_boolean_t cross_copies,
>+                     apr_pool_t *pool)
>+{
>   svn_fs_history_t *history;
>   apr_pool_t *oldpool = svn_pool_create (pool);
>   apr_pool_t *newpool = svn_pool_create (pool);
>@@ -209,6 +229,16 @@
> 
>   /* Get a revision root for END, and an initial HISTORY baton.  */
>   SVN_ERR (svn_fs_revision_root (&root, fs, end, pool));
>+
>+  if (authz_read_func)
>+    {
>+      svn_boolean_t readable;
>+      SVN_ERR (authz_read_func (&readable, root, path,
>+                                authz_read_baton, pool));
>+      if (! readable)
>+        return svn_error_create (SVN_ERR_ENTRY_NOT_FOUND, NULL, NULL);
>+    }
>+
>   SVN_ERR (svn_fs_node_history (&history, root, path, oldpool));
> 
>   /* Now, we loop over the history items, calling svn_fs_history_prev(). */
>@@ -234,6 +264,19 @@
>       if (history_rev < start)
>         break;
> 
>+      /* Is the history item readable?  If not, quit. */
>+      if (authz_read_func)
>+        {
>+          svn_boolean_t readable;
>+          svn_fs_root_t *history_root;
>+          SVN_ERR (svn_fs_revision_root (&history_root, fs,
>+                                         history_rev, newpool));
>+          SVN_ERR (authz_read_func (&readable, history_root, history_path,
>+                                    authz_read_baton, newpool));
>+          if (! readable)
>+            break;
>+        }
>+      
>       /* Call the user-provided callback function. */
>       SVN_ERR (history_func (history_baton, history_path, 
>                              history_rev, newpool));
>Index: subversion/libsvn_repos/repos.h
>===================================================================
>--- subversion/libsvn_repos/repos.h	(revision 10978)
>+++ subversion/libsvn_repos/repos.h	(working copy)
>@@ -162,8 +162,41 @@
>                                       const char *name,
>                                       svn_string_t *old_value,
>                                       apr_pool_t *pool);
>+  
>+  
>+/* Created for 1.0.7 security fix.  Only used by mod_dav_svn; allows
>+   mod_dav_svn to pass in a new authz_read_func which is used to
>+   validate paths. */
>+svn_error_t *
>+svn_repos__get_logs2 (svn_repos_t *repos,
>+                     const apr_array_header_t *paths,
>+                     svn_revnum_t start,
>+                     svn_revnum_t end,
>+                     svn_boolean_t discover_changed_paths,
>+                     svn_boolean_t strict_node_history,
>+                     svn_repos_authz_func_t authz_read_func,
>+                     void *authz_read_baton,
>+                     svn_log_message_receiver_t receiver,
>+                     void *receiver_baton,
>+                     apr_pool_t *pool);
> 
>+/* Created for 1.0.7 security fix.  Only used by mod_dav_svn; allows
>+   mod_dav_svn to pass in a new authz_read_func which is used to
>+   validate paths.  */
>+svn_error_t *
>+svn_repos__history2 (svn_fs_t *fs,
>+                    const char *path,
>+                    svn_repos_history_func_t history_func,
>+                    void *history_baton,
>+                    svn_repos_authz_func_t authz_read_func,
>+                    void *authz_read_baton,
>+                    svn_revnum_t start,
>+                    svn_revnum_t end,
>+                    svn_boolean_t cross_copies,
>+                    apr_pool_t *pool);
> 
>+
>+
> #ifdef __cplusplus
> }
> #endif /* __cplusplus */

Actions: View | Diff

Attachments on bug 60610: 23673 | 23674 | 23675