You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Jani Averbach <ja...@jaa.iki.fi> on 2004/09/18 18:12:01 UTC
[PATCH]: mod_authz_svn returns HTTP 500 instead of HTTP 403
Hello,
I didn't like to commit this just by myself because this is a security
related change. However, It should be ok, but please review. Thanks.
BR, Jani
Log:
With combination mod_dav_svn + mod_authz_svn + SVNParentPath you get
'500 Internal Server Error', when mod_dav_svn's dav_svn_split_uri
originally returned '403 Forbidden'. This happens when you access the
root of SVNParentPath. Replay forward dav_svn_split_uri's return
value iff it won't clash with our access releated return codes.
* subversion/mod_authz_svn/mod_authz_svn.c
(req_check_access): if dav_svn_split_uri's error code didn't clash with
our access related values, replay it forward.
Index: subversion/mod_authz_svn/mod_authz_svn.c
===================================================================
--- subversion/mod_authz_svn/mod_authz_svn.c (revision 11042)
+++ subversion/mod_authz_svn/mod_authz_svn.c (working copy)
@@ -366,7 +366,9 @@
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"%s [%d, #%d]",
dav_err->desc, dav_err->status, dav_err->error_id);
- return HTTP_INTERNAL_SERVER_ERROR;
+ /* Ensure that we never allow access by dav_err->status */
+ return (dav_err->status != OK && dav_err->status != DECLINED) ?
+ dav_err->status : HTTP_INTERNAL_SERVER_ERROR;
}
/* Ignore the URI passed to MERGE, like mod_dav_svn does.
@@ -417,7 +419,9 @@
ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
"%s [%d, #%d]",
dav_err->desc, dav_err->status, dav_err->error_id);
- return HTTP_INTERNAL_SERVER_ERROR;
+ /* Ensure that we never allow access by dav_err->status */
+ return (dav_err->status != OK && dav_err->status != DECLINED) ?
+ dav_err->status : HTTP_INTERNAL_SERVER_ERROR;
}
if (dest_repos_path)
--
Jani Averbach
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: mod_authz_svn returns HTTP 500 instead of HTTP 403
Posted by Jani Averbach <ja...@jaa.iki.fi>.
On 2004-09-20 07:49-0500, kfogel@collab.net wrote:
> Jani Averbach <ja...@jaa.iki.fi> writes:
> > Log:
> >
> > With combination mod_dav_svn + mod_authz_svn + SVNParentPath you get
> > '500 Internal Server Error', when mod_dav_svn's dav_svn_split_uri
> > originally returned '403 Forbidden'. This happens when you access the
> > root of SVNParentPath. Replay forward dav_svn_split_uri's return
> > value iff it won't clash with our access releated return codes.
>
> At first I thought the former, but on reading the patch, I now think
> the latter.
The latter, how things will be after change. At the moment you will
get 'Internal server error'.
>
> What exactly was the security issue?
The security related change is that 'req_check_access' will return OK
or DECLINED or some error code to upper caller 'access_checker' which
will decide by these return values if access is granted or not.
Before my modifications, this return code space was tightly controlled
by 'req_check_access'. But my modifications will broaden it to
include also dav_svn_split uri's return codes space. For this reason
we have to take extra step to check that we won't ever give access by
dav_svn_split in (erroneous) case when dav_svn_split returned error,
but error code was OK.
BR, Jani
--
Jani Averbach
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: mod_authz_svn returns HTTP 500 instead of HTTP 403
Posted by kf...@collab.net.
Jani Averbach <ja...@jaa.iki.fi> writes:
> Log:
>
> With combination mod_dav_svn + mod_authz_svn + SVNParentPath you get
> '500 Internal Server Error', when mod_dav_svn's dav_svn_split_uri
> originally returned '403 Forbidden'. This happens when you access the
> root of SVNParentPath. Replay forward dav_svn_split_uri's return
> value iff it won't clash with our access releated return codes.
The log message is a little unclear on what's changing. Is the first
sentence describing the way things are currently, or the way things
will be after this change? At first I thought the former, but on
reading the patch, I now think the latter.
What exactly was the security issue? Is it that someone could
differentiate between the URL that is the parent of all repositories
and the URLs that are inside repositories, and then they could (maybe)
use that to guess at other repository names underneath the root or
something?
I don't have any problems with the patch, I'm just trying to
understand it better.
Thanks,
-Karl
> * subversion/mod_authz_svn/mod_authz_svn.c
> (req_check_access): if dav_svn_split_uri's error code didn't clash with
> our access related values, replay it forward.
>
> Index: subversion/mod_authz_svn/mod_authz_svn.c
> ===================================================================
> --- subversion/mod_authz_svn/mod_authz_svn.c (revision 11042)
> +++ subversion/mod_authz_svn/mod_authz_svn.c (working copy)
> @@ -366,7 +366,9 @@
> ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
> "%s [%d, #%d]",
> dav_err->desc, dav_err->status, dav_err->error_id);
> - return HTTP_INTERNAL_SERVER_ERROR;
> + /* Ensure that we never allow access by dav_err->status */
> + return (dav_err->status != OK && dav_err->status != DECLINED) ?
> + dav_err->status : HTTP_INTERNAL_SERVER_ERROR;
> }
>
> /* Ignore the URI passed to MERGE, like mod_dav_svn does.
> @@ -417,7 +419,9 @@
> ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
> "%s [%d, #%d]",
> dav_err->desc, dav_err->status, dav_err->error_id);
> - return HTTP_INTERNAL_SERVER_ERROR;
> + /* Ensure that we never allow access by dav_err->status */
> + return (dav_err->status != OK && dav_err->status != DECLINED) ?
> + dav_err->status : HTTP_INTERNAL_SERVER_ERROR;
> }
>
> if (dest_repos_path)
>
> --
> Jani Averbach
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org