You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Marc Singer <el...@buici.com> on 2003/09/05 02:14:49 UTC
Could svn_backend check permissions?
1) Obviously, a permissions check, if there is one, belongs in the
filesystem driver.
2) The cost of the check is minimal. Stat'ing a dozen files is cheap
because the OS caches the data. Subsequent access by BDB will open
the same files using the cached data.
3) The expense of not performing it is kinda high when we see how
often permissions problems plague users. If Subversion isn't easy
to use then it won't be used. Admonishing people for failing to
read the documentation is a poor sales pitch.
4) AFAICT, there is no fix to be made in svnadmin because this isn't
really a setup issue. People change the way that they use
their repositories. Sometimes that new behavior, while valid in
concept, isn't well executed. It would be *way* helpful if the
program let the user know more explicitly what the problem is.
The HACKING guide suggests to me that the project bears the value of
sturdy design:
Input validation is the act of defining legal input and rejecting
everything else. The code must perform input validation on all
untrusted
input.
I asked the question to see if this has come up before. It appears
that it hasn't.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Marc Singer <el...@buici.com>.
On Fri, Sep 05, 2003 at 11:42:59PM +0200, Tobias Ringstr?m wrote:
> e.huelsmann@gmx.net wrote:
> >Tobias,
> >
> >I'm a nobody too. We were definitely not discussion messing with
> >permissions. We were discussing where and how to throw an understandable
> >error message
> >or to leave it to berkeley to display any error (not permission denied
> >btw).
> >
> >Greg Hudson, Mark and I advocated checking conditions so as to be sure that
> >if any db errors occur, that they are not due to inaccessability of the
> >database by berkeley. John said "there's nothing wrong with BDB and
> >nothing with
> >our use of it, so we should do nothing. It's the users fault". (Ofcourse
> >this
> >is what I understood their comments to be.)
> >
> >So I think we agree to the solution we are currently persuing?
>
> OK, I've tried to read up on the whole conversation. I'm tired and
> stupid so I may have misuderstood it all, but here goes nothing:
>
> The current code will print a "permission denied" error message if BDB
> fails because of a permission problem. When this happens, the admin must
> fix the problem by hand. There will not be any confusion because the
> error message is clear. It it not clear enough, it would be a fiveliner
> to add an URL to the FAQ in the error message.
>
> The holy grail would of course be to prevent a user with a bad umask
> (for example) to wedge the repos for all the other users. Doing so would
> definately involve messing with permissions, and it would be hard to get
> it right, IMHO.
>
> Is it possible that you are trying to solve an old problem? Just a short
> time ago neither ra_dav, ra_svn nor ra_local did manage to present a
> "permission denied" error message to the user, but now they do.
>
> So I'm just wondering if this is still a problem? Do you have an example
> where you get a different error than "permission denied" when it really
> is a perssion problem?
>
> /Tobias, nobody, tired and stupid :-)
As far as I can tell, there isn't a way we can perform an a priori
check on permissions and determine whether or a given user's access
will trash the repository.
What has been suggested is to check that the umask is either 0002 or
that the owner of the directory is the same as the user who is about
to create files in that directory.
However, it is valid for svn.svn (group writable) to own a directory
and for bob.svn to write files to the repository with a 0022 mask as
long bob is the only user using the repository.
Perhaps you are right that the existing 'permission denied' message is
sufficient. I wonder, though, that users are still confused that it
means that their repository permissions are inconsistent.
The tack I've been angling is to check that the user accessing the
repository will have sufficient priviledge to access it. There is
another angle, though, which is a bit more aggressive. We could check
that a file written by the current user would be accessible to others
who have written log files. This can be done without violating the
BDB abstraction since we can enumerate log files using a BDB call.
Using this kind of check, we can block some types of repository access
that would corrupt the DB. It wouldn't be foolproof, but it would be
a) inexpensive, and b) never a false negative...I think.
What about that?
Cheers.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Marc Singer <el...@buici.com>.
> The rule I'm suggesting is: if you are relying on group write access to
> open the DB files themselves, your umask must not contain the 020 or 040
> bits. I didn't say anything about the owner of the directory.
>
> Certainly, it's possible to contrive a case where I violate this rule
> and things continue to work; it just requires that nobody else tries to
> use the repository but me, ever again. But there's no conceivable
> reason why I'd want to set up a repository that way.
For you, perhaps, but others may want to. The rule I suggested,
maintaining permission compatibility with previous log files, excludes
no case that works in subversion now.
Cheers.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Tobias Ringström <to...@ringstrom.mine.nu>.
Greg Hudson wrote:
> On Fri, 2003-09-05 at 17:42, Tobias Ringström wrote:
>
>>The holy grail would of course be to prevent a user with a bad umask
>>(for example) to wedge the repos for all the other users. Doing so would
>>definately involve messing with permissions, and it would be hard to get
>>it right, IMHO.
>
>>From BDB's point of view, it's not very hard: when a new log file is
> created, chmod() it to match the mode of the database it belongs to.
> There's never any good reason for a log file to have a different mode
> than the database itself. Keith Bostic has agreed to look into making
> this hapen in a future version of BDB, though it's not clear whether
> that will be 4.2.x (due out later this year) or some indefinite future
> version.
Nice. The BDB layer feels like the right place to fix this.
I wonder if it will work with systems using ACLs though (e.g. Windows),
but maybe those systems do not have a problem as it is. Even if it only
improves systems with the (limited) traditional UNIX filesystem security
(uid,gid,umask), it would most likely be worth the effort.
/Tobias
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Greg Hudson <gh...@MIT.EDU>.
On Fri, 2003-09-05 at 17:42, Tobias Ringström wrote:
> The holy grail would of course be to prevent a user with a bad umask
> (for example) to wedge the repos for all the other users. Doing so would
> definately involve messing with permissions, and it would be hard to get
> it right, IMHO.
From BDB's point of view, it's not very hard: when a new log file is
created, chmod() it to match the mode of the database it belongs to.
There's never any good reason for a log file to have a different mode
than the database itself. Keith Bostic has agreed to look into making
this hapen in a future version of BDB, though it's not clear whether
that will be 4.2.x (due out later this year) or some indefinite future
version.
> Is it possible that you are trying to solve an old problem? Just a short
> time ago neither ra_dav, ra_svn nor ra_local did manage to present a
> "permission denied" error message to the user, but now they do.
I believe if the DB is writable to you but a log file is not, you will
still get some scary BDB error message about needing to run recovery.
Marc Singer wrote:
> What has been suggested is to check that the umask is either 0002 or
> that the owner of the directory is the same as the user who is about
> to create files in that directory.
> However, it is valid for svn.svn (group writable) to own a directory
> and for bob.svn to write files to the repository with a 0022 mask as
> long bob is the only user using the repository.
The rule I'm suggesting is: if you are relying on group write access to
open the DB files themselves, your umask must not contain the 020 or 040
bits. I didn't say anything about the owner of the directory.
Certainly, it's possible to contrive a case where I violate this rule
and things continue to work; it just requires that nobody else tries to
use the repository but me, ever again. But there's no conceivable
reason why I'd want to set up a repository that way.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Tobias Ringström <to...@ringstrom.mine.nu>.
e.huelsmann@gmx.net wrote:
> Tobias,
>
> I'm a nobody too. We were definitely not discussion messing with
> permissions. We were discussing where and how to throw an understandable error message
> or to leave it to berkeley to display any error (not permission denied btw).
>
> Greg Hudson, Mark and I advocated checking conditions so as to be sure that
> if any db errors occur, that they are not due to inaccessability of the
> database by berkeley. John said "there's nothing wrong with BDB and nothing with
> our use of it, so we should do nothing. It's the users fault". (Ofcourse this
> is what I understood their comments to be.)
>
> So I think we agree to the solution we are currently persuing?
OK, I've tried to read up on the whole conversation. I'm tired and
stupid so I may have misuderstood it all, but here goes nothing:
The current code will print a "permission denied" error message if BDB
fails because of a permission problem. When this happens, the admin must
fix the problem by hand. There will not be any confusion because the
error message is clear. It it not clear enough, it would be a fiveliner
to add an URL to the FAQ in the error message.
The holy grail would of course be to prevent a user with a bad umask
(for example) to wedge the repos for all the other users. Doing so would
definately involve messing with permissions, and it would be hard to get
it right, IMHO.
Is it possible that you are trying to solve an old problem? Just a short
time ago neither ra_dav, ra_svn nor ra_local did manage to present a
"permission denied" error message to the user, but now they do.
So I'm just wondering if this is still a problem? Do you have an example
where you get a different error than "permission denied" when it really
is a perssion problem?
/Tobias, nobody, tired and stupid :-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by e....@gmx.net.
Tobias,
I'm a nobody too. We were definitely not discussion messing with
permissions. We were discussing where and how to throw an understandable error message
or to leave it to berkeley to display any error (not permission denied btw).
Greg Hudson, Mark and I advocated checking conditions so as to be sure that
if any db errors occur, that they are not due to inaccessability of the
database by berkeley. John said "there's nothing wrong with BDB and nothing with
our use of it, so we should do nothing. It's the users fault". (Ofcourse this
is what I understood their comments to be.)
So I think we agree to the solution we are currently persuing?
bye,
Erik.
> e.huelsmann@gmx.net wrote:
> > It seems like this patch is addressing another issue:
> > whether or not there is a repository. Our problem does not focus on the
> > existence of one, but on the accessibility in case one is found.
>
> That patch was about fixing ra_svn and ra_local to make sure that any
> error detected when opening the repository is sent upwards and displayed
> to the user.
>
> FWIW, I think that it's the wrong approach to mess with the permission
> of the files inside the db dir. If the permissions are wrong, BDB will
> throw an error. Subversion's mission is to show a nice and clear error
> message to the user so that (s)he can understand what the problem is and
> fix it.
>
> If there are places where subversion failes to spit out a nice
> "Permission denied" error message, those places should be fixed.
>
> If the "Permission denied" at the end of the current error message is
> not clear enough, it would be a simple matter to clarify it. It could
> even contain an URL to the FAQ.
>
> ...but I'm just a nobody, of course... :-)
>
> /Tobias
>
--
COMPUTERBILD 15/03: Premium-e-mail-Dienste im Test
--------------------------------------------------
1. GMX TopMail - Platz 1 und Testsieger!
2. GMX ProMail - Platz 2 und Preis-Qualitätssieger!
3. Arcor - 4. web.de - 5. T-Online - 6. freenet.de - 7. daybyday - 8. e-Post
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Tobias Ringström <to...@ringstrom.mine.nu>.
e.huelsmann@gmx.net wrote:
> It seems like this patch is addressing another issue:
> whether or not there is a repository. Our problem does not focus on the
> existence of one, but on the accessibility in case one is found.
That patch was about fixing ra_svn and ra_local to make sure that any
error detected when opening the repository is sent upwards and displayed
to the user.
FWIW, I think that it's the wrong approach to mess with the permission
of the files inside the db dir. If the permissions are wrong, BDB will
throw an error. Subversion's mission is to show a nice and clear error
message to the user so that (s)he can understand what the problem is and
fix it.
If there are places where subversion failes to spit out a nice
"Permission denied" error message, those places should be fixed.
If the "Permission denied" at the end of the current error message is
not clear enough, it would be a simple matter to clarify it. It could
even contain an URL to the FAQ.
...but I'm just a nobody, of course... :-)
/Tobias
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by e....@gmx.net.
It seems like this patch is addressing another issue:
whether or not there is a repository. Our problem does not focus on the
existence of one, but on the accessibility in case one is found.
bye,
Erik.
> Looking at the patch, I wonder if it is the right place to do the
> check. Can someone more qualified comment?
>
> On Fri, Sep 05, 2003 at 10:17:18PM +0300, Jani Averbach wrote:
> >
> > Sorry for posting off/miss-thread, and not following this thread very
> > closely, but how this compares to current topic?
> >
> > http://www.contactor.se/~dast/svn/archive-2003-08/1310.shtml
> >
> > From: tobias_at_ringstrom.mine.nu
> > Subject: [PATCH] Unifying and fixing the error handling for ra_local and
> > svnserve
> >
> > It later turned out to be a permission/umask problem. With the enclosed
> > patch, the output looks like this instead:
> >
> > svn: Couldn't find a repository
> > svn: No repository found in 'svn://localhost/home/svn/trunk'
> > svn: Berkeley DB error
> > svn: Berkeley DB error while opening environment for filesystem
> > /home/svn/db:
> > Permission denied
> >
> >
> > This patch has been applied:
> > http://www.contactor.se/~dast/svn/archive-2003-08/1357.shtml
> >
> >
> > BR, Jani
> >
> > --
> > Jani Averbach
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
--
COMPUTERBILD 15/03: Premium-e-mail-Dienste im Test
--------------------------------------------------
1. GMX TopMail - Platz 1 und Testsieger!
2. GMX ProMail - Platz 2 und Preis-Qualitätssieger!
3. Arcor - 4. web.de - 5. T-Online - 6. freenet.de - 7. daybyday - 8. e-Post
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Marc Singer <el...@buici.com>.
Looking at the patch, I wonder if it is the right place to do the
check. Can someone more qualified comment?
On Fri, Sep 05, 2003 at 10:17:18PM +0300, Jani Averbach wrote:
>
> Sorry for posting off/miss-thread, and not following this thread very
> closely, but how this compares to current topic?
>
> http://www.contactor.se/~dast/svn/archive-2003-08/1310.shtml
>
> From: tobias_at_ringstrom.mine.nu
> Subject: [PATCH] Unifying and fixing the error handling for ra_local and
> svnserve
>
> It later turned out to be a permission/umask problem. With the enclosed
> patch, the output looks like this instead:
>
> svn: Couldn't find a repository
> svn: No repository found in 'svn://localhost/home/svn/trunk'
> svn: Berkeley DB error
> svn: Berkeley DB error while opening environment for filesystem
> /home/svn/db:
> Permission denied
>
>
> This patch has been applied:
> http://www.contactor.se/~dast/svn/archive-2003-08/1357.shtml
>
>
> BR, Jani
>
> --
> Jani Averbach
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Marc Singer <el...@buici.com>.
On Fri, Sep 05, 2003 at 02:43:36PM -0400, Greg Hudson wrote:
> On Fri, 2003-09-05 at 14:38, Marc Singer wrote:
> > > (1) The user has full access to the repository, but is not the same as
> > > the owner of the repository, and has a umask of 022 or worse. So
> > > accessing the repository would work fine, but would break things for the
> > > next person to come along. (We can't just reset the umask because we're
> > > a library and we can't muck with global process state like that, and we
> > > can't munge the permissions of newly-created logfiles because Berkeley
> > > DB has no support for that.) We would like to preemptively error out in
> > > this case to avoid putting repositories into inconsistent states.
> >
> > I think that we can ignore this problem. Before yelling... If a user
> > does this, then it isn't really our problem.
>
> It absolutely is our problem. It comes up on the list all the time.
> Anything users do frequently, even if it's something wrong, is something
> we have to handle gracefully if at all possible. (That can mean
> succeeding, or failing with a helpful error message.)
We have two responsibilities:
1) prevent data loss
2) be clear with error messages
IMHO, the problem with the troubles seen by users is that their
repositories become corrupt without being told why. *AND* we can be
more clear.
Without mandating a setup, there will always ways that users can make
their repository non-functional. As long as we can tell them why, I
think we can finesse some situations that create the problem.
> > Moreover, we don't know if a user has
> > configured a private repository that can only be accessed by a single
> > local user. A umask of 077 could be valid.
>
> Yes, but I specifically wrote "... but is not the same as the owner of
> the repository..." to address that case.
>
> > > (2) The user does not have required access to the repository, perhaps
> > > because of something like what I described in (1). We would like to
> > > preemptively error out in this case so that people realize they have a
> > > permissions problem and not a data corruption problem.
> >
> > Right. If we catch this case, then we handle 1) because someone will
> > later experience a well-worded error that explains the situation.
>
> But they may not be in a position to fix it, whereas the user in (1) is
> in a position to prevent it from occurring in the first place.
I see what you mean. This is the case where any files created by the
ambient user will not be accessible by the user who created other
files. This may be a tricky one to enforce since it may be hard to
sus the user's intent without making some unacceptable inferences
about the BDB layout. For example, it is possible that the directory
is owned by svn.svn group writable, and the log files are all owned by
bob.svn umask 077, and that bob owns the whole repository. The SVN
user can administer, but bob is the only user who can use the
repository.
As such, I'm not certain there is legal check we can make.
> > I don't think we need to worry about violating the abstraction. A
> > simple filesystem enumeration just means that the ambient user has
> > access to files in the repository.
>
> Well, I think it's possible to shunt the logfiles off to a different
> directory, and that would prevent a simple directory enumeration from
> working.
I'm convinced. I'll see what it looks like.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Marc Singer <el...@buici.com>.
On Fri, Sep 05, 2003 at 08:21:49PM +0100, Philip Martin wrote:
> Greg Hudson <gh...@MIT.EDU> writes:
>
> >> I don't think we need to worry about violating the abstraction. A
> >> simple filesystem enumeration just means that the ambient user has
> >> access to files in the repository.
> >
> > Well, I think it's possible to shunt the logfiles off to a different
> > directory, and that would prevent a simple directory enumeration from
> > working.
>
> Verifying that access will work involves checking the data files, the
> environment files and the log files at the very least. The location
> of both the data files and the log files can be customised, see BDB
> documentation of set_data_dir and set_lg_dir.
>
> A couple of other things to bear in mind:
>
> - In normal use a user needs to be able to create new log files,
> verifying that this is possible involves checking directory
> permissions all the way back to root. I don't know whether failure to
> create a log file affects only the one user, I assume the transaction
> will fail, or whether it will lead to a need for database recovery.
>
> - BDB may create temporary files, although I don't know if Subversion
> causes any temporary files to be created. The location can be
> customised, see set_tmp_dir, and once again I don't know if failure to
> create such a file affects just the single user or whether it will
> lead to need for database recovery.
Looks tractable.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Philip Martin <ph...@codematters.co.uk>.
Greg Hudson <gh...@MIT.EDU> writes:
> On Fri, 2003-09-05 at 15:21, Philip Martin wrote:
>> - In normal use a user needs to be able to create new log files,
>> verifying that this is possible involves checking directory
>> permissions all the way back to root.
>
> Really? If we can stat() the directory at all, then only the mode bits
> on that directory should affect whether we can create files there.
Yes, you are right.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Marc Singer <el...@buici.com>.
On Fri, Sep 05, 2003 at 03:46:35PM -0400, Greg Hudson wrote:
> On Fri, 2003-09-05 at 15:21, Philip Martin wrote:
> > - In normal use a user needs to be able to create new log files,
> > verifying that this is possible involves checking directory
> > permissions all the way back to root.
>
> Really? If we can stat() the directory at all, then only the mode bits
> on that directory should affect whether we can create files there.
I think you are right. Stat'ing that target is sufficient.
But that isn't all. Don't we also need to be able write to existing
log file?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Greg Hudson <gh...@MIT.EDU>.
On Fri, 2003-09-05 at 15:21, Philip Martin wrote:
> - In normal use a user needs to be able to create new log files,
> verifying that this is possible involves checking directory
> permissions all the way back to root.
Really? If we can stat() the directory at all, then only the mode bits
on that directory should affect whether we can create files there.
(Of course, if the repository is in AFS, our permissions-checking is
just going to get in the way. I don't know how well BDB 4 databases in
AFS work.)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Philip Martin <ph...@codematters.co.uk>.
Greg Hudson <gh...@MIT.EDU> writes:
>> I don't think we need to worry about violating the abstraction. A
>> simple filesystem enumeration just means that the ambient user has
>> access to files in the repository.
>
> Well, I think it's possible to shunt the logfiles off to a different
> directory, and that would prevent a simple directory enumeration from
> working.
Verifying that access will work involves checking the data files, the
environment files and the log files at the very least. The location
of both the data files and the log files can be customised, see BDB
documentation of set_data_dir and set_lg_dir.
A couple of other things to bear in mind:
- In normal use a user needs to be able to create new log files,
verifying that this is possible involves checking directory
permissions all the way back to root. I don't know whether failure to
create a log file affects only the one user, I assume the transaction
will fail, or whether it will lead to a need for database recovery.
- BDB may create temporary files, although I don't know if Subversion
causes any temporary files to be created. The location can be
customised, see set_tmp_dir, and once again I don't know if failure to
create such a file affects just the single user or whether it will
lead to need for database recovery.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Greg Hudson <gh...@MIT.EDU>.
On Fri, 2003-09-05 at 14:38, Marc Singer wrote:
> > (1) The user has full access to the repository, but is not the same as
> > the owner of the repository, and has a umask of 022 or worse. So
> > accessing the repository would work fine, but would break things for the
> > next person to come along. (We can't just reset the umask because we're
> > a library and we can't muck with global process state like that, and we
> > can't munge the permissions of newly-created logfiles because Berkeley
> > DB has no support for that.) We would like to preemptively error out in
> > this case to avoid putting repositories into inconsistent states.
>
> I think that we can ignore this problem. Before yelling... If a user
> does this, then it isn't really our problem.
It absolutely is our problem. It comes up on the list all the time.
Anything users do frequently, even if it's something wrong, is something
we have to handle gracefully if at all possible. (That can mean
succeeding, or failing with a helpful error message.)
> Moreover, we don't know if a user has
> configured a private repository that can only be accessed by a single
> local user. A umask of 077 could be valid.
Yes, but I specifically wrote "... but is not the same as the owner of
the repository..." to address that case.
> > (2) The user does not have required access to the repository, perhaps
> > because of something like what I described in (1). We would like to
> > preemptively error out in this case so that people realize they have a
> > permissions problem and not a data corruption problem.
>
> Right. If we catch this case, then we handle 1) because someone will
> later experience a well-worded error that explains the situation.
But they may not be in a position to fix it, whereas the user in (1) is
in a position to prevent it from occurring in the first place.
> I don't think we need to worry about violating the abstraction. A
> simple filesystem enumeration just means that the ambient user has
> access to files in the repository.
Well, I think it's possible to shunt the logfiles off to a different
directory, and that would prevent a simple directory enumeration from
working.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Marc Singer <el...@buici.com>.
On Fri, Sep 05, 2003 at 12:19:37PM -0400, Greg Hudson wrote:
> On Fri, 2003-09-05 at 11:19, Marc Singer wrote:
> > The BDB backend is a process running within the context of a user, no
> > what operating system this happens to execute on. If the privilege of
> > that user is insufficient to write to the database, any operations on
> > the repository will fail. We can check for this kind of failure.
>
> Aha, now I get it. I can further break this down into two kinds of
> failure tests:
>
> (1) The user has full access to the repository, but is not the same as
> the owner of the repository, and has a umask of 022 or worse. So
> accessing the repository would work fine, but would break things for the
> next person to come along. (We can't just reset the umask because we're
> a library and we can't muck with global process state like that, and we
> can't munge the permissions of newly-created logfiles because Berkeley
> DB has no support for that.) We would like to preemptively error out in
> this case to avoid putting repositories into inconsistent states.
I think that we can ignore this problem. Before yelling... If a user
does this, then it isn't really our problem. This is a bonafide FU in
the systems configuration. Moreover, we don't know if a user has
configured a private repository that can only be accessed by a single
local user. A umask of 077 could be valid.
> (2) The user does not have required access to the repository, perhaps
> because of something like what I described in (1). We would like to
> preemptively error out in this case so that people realize they have a
> permissions problem and not a data corruption problem.
Right. If we catch this case, then we handle 1) because someone will
later experience a well-worded error that explains the situation.
> In addition to portability concerns (which are probably manageable), the
> difficult part about writing tests for these failures is that we don't
> want to violate the Berkeley DB abstraction. But BDB does have a
> function to list all logfiles, so I think both tests are probably still
> doable. So, +1. Someone should submit a patch or, failing that, file
> an issue. Even if (2) turns out to be difficult, (1) shouldn't be very
> hard, and that would solve 90% of the problem.
I don't think we need to worry about violating the abstraction. A
simple filesystem enumeration just means that the ambient user has
access to files in the repository. Whether those files are BDB or
some other applications isn't really interesting. AFAICT, there is no
currently valid configuration where a user can read/write to the
repository *and* should be legally prevented from writing to other
files in the repository.
In the event that we switch to another DB that uses a server process,
a backend check will be written for it.
I'll submit a patch for review.
Cheers.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Greg Hudson <gh...@MIT.EDU>.
On Fri, 2003-09-05 at 11:19, Marc Singer wrote:
> The BDB backend is a process running within the context of a user, no
> what operating system this happens to execute on. If the privilege of
> that user is insufficient to write to the database, any operations on
> the repository will fail. We can check for this kind of failure.
Aha, now I get it. I can further break this down into two kinds of
failure tests:
(1) The user has full access to the repository, but is not the same as
the owner of the repository, and has a umask of 022 or worse. So
accessing the repository would work fine, but would break things for the
next person to come along. (We can't just reset the umask because we're
a library and we can't muck with global process state like that, and we
can't munge the permissions of newly-created logfiles because Berkeley
DB has no support for that.) We would like to preemptively error out in
this case to avoid putting repositories into inconsistent states.
(2) The user does not have required access to the repository, perhaps
because of something like what I described in (1). We would like to
preemptively error out in this case so that people realize they have a
permissions problem and not a data corruption problem.
In addition to portability concerns (which are probably manageable), the
difficult part about writing tests for these failures is that we don't
want to violate the Berkeley DB abstraction. But BDB does have a
function to list all logfiles, so I think both tests are probably still
doable. So, +1. Someone should submit a patch or, failing that, file
an issue. Even if (2) turns out to be difficult, (1) shouldn't be very
hard, and that would solve 90% of the problem.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by "B. W. Fitzpatrick" <fi...@red-bean.com>.
Marc Singer <el...@buici.com> writes:
> On Fri, Sep 05, 2003 at 12:12:15PM -0500, B. W. Fitzpatrick wrote:
> > Marc Singer <el...@buici.com> writes:
> > > On Thu, Sep 04, 2003 at 09:57:03PM -0500, B. W. Fitzpatrick wrote:
> > > > Actually, I believe that it has, and it was rejected because there is n
> o
> > > > logical mapping between UNIX fs permissions and Windows fs ACLs. I
> > > > recall the consensus being that it wasn't worth it to stuff in a whole
> > > > bunch of platform specific hoo-ha to accomodate this buglet.
> > >
> > > That isn't the kind of permissions check I'm talking about.
> > >
> > > The BDB backend is a process running within the context of a user, no
> > > what operating system this happens to execute on. If the privilege of
> > > that user is insufficient to write to the database, any operations on
> > > the repository will fail. We can check for this kind of failure.
> > > Moreover, we can check for a privilege failure where the user can
> > > access some files/directories but not all of them. Should the user be
> > > running on Windows and this check is difficult to make, then oh well,
> > > we don't do it. In this case, there no priviledge 'mapping' to be
> > > done.
> >
> > Aha. Well, that sounds OK to me. Consider this a +.5, and a +1 if you
> > want to write the patch. :-)
>
> What's that? +1 what?
Subversion uses Apache style voting:
http://httpd.apache.org/dev/guidelines.html
> I'll look at writing a patch.
Groovy.
-Fitz
--
Brian W. Fitzpatrick <fi...@red-bean.com> http://www.red-bean.com/fitz/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Marc Singer <el...@buici.com>.
On Fri, Sep 05, 2003 at 12:12:15PM -0500, B. W. Fitzpatrick wrote:
> Marc Singer <el...@buici.com> writes:
> > On Thu, Sep 04, 2003 at 09:57:03PM -0500, B. W. Fitzpatrick wrote:
> > > Actually, I believe that it has, and it was rejected because there is no
> > > logical mapping between UNIX fs permissions and Windows fs ACLs. I
> > > recall the consensus being that it wasn't worth it to stuff in a whole
> > > bunch of platform specific hoo-ha to accomodate this buglet.
> >
> > That isn't the kind of permissions check I'm talking about.
> >
> > The BDB backend is a process running within the context of a user, no
> > what operating system this happens to execute on. If the privilege of
> > that user is insufficient to write to the database, any operations on
> > the repository will fail. We can check for this kind of failure.
> > Moreover, we can check for a privilege failure where the user can
> > access some files/directories but not all of them. Should the user be
> > running on Windows and this check is difficult to make, then oh well,
> > we don't do it. In this case, there no priviledge 'mapping' to be
> > done.
>
> Aha. Well, that sounds OK to me. Consider this a +.5, and a +1 if you
> want to write the patch. :-)
What's that? +1 what?
I'll look at writing a patch.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by "B. W. Fitzpatrick" <fi...@red-bean.com>.
Marc Singer <el...@buici.com> writes:
> On Thu, Sep 04, 2003 at 09:57:03PM -0500, B. W. Fitzpatrick wrote:
> > Actually, I believe that it has, and it was rejected because there is no
> > logical mapping between UNIX fs permissions and Windows fs ACLs. I
> > recall the consensus being that it wasn't worth it to stuff in a whole
> > bunch of platform specific hoo-ha to accomodate this buglet.
>
> That isn't the kind of permissions check I'm talking about.
>
> The BDB backend is a process running within the context of a user, no
> what operating system this happens to execute on. If the privilege of
> that user is insufficient to write to the database, any operations on
> the repository will fail. We can check for this kind of failure.
> Moreover, we can check for a privilege failure where the user can
> access some files/directories but not all of them. Should the user be
> running on Windows and this check is difficult to make, then oh well,
> we don't do it. In this case, there no priviledge 'mapping' to be
> done.
Aha. Well, that sounds OK to me. Consider this a +.5, and a +1 if you
want to write the patch. :-)
-Fitz
--
Brian W. Fitzpatrick <fi...@red-bean.com> http://www.red-bean.com/fitz/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Jani Averbach <ja...@cc.jyu.fi>.
Sorry for posting off/miss-thread, and not following this thread very
closely, but how this compares to current topic?
http://www.contactor.se/~dast/svn/archive-2003-08/1310.shtml
From: tobias_at_ringstrom.mine.nu
Subject: [PATCH] Unifying and fixing the error handling for ra_local and
svnserve
It later turned out to be a permission/umask problem. With the enclosed
patch, the output looks like this instead:
svn: Couldn't find a repository
svn: No repository found in 'svn://localhost/home/svn/trunk'
svn: Berkeley DB error
svn: Berkeley DB error while opening environment for filesystem
/home/svn/db:
Permission denied
This patch has been applied:
http://www.contactor.se/~dast/svn/archive-2003-08/1357.shtml
BR, Jani
--
Jani Averbach
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Marc Singer <el...@buici.com>.
On Thu, Sep 04, 2003 at 09:57:03PM -0500, B. W. Fitzpatrick wrote:
>
> Marc Singer <el...@buici.com> writes:
> > 1) Obviously, a permissions check, if there is one, belongs in the
> > filesystem driver.
> >
> > 2) The cost of the check is minimal. Stat'ing a dozen files is cheap
> > because the OS caches the data. Subsequent access by BDB will open
> > the same files using the cached data.
> >
> > 3) The expense of not performing it is kinda high when we see how
> > often permissions problems plague users. If Subversion isn't easy
> > to use then it won't be used. Admonishing people for failing to
> > read the documentation is a poor sales pitch.
> >
> > 4) AFAICT, there is no fix to be made in svnadmin because this isn't
> > really a setup issue. People change the way that they use
> > their repositories. Sometimes that new behavior, while valid in
> > concept, isn't well executed. It would be *way* helpful if the
> > program let the user know more explicitly what the problem is.
> >
> > The HACKING guide suggests to me that the project bears the value of
> > sturdy design:
> >
> > Input validation is the act of defining legal input and rejecting
> > everything else. The code must perform input validation on all
> > untrusted
> > input.
> >
> > I asked the question to see if this has come up before. It appears
> > that it hasn't.
>
> Actually, I believe that it has, and it was rejected because there is no
> logical mapping between UNIX fs permissions and Windows fs ACLs. I
> recall the consensus being that it wasn't worth it to stuff in a whole
> bunch of platform specific hoo-ha to accomodate this buglet.
That isn't the kind of permissions check I'm talking about.
The BDB backend is a process running within the context of a user, no
what operating system this happens to execute on. If the privilege of
that user is insufficient to write to the database, any operations on
the repository will fail. We can check for this kind of failure.
Moreover, we can check for a privilege failure where the user can
access some files/directories but not all of them. Should the user be
running on Windows and this check is difficult to make, then oh well,
we don't do it. In this case, there no priviledge 'mapping' to be
done.
Cheers.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by "B. W. Fitzpatrick" <fi...@red-bean.com>.
Marc Singer <el...@buici.com> writes:
> 1) Obviously, a permissions check, if there is one, belongs in the
> filesystem driver.
>
> 2) The cost of the check is minimal. Stat'ing a dozen files is cheap
> because the OS caches the data. Subsequent access by BDB will open
> the same files using the cached data.
>
> 3) The expense of not performing it is kinda high when we see how
> often permissions problems plague users. If Subversion isn't easy
> to use then it won't be used. Admonishing people for failing to
> read the documentation is a poor sales pitch.
>
> 4) AFAICT, there is no fix to be made in svnadmin because this isn't
> really a setup issue. People change the way that they use
> their repositories. Sometimes that new behavior, while valid in
> concept, isn't well executed. It would be *way* helpful if the
> program let the user know more explicitly what the problem is.
>
> The HACKING guide suggests to me that the project bears the value of
> sturdy design:
>
> Input validation is the act of defining legal input and rejecting
> everything else. The code must perform input validation on all
> untrusted
> input.
>
> I asked the question to see if this has come up before. It appears
> that it hasn't.
Actually, I believe that it has, and it was rejected because there is no
logical mapping between UNIX fs permissions and Windows fs ACLs. I
recall the consensus being that it wasn't worth it to stuff in a whole
bunch of platform specific hoo-ha to accomodate this buglet.
-Fitz
--
Brian W. Fitzpatrick <fi...@red-bean.com> http://www.red-bean.com/fitz/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Could svn_backend check permissions?
Posted by Greg Hudson <gh...@MIT.EDU>.
On Thu, 2003-09-04 at 22:14, Marc Singer wrote:
> 1) Obviously, a permissions check, if there is one, belongs in the
> filesystem driver.
I'm afraid I have no idea what you're talking about. Could you take a
step back and better describe what you're asking for?
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org