You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by David Kimdon <dw...@debian.org> on 2002/10/03 04:49:49 UTC
[PATCH]: don't store auth info in world-readable files
Hi,
The included patch makes the .svn/auth/ files only readable by the current
user. I was motivated to create this patch by a bug report that can be found
at http://bugs.debian.org/162953 . In the spirit of openness I could have left
the username file world readable but life is simpler if everything in the auth
dir is only u+r. The patch passed 'make check'.
-David
* subversion/libsvn_wc/adm_files.c : Make auth area only readable by user.
(open_adm_file) : Add parameter 'protection' to indicate the permissions
a created file should have. This parameter is passed to svn_io_file_open().
(svn_wc__open_adm_file, svn_wc__open_empty_file, svn_wc__open_text_base,
svn_wc__open_props) : pass APR_OS_DEFAULT as protection to open_adm_file()
(svn_wc__open_auth_file) : pass APR_UREAD to open_adm_file()
Index: subversion/libsvn_wc/adm_files.c
===================================================================
--- subversion/libsvn_wc/adm_files.c
+++ subversion/libsvn_wc/adm_files.c 2002-10-02 20:57:22.000000000 -0700
@@ -504,6 +504,7 @@
open_adm_file (apr_file_t **handle,
const char *path,
const char *extension,
+ apr_fileperms_t protection,
apr_int32_t flags,
apr_pool_t *pool,
...)
@@ -545,7 +546,7 @@
va_end (ap);
}
- err = svn_io_file_open (handle, path, flags, APR_OS_DEFAULT, pool);
+ err = svn_io_file_open (handle, path, flags, protection, pool);
if (err)
{
/* Oddly enough, APR will set *HANDLE even if the open failed.
@@ -618,7 +619,7 @@
apr_int32_t flags,
apr_pool_t *pool)
{
- return open_adm_file (handle, path, NULL, flags, pool, fname, NULL);
+ return open_adm_file (handle, path, NULL, APR_OS_DEFAULT, flags, pool, fname, NULL);
}
@@ -666,8 +667,8 @@
apr_pool_t *pool)
{
const char *parent_path = svn_path_remove_component_nts (path, pool);
- return open_adm_file (handle, parent_path, NULL, APR_READ, pool,
- SVN_WC__ADM_EMPTY_FILE, NULL);
+ return open_adm_file (handle, parent_path, NULL, APR_OS_DEFAULT, APR_READ,
+ pool, SVN_WC__ADM_EMPTY_FILE, NULL);
}
@@ -690,8 +691,8 @@
{
const char *parent_path, *base_name;
svn_path_split_nts (path, &parent_path, &base_name, pool);
- return open_adm_file (handle, parent_path, SVN_WC__BASE_EXT, flags, pool,
- SVN_WC__ADM_TEXT_BASE, base_name, NULL);
+ return open_adm_file (handle, parent_path, SVN_WC__BASE_EXT, APR_OS_DEFAULT,
+ flags, pool, SVN_WC__ADM_TEXT_BASE, base_name, NULL);
}
@@ -715,7 +716,7 @@
apr_int32_t flags,
apr_pool_t *pool)
{
- return open_adm_file (handle, path, NULL, flags, pool,
+ return open_adm_file (handle, path, NULL, APR_UREAD, flags, pool,
SVN_WC__ADM_AUTH_DIR, auth_filename, NULL);
}
@@ -780,38 +781,38 @@
else if (base)
{
if (kind == svn_node_dir)
- return open_adm_file (handle, parent_dir, NULL, flags, pool,
- SVN_WC__ADM_DIR_PROP_BASE, NULL);
+ return open_adm_file (handle, parent_dir, NULL, APR_OS_DEFAULT, flags,
+ pool, SVN_WC__ADM_DIR_PROP_BASE, NULL);
else
- return open_adm_file (handle, parent_dir, SVN_WC__BASE_EXT, flags,
- pool, SVN_WC__ADM_PROP_BASE, base_name,
- NULL);
+ return open_adm_file (handle, parent_dir, SVN_WC__BASE_EXT,
+ APR_OS_DEFAULT, flags, pool,
+ SVN_WC__ADM_PROP_BASE, base_name, NULL);
}
else if (wcprops)
{
if (kind == svn_node_dir)
- return open_adm_file (handle, parent_dir, NULL, flags, pool,
- SVN_WC__ADM_DIR_WCPROPS, NULL);
+ return open_adm_file (handle, parent_dir, NULL, APR_OS_DEFAULT, flags,
+ pool, SVN_WC__ADM_DIR_WCPROPS, NULL);
else
{
return open_adm_file
(handle, parent_dir,
((wc_format_version <= SVN_WC__OLD_PROPNAMES_VERSION) ?
- NULL : SVN_WC__WORK_EXT),
+ NULL : SVN_WC__WORK_EXT), APR_OS_DEFAULT,
flags, pool, SVN_WC__ADM_WCPROPS, base_name, NULL);
}
}
else /* plain old property file */
{
if (kind == svn_node_dir)
- return open_adm_file (handle, parent_dir, NULL, flags, pool,
- SVN_WC__ADM_DIR_PROPS, NULL);
+ return open_adm_file (handle, parent_dir, NULL, APR_OS_DEFAULT, flags,
+ pool, SVN_WC__ADM_DIR_PROPS, NULL);
else
{
return open_adm_file
(handle, parent_dir,
((wc_format_version <= SVN_WC__OLD_PROPNAMES_VERSION) ?
- NULL : SVN_WC__WORK_EXT),
+ NULL : SVN_WC__WORK_EXT), APR_OS_DEFAULT,
flags, pool, SVN_WC__ADM_PROPS, base_name, NULL);
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: don't store auth info in world-readable files
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Karl Fogel wrote:
> Garrett Rooney <ro...@electricjellyfish.net> writes:
>
>>personally, i'd be +1 on committing this, since i'm tired of seeing
>>the question posted on the list, and it doesn't actually seem to harm
>>anything.
>
>
> I wrote:
>
>>Let's see if that really happens in practice. For example, the Debian
>>bug report http://bugs.debian.org/162953 got corrected almost as soon
>>as it was filed.
>
>
> Yeah, on further consideration I think you're right. If it really
> does no harm, let's just commit the patch and be done with it. (Are
> you planning to review/test/commit? If not, let me know.)
i can test and commit it tonight if nobody else does it first.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: don't store auth info in world-readable files
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Garrett Rooney <ro...@electricjellyfish.net> writes:
> personally, i'd be +1 on committing this, since i'm tired of seeing
> the question posted on the list, and it doesn't actually seem to harm
> anything.
I wrote:
> Let's see if that really happens in practice. For example, the Debian
> bug report http://bugs.debian.org/162953 got corrected almost as soon
> as it was filed.
Yeah, on further consideration I think you're right. If it really
does no harm, let's just commit the patch and be done with it. (Are
you planning to review/test/commit? If not, let me know.)
-K
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: don't store auth info in world-readable files
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
David Mankin wrote:
>
> On Thursday, October 3, 2002, at 07:02 AM, Karl Fogel wrote:
>
>> David Kimdon <dw...@debian.org> writes:
>>
>>>> This gets mentioned every now and then. .svn/auth/* are world-readable,
>>>> but .svn/auth is not world-traversable. So I don't see any real
>>>> security
>>>> problem here.
>>>
>>>
>>> hmm, good point, sounds like this patch can be safely ignored.
>>
>>
>> I'm beginning to think a new patch to the FAQ is called for instead...
>> :-)
>>
>
> Real security problem or not, isn't a patch to fix a perceived security
> problem a good idea too? If it keeps coming up, and there's no downside
> to fixing it, I can't see why the patch shouldn't be included. I don't
> think we want security related bug reports about SVN in various OS
> trackers; it might make it harder to convince our respective bosses to
> let us switch our SCM to SVN.
personally, i'd be +1 on committing this, since i'm tired of seeing the
question posted on the list, and it doesn't actually seem to harm anything.
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: don't store auth info in world-readable files
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
David Mankin <ma...@ants.com> writes:
> Real security problem or not, isn't a patch to fix a perceived
> security problem a good idea too? If it keeps coming up, and there's
> no downside to fixing it, I can't see why the patch shouldn't be
> included. I don't think we want security related bug reports about SVN
> in various OS trackers; it might make it harder to convince our
> respective bosses to let us switch our SCM to SVN.
Let's see if that really happens in practice. For example, the Debian
bug report http://bugs.debian.org/162953 got corrected almost as soon
as it was filed.
-Karl
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: don't store auth info in world-readable files
Posted by David Mankin <ma...@ants.com>.
On Thursday, October 3, 2002, at 07:02 AM, Karl Fogel wrote:
> David Kimdon <dw...@debian.org> writes:
>>> This gets mentioned every now and then. .svn/auth/* are
>>> world-readable,
>>> but .svn/auth is not world-traversable. So I don't see any real
>>> security
>>> problem here.
>>
>> hmm, good point, sounds like this patch can be safely ignored.
>
> I'm beginning to think a new patch to the FAQ is called for instead...
> :-)
>
Real security problem or not, isn't a patch to fix a perceived security
problem a good idea too? If it keeps coming up, and there's no
downside to fixing it, I can't see why the patch shouldn't be included.
I don't think we want security related bug reports about SVN in
various OS trackers; it might make it harder to convince our respective
bosses to let us switch our SCM to SVN.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: don't store auth info in world-readable files
Posted by Karl Fogel <kf...@newton.ch.collab.net>.
David Kimdon <dw...@debian.org> writes:
> > This gets mentioned every now and then. .svn/auth/* are world-readable,
> > but .svn/auth is not world-traversable. So I don't see any real security
> > problem here.
>
> hmm, good point, sounds like this patch can be safely ignored.
I'm beginning to think a new patch to the FAQ is called for instead... :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: don't store auth info in world-readable files
Posted by David Kimdon <dw...@debian.org>.
Wed, Oct 02, 2002 at 11:58:25PM -0500 wrote:
>
> This gets mentioned every now and then. .svn/auth/* are world-readable,
> but .svn/auth is not world-traversable. So I don't see any real security
> problem here.
hmm, good point, sounds like this patch can be safely ignored.
-David
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH]: don't store auth info in world-readable files
Posted by Scott Lamb <sl...@slamb.org>.
David Kimdon wrote:
> Hi,
>
> The included patch makes the .svn/auth/ files only readable by the current
> user. I was motivated to create this patch by a bug report that can be found
> at http://bugs.debian.org/162953 . In the spirit of openness I could have left
> the username file world readable but life is simpler if everything in the auth
> dir is only u+r. The patch passed 'make check'.
This gets mentioned every now and then. .svn/auth/* are world-readable,
but .svn/auth is not world-traversable. So I don't see any real security
problem here.
Scott
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org