You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ph...@apache.org on 2012/07/17 12:12:20 UTC
svn commit: r1362434 - in /subversion/trunk: configure.ac
subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c
Author: philip
Date: Tue Jul 17 10:12:20 2012
New Revision: 1362434
URL: http://svn.apache.org/viewvc?rev=1362434&view=rev
Log:
Allow third party FS modules to be loaded when configured
with --enable-runtime-module-search.
* subversion/libsvn_fs/fs-loader.c
(struct fs_type_defn): Make into a linked list.
(base_defn, fsfs_defn): New list elements.
(fs_modules): Make into head of list.
(get_or_allocate_third): New.
(get_library_vtable): Iterate over linked list, try to load third
party FS module when SVN_USE_DSO is defined.
(svn_fs_print_modules): Iterate over linked list.
* configure.ac
(runtime-module-search): Tweak help string.
* subversion/include/svn_fs.h
(svn_fs_print_modules): Tweak doc string.
Modified:
subversion/trunk/configure.ac
subversion/trunk/subversion/include/svn_fs.h
subversion/trunk/subversion/libsvn_fs/fs-loader.c
Modified: subversion/trunk/configure.ac
URL: http://svn.apache.org/viewvc/subversion/trunk/configure.ac?rev=1362434&r1=1362433&r2=1362434&view=diff
==============================================================================
--- subversion/trunk/configure.ac (original)
+++ subversion/trunk/configure.ac Tue Jul 17 10:12:20 2012
@@ -1192,7 +1192,8 @@ SVN_CHECK_CTYPESGEN
dnl decide whether we want to link against the RA/FS libraries
AC_ARG_ENABLE(runtime-module-search,
AS_HELP_STRING([--enable-runtime-module-search],
- [Turn on dynamic loading of RA/FS libraries]),
+ [Turn on dynamic loading of RA/FS libraries including
+ third-party FS libraries]),
[
if test "$enableval" = "yes"; then
use_dso=yes
Modified: subversion/trunk/subversion/include/svn_fs.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_fs.h?rev=1362434&r1=1362433&r2=1362434&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_fs.h (original)
+++ subversion/trunk/subversion/include/svn_fs.h Tue Jul 17 10:12:20 2012
@@ -2405,7 +2405,8 @@ svn_fs_get_locks(svn_fs_t *fs,
/**
* Append a textual list of all available FS modules to the stringbuf
- * @a output.
+ * @a output. Third-party modules are only included if repository
+ * access has caused them to be loaded.
*
* @since New in 1.2.
*/
Modified: subversion/trunk/subversion/libsvn_fs/fs-loader.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs/fs-loader.c?rev=1362434&r1=1362433&r2=1362434&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs/fs-loader.c (original)
+++ subversion/trunk/subversion/libsvn_fs/fs-loader.c Tue Jul 17 10:12:20 2012
@@ -28,6 +28,7 @@
#include <apr_md5.h>
#include <apr_thread_mutex.h>
#include <apr_uuid.h>
+#include <apr_strings.h>
#include "svn_ctype.h"
#include "svn_types.h"
@@ -65,27 +66,37 @@ svn_mutex__t *common_pool_lock;
/* --- Utility functions for the loader --- */
-static const struct fs_type_defn {
+struct fs_type_defn {
const char *fs_type;
const char *fsap_name;
fs_init_func_t initfunc;
-} fs_modules[] = {
+ struct fs_type_defn *next;
+};
+
+static struct fs_type_defn base_defn =
{
SVN_FS_TYPE_BDB, "base",
#ifdef SVN_LIBSVN_FS_LINKS_FS_BASE
- svn_fs_base__init
+ svn_fs_base__init,
+#else
+ NULL,
#endif
- },
+ NULL
+ };
+static struct fs_type_defn fsfs_defn =
{
SVN_FS_TYPE_FSFS, "fs",
#ifdef SVN_LIBSVN_FS_LINKS_FS_FS
- svn_fs_fs__init
+ svn_fs_fs__init,
+#else
+ NULL,
#endif
- },
+ &base_defn
+ };
+
+static struct fs_type_defn *fs_modules = &fsfs_defn;
- { NULL }
-};
static svn_error_t *
load_module(fs_init_func_t *initfunc, const char *name, apr_pool_t *pool)
@@ -173,21 +184,66 @@ get_library_vtable_direct(fs_library_vta
return SVN_NO_ERROR;
}
+#if defined(SVN_USE_DSO) && APR_HAS_DSO
+/* Return *FST for the third party FS_TYPE */
+static svn_error_t *
+get_or_allocate_third(struct fs_type_defn **fst,
+ const char *fs_type)
+{
+ fst = &fs_modules;
+
+ while (*fst)
+ fst = &(*fst)->next;
+
+ *fst = apr_palloc(common_pool, sizeof(struct fs_type_defn));
+ (*fst)->fs_type = apr_pstrdup(common_pool, fs_type);
+ (*fst)->fsap_name = (*fst)->fs_type;
+ (*fst)->initfunc = NULL;
+ (*fst)->next = NULL;
+
+ return SVN_NO_ERROR;
+}
+#endif
+
/* Fetch a library vtable by FS type. */
static svn_error_t *
get_library_vtable(fs_library_vtable_t **vtable, const char *fs_type,
apr_pool_t *pool)
{
- const struct fs_type_defn *fst;
+ struct fs_type_defn **fst = &fs_modules;
- for (fst = fs_modules; fst->fs_type; fst++)
+ while (*fst)
{
- if (strcmp(fs_type, fst->fs_type) == 0)
- return get_library_vtable_direct(vtable, fst, pool);
+ if (strcmp(fs_type, (*fst)->fs_type) == 0)
+ break;
+ (*fst) = (*fst)->next;
}
- return svn_error_createf(SVN_ERR_FS_UNKNOWN_FS_TYPE, NULL,
- _("Unknown FS type '%s'"), fs_type);
+#if defined(SVN_USE_DSO) && APR_HAS_DSO
+ /* Third party FS modules that are unknown at compile time.
+
+ A third party FS is identified by the file fs-type containing a
+ third party name, say "foo". The loader will load the DSO with
+ the name "libsvn_fs_foo" and use the entry point with the name
+ "svn_fs_foo__init".
+
+ Note: the BDB and FSFS modules don't follow this naming scheme
+ and this allows them to be used to test the third party loader.
+ Change the content of fs-type to "base" in a BDB filesystem or to
+ "fs" in an FSFS filesystem and they will be loaded as third party
+ modules. */
+ if (! *fst)
+ {
+ if (!common_pool) /* Best-effort init, see get_library_vtable_direct. */
+ SVN_ERR(svn_fs_initialize(NULL));
+ SVN_MUTEX__WITH_LOCK(common_pool_lock,
+ get_or_allocate_third(fst, fs_type));
+ }
+#endif
+ if (!*fst)
+ return svn_error_createf(SVN_ERR_FS_UNKNOWN_FS_TYPE, NULL,
+ _("Unknown FS type '%s'"), fs_type);
+ return get_library_vtable_direct(vtable, *fst, pool);
}
svn_error_t *
@@ -1414,11 +1470,11 @@ svn_error_t *
svn_fs_print_modules(svn_stringbuf_t *output,
apr_pool_t *pool)
{
- const struct fs_type_defn *defn;
+ const struct fs_type_defn *defn = fs_modules;
fs_library_vtable_t *vtable;
apr_pool_t *iterpool = svn_pool_create(pool);
- for (defn = fs_modules; defn->fs_type != NULL; ++defn)
+ while (defn)
{
char *line;
svn_error_t *err;
@@ -1431,6 +1487,7 @@ svn_fs_print_modules(svn_stringbuf_t *ou
if (err->apr_err == SVN_ERR_FS_UNKNOWN_FS_TYPE)
{
svn_error_clear(err);
+ defn = defn->next;
continue;
}
else
@@ -1440,6 +1497,7 @@ svn_fs_print_modules(svn_stringbuf_t *ou
line = apr_psprintf(iterpool, "* fs_%s : %s\n",
defn->fsap_name, vtable->get_description());
svn_stringbuf_appendcstr(output, line);
+ defn = defn->next;
}
svn_pool_destroy(iterpool);
Re: svn commit: r1362434 - in /subversion/trunk: configure.ac
subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c
Posted by Stefan Sperling <st...@elego.de>.
On Wed, Jul 18, 2012 at 11:45:52AM +0100, Philip Martin wrote:
> Stefan Sperling <st...@elego.de> writes:
>
> > On Tue, Jul 17, 2012 at 11:14:10AM +0100, Philip Martin wrote:
> >> Disclaimer: at WANdisco we plan to use this feature.
> >
> > Will generally useful FS features developed by Wandisco be fed
> > back to the Subversion project? Or are there plans to develop
> > a competing proprietary backend that only works with Wandisco
> > products and offers generally useful FS features?
>
> WANdisco is committed to doing FS development as part of the Subversion
> project. We are interested in all the usual things: ways to store move
> and merge information, different ways to store directories, forward
> history, etc. We have no plans to develop a proprietary FS backend.
Great! This is what I had hoped and expected.
Thanks for clarifying, Philip!
Re: svn commit: r1362434 - in /subversion/trunk: configure.ac subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c
Posted by Philip Martin <ph...@wandisco.com>.
Stefan Sperling <st...@elego.de> writes:
> On Tue, Jul 17, 2012 at 11:14:10AM +0100, Philip Martin wrote:
>> Disclaimer: at WANdisco we plan to use this feature.
>
> Will generally useful FS features developed by Wandisco be fed
> back to the Subversion project? Or are there plans to develop
> a competing proprietary backend that only works with Wandisco
> products and offers generally useful FS features?
WANdisco is committed to doing FS development as part of the Subversion
project. We are interested in all the usual things: ways to store move
and merge information, different ways to store directories, forward
history, etc. We have no plans to develop a proprietary FS backend.
--
Certified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: svn commit: r1362434 - in /subversion/trunk: configure.ac
subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c
Posted by Stefan Sperling <st...@elego.de>.
On Tue, Jul 17, 2012 at 11:14:10AM +0100, Philip Martin wrote:
> Disclaimer: at WANdisco we plan to use this feature.
Will generally useful FS features developed by Wandisco be fed
back to the Subversion project? Or are there plans to develop
a competing proprietary backend that only works with Wandisco
products and offers generally useful FS features?
I don't mind your change. I'm just curious about what the
Subversion project itself can expect from getting out of this.
RE: svn commit: r1362434 - in /subversion/trunk: configure.ac subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: MARTIN PHILIP [mailto:codematters@ntlworld.com] On Behalf Of
> Philip Martin
> Sent: dinsdag 17 juli 2012 16:07
> To: Ivan Zhakov
> Cc: dev@subversion.apache.org
> Subject: Re: svn commit: r1362434 - in /subversion/trunk: configure.ac
> subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c
>
> Ivan Zhakov <iv...@visualsvn.com> writes:
>
> >> If the victim has a world writeable location in the search path the
attacker
> >> could replace any DSO.
> >
> > The attacker cannot replace any DSO, because current directory has
> > lower priority than other locations. So in typical scenarios user is
> > not vulnerable, because DSO is found in other location.
> >
> > So for security reason we should load DSO using absolute path at least
> > on Windows.
>
> I'm not sure how to get the absolute path.
>
> Does anyone build on Windows with SVN_USE_DSO set?
I don't think so, as we don't provide a way to enable this via gen-make.py.
We use apr to load libraries and APR loads libraries with the
LOAD_WITH_ALTERED_SEARCH_PATH flag, so Windows starts by checking the
directory in which the current executable is located. So for our own
executables this should be a safe default.
If some other application is using our libraries they should set a good
default policy via SetDllDirectory.
(We could call SetDllDirectory from our own executables, but we should make
sure we don't do it by default for our api users as this might break their
current security model)
For some specific case such as JavaHL (which would run in the java runtime,
where the application dir is something out of our control) we may improve
the situation a bit by providing a full path instead of just a dll name.
But then: Java has already loaded our javahl dll via an unsecure route, so
the front door is already open. No need to lock down the windows :)
(Note: The police recommends to lock the Windows anyway, to make sure
burglars don't feel they have a safe way out. But I don't think this part
applies to software security)
Bert
Re: svn commit: r1362434 - in /subversion/trunk: configure.ac subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c
Posted by Philip Martin <ph...@wandisco.com>.
Ivan Zhakov <iv...@visualsvn.com> writes:
>> If the victim has a world writeable location in the search path the attacker
>> could replace any DSO.
>
> The attacker cannot replace any DSO, because current directory has
> lower priority than other locations. So in typical scenarios user is
> not vulnerable, because DSO is found in other location.
>
> So for security reason we should load DSO using absolute path at least
> on Windows.
I'm not sure how to get the absolute path.
Does anyone build on Windows with SVN_USE_DSO set?
--
Cerified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: svn commit: r1362434 - in /subversion/trunk: configure.ac
subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Tue, Jul 17, 2012 at 5:21 PM, Philip Martin
<ph...@wandisco.com> wrote:
> Ivan Zhakov <iv...@visualsvn.com> writes:
>
>> On Tue, Jul 17, 2012 at 2:14 PM, Philip Martin
>> <ph...@wandisco.com> wrote:
>>>
>>> philip@apache.org writes:
>>>
>>> > Author: philip
>>> > Date: Tue Jul 17 10:12:20 2012
>>> > New Revision: 1362434
>>> >
>>> > URL: http://svn.apache.org/viewvc?rev=1362434&view=rev
>>> > Log:
>>> > Allow third party FS modules to be loaded when configured
>>> > with --enable-runtime-module-search.
>>>
>>> Until now anyone wanting to write an FS module had a problem: only
>>> modules known to the Subversion project could be loaded and used.
>>> That means that anyone wanting to write their own module had to get a
>>> patch for their module name into the core Subversion code. Or write
>>> their own loader/server.
>>>
>>> I don't think there is any security risk here: I need to write to the
>>> repository fs-type file to get a malicious module to load and if I can
>>> do that it would be far easier to use one of the hook scripts.
>>>
>> It still possible security issue here. Just image that repository is
>> stored on network share or something. Someone tweaked fs-type and put
>> fake .dll in repository folder. Then another user accesses this
>> repository and gets this dll loaded on his behalf!
>
> To get a DSO loaded it has to go into the library search path.
Problem that on Windows current directory is also library search path [1].
> If the victim has a world writeable location in the search path the attacker
> could replace any DSO.
The attacker cannot replace any DSO, because current directory has
lower priority than other locations. So in typical scenarios user is
not vulnerable, because DSO is found in other location.
So for security reason we should load DSO using absolute path at least
on Windows.
[1] http://msdn.microsoft.com/en-us/library/windows/desktop/ms682586%28v=vs.85%29.aspx#standard_search_order_for_desktop_applications
>> To prevent such issues we should valdiate fs-type to be only file name
>> with only alphanumeric characters. No dots, spaces or slashes. We also
>> should only load DSO module from directory where Subversion installed
>> for better protection.
> That's a good idea. r1362480.
Great!
--
Ivan Zhakov
Re: svn commit: r1362434 - in /subversion/trunk: configure.ac subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c
Posted by Philip Martin <ph...@wandisco.com>.
Ivan Zhakov <iv...@visualsvn.com> writes:
> On Tue, Jul 17, 2012 at 2:14 PM, Philip Martin
> <ph...@wandisco.com> wrote:
>>
>> philip@apache.org writes:
>>
>> > Author: philip
>> > Date: Tue Jul 17 10:12:20 2012
>> > New Revision: 1362434
>> >
>> > URL: http://svn.apache.org/viewvc?rev=1362434&view=rev
>> > Log:
>> > Allow third party FS modules to be loaded when configured
>> > with --enable-runtime-module-search.
>>
>> Until now anyone wanting to write an FS module had a problem: only
>> modules known to the Subversion project could be loaded and used.
>> That means that anyone wanting to write their own module had to get a
>> patch for their module name into the core Subversion code. Or write
>> their own loader/server.
>>
>> I don't think there is any security risk here: I need to write to the
>> repository fs-type file to get a malicious module to load and if I can
>> do that it would be far easier to use one of the hook scripts.
>>
> It still possible security issue here. Just image that repository is
> stored on network share or something. Someone tweaked fs-type and put
> fake .dll in repository folder. Then another user accesses this
> repository and gets this dll loaded on his behalf!
To get a DSO loaded it has to go into the library search path. If the
victim has a world writeable location in the search path the attacker
could replace any DSO.
> To prevent such issues we should valdiate fs-type to be only file name
> with only alphanumeric characters. No dots, spaces or slashes. We also
> should only load DSO module from directory where Subversion installed
> for better protection.
That's a good idea. r1362480.
--
Cerified & Supported Apache Subversion Downloads:
http://www.wandisco.com/subversion/download
Re: svn commit: r1362434 - in /subversion/trunk: configure.ac
subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c
Posted by Ivan Zhakov <iv...@visualsvn.com>.
On Tue, Jul 17, 2012 at 2:14 PM, Philip Martin
<ph...@wandisco.com> wrote:
>
> philip@apache.org writes:
>
> > Author: philip
> > Date: Tue Jul 17 10:12:20 2012
> > New Revision: 1362434
> >
> > URL: http://svn.apache.org/viewvc?rev=1362434&view=rev
> > Log:
> > Allow third party FS modules to be loaded when configured
> > with --enable-runtime-module-search.
>
> Until now anyone wanting to write an FS module had a problem: only
> modules known to the Subversion project could be loaded and used.
> That means that anyone wanting to write their own module had to get a
> patch for their module name into the core Subversion code. Or write
> their own loader/server.
>
> I don't think there is any security risk here: I need to write to the
> repository fs-type file to get a malicious module to load and if I can
> do that it would be far easier to use one of the hook scripts.
>
It still possible security issue here. Just image that repository is
stored on network share or something. Someone tweaked fs-type and put
fake .dll in repository folder. Then another user accesses this
repository and gets this dll loaded on his behalf!
To prevent such issues we should valdiate fs-type to be only file name
with only alphanumeric characters. No dots, spaces or slashes. We also
should only load DSO module from directory where Subversion installed
for better protection.
--
Ivan Zhakov
Re: svn commit: r1362434 - in /subversion/trunk: configure.ac subversion/include/svn_fs.h subversion/libsvn_fs/fs-loader.c
Posted by Philip Martin <ph...@wandisco.com>.
philip@apache.org writes:
> Author: philip
> Date: Tue Jul 17 10:12:20 2012
> New Revision: 1362434
>
> URL: http://svn.apache.org/viewvc?rev=1362434&view=rev
> Log:
> Allow third party FS modules to be loaded when configured
> with --enable-runtime-module-search.
Until now anyone wanting to write an FS module had a problem: only
modules known to the Subversion project could be loaded and used.
That means that anyone wanting to write their own module had to get a
patch for their module name into the core Subversion code. Or write
their own loader/server.
I don't think there is any security risk here: I need to write to the
repository fs-type file to get a malicious module to load and if I can
do that it would be far easier to use one of the hook scripts.
We could do the same for RA modules but I have no plans to implement
it.
Disclaimer: at WANdisco we plan to use this feature.
--
Philip