You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Kristian Kauper <kk...@au.yahoo-inc.com> on 2008/04/27 03:15:58 UTC

passwd file permissions with svn+ssh

Does anyone know if the patch that Marcus refers to ever made it into a 
release?

Since my original query, I've upgrade to 1.4.4, but this problem still 
exists.

Thanks.

Kristian Kauper

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: passwd file permissions with svn+ssh

Posted by Kristian Kauper <kk...@au.yahoo-inc.com>.
Thanks Richard, I'll have a play with it.

-K

Richard Hansen wrote:
> Kristian Kauper wrote on 2006-11-13:
>> I allow access to my repository via the svn:// scheme. So I have user
>> credentials stored in my repository's "passwd" file (to be clear, this is
>> subversion's passwd file, not the system passwd file). For security, I want
>> the permissions on this file to be set to 0600.
>>
>> The problem is that I also want to support the svn+ssh:// scheme. But, when
>> I try to use ssh to access the repository, SVN complains that it can't read
>> the passwd file -- no kidding, I certainly don't want everyone who can log
>> in via SSH (or, to be precise, those in the "svn" group) to also be able to
>> read all of the subversion credentials.
>>
>> Is there any way around this? I certainly think this is a bug, as SVN should
>> not have to read the passwd file if the user is already authenticated via
>> SSH.
> 
> Assuming your server is on a POSIX machine, you could use the
> setuid/setgid svnserve wrapper I recently announced
> (http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=137577) to
> work around this problem.
> 
> Here's how you could set it up:
>    1. rename svnserve to svnserve.real
>    2. install svnstsw (the svnserve tunnel-mode setuid wrapper I wrote)
>    3. create a symlink named 'svnserve' (in the PATH) that points to the
> svnstsw wrapper
>    4. create a 'svn' group (do not add anyone to this group)
>    5. make 'svn' the group owner of the svnstsw executable
>    6. set the setgid bit on the svnstsw executable
>    7. set permissions on the passwd file so that only the svnserve.real
> daemon you use for svn:// access and the svn group can read the passwd file
> 
> With the above setup, users wouldn't be able to read the passwd file,
> yet svnserve would not complain when in svn+ssh:// mode since the setgid
> bit allows it to read passwd.
> 
> Hope this helps,
> Richard
> 
> 
> Kristian Kauper recently wrote:
>> Sorry! I thought I was replying to the original message thread. Here's
>> Marcus's response, with links to my original email:
>>
>> http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=121507
>>
>> BTW, I also received this response from David Anderson:
>>
>> "The password storage and shell access issues with svnserve is well
>> documented. The recommended course of action (if apache+mod_dav_svn is
>> not an option) is to configure ssh to allow svn committer users no
>> access to the machine other than execution of /usr/bin/svnserve. This
>> forces all their interaction with the repository to go through the
>> svnserve interface, thus preventing them from accessing anything
>> directly (and certainly from getting at the configuration). Another
>> recommended policy is to disallow users to set their own password for
>> svnserve, but rather to autogenerate it and use the client caching
>> mechanism to not have to remember it. This password then offers access
>> to nothing other than the version control repository, where any
>> unauthorized change can be reverted easily. All these options, and
>> more, are discussed in the svn book, at http://svnbook.org ."
>>
>> To be honest, however, I don't think these are great suggestions. The
>> first is just impractical in our environment, and I'd assume that many
>> people would want to have a system that allows SSH access for both SVN
>> and shell.
>>
>> I just don't get why this is an issue in the first place. Why does the
>> code need to read the passwd file if a user has already authenticated
>> via SSH? I thought that was the point of the SSH access method.
>>
>> David says that this issue is well documented, but for the life of me I
>> couldn't find much in the way of formal documentation or mailing list
>> discussions.
>>
>> Thanks.
>>
>> -K
>>
>> Karl Fogel wrote:
>>> Kristian Kauper <kk...@au.yahoo-inc.com> writes:
>>>> Does anyone know if the patch that Marcus refers to ever made it into
>>>> a release?
>>>>
>>>> Since my original query, I've upgrade to 1.4.4, but this problem still
>>>> exists.
>>> Can you point to the original mail in the archives, so people know what
>>> you're referring to?  Thanks.
>>>
>>> -Karl
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: passwd file permissions with svn+ssh

Posted by Richard Hansen <rh...@bbn.com>.
Kristian Kauper wrote on 2006-11-13:
> I allow access to my repository via the svn:// scheme. So I have user 
> credentials stored in my repository's "passwd" file (to be clear, this is 
> subversion's passwd file, not the system passwd file). For security, I want 
> the permissions on this file to be set to 0600.
> 
> The problem is that I also want to support the svn+ssh:// scheme. But, when 
> I try to use ssh to access the repository, SVN complains that it can't read 
> the passwd file -- no kidding, I certainly don't want everyone who can log 
> in via SSH (or, to be precise, those in the "svn" group) to also be able to 
> read all of the subversion credentials.
> 
> Is there any way around this? I certainly think this is a bug, as SVN should 
> not have to read the passwd file if the user is already authenticated via 
> SSH.

Assuming your server is on a POSIX machine, you could use the 
setuid/setgid svnserve wrapper I recently announced 
(http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=137577) to 
work around this problem.

Here's how you could set it up:
   1. rename svnserve to svnserve.real
   2. install svnstsw (the svnserve tunnel-mode setuid wrapper I wrote)
   3. create a symlink named 'svnserve' (in the PATH) that points to the 
svnstsw wrapper
   4. create a 'svn' group (do not add anyone to this group)
   5. make 'svn' the group owner of the svnstsw executable
   6. set the setgid bit on the svnstsw executable
   7. set permissions on the passwd file so that only the svnserve.real 
daemon you use for svn:// access and the svn group can read the passwd file

With the above setup, users wouldn't be able to read the passwd file, 
yet svnserve would not complain when in svn+ssh:// mode since the setgid 
bit allows it to read passwd.

Hope this helps,
Richard


Kristian Kauper recently wrote:
> Sorry! I thought I was replying to the original message thread. Here's 
> Marcus's response, with links to my original email:
> 
> http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=121507
> 
> BTW, I also received this response from David Anderson:
> 
> "The password storage and shell access issues with svnserve is well
> documented. The recommended course of action (if apache+mod_dav_svn is
> not an option) is to configure ssh to allow svn committer users no
> access to the machine other than execution of /usr/bin/svnserve. This
> forces all their interaction with the repository to go through the
> svnserve interface, thus preventing them from accessing anything
> directly (and certainly from getting at the configuration). Another
> recommended policy is to disallow users to set their own password for
> svnserve, but rather to autogenerate it and use the client caching
> mechanism to not have to remember it. This password then offers access
> to nothing other than the version control repository, where any
> unauthorized change can be reverted easily. All these options, and
> more, are discussed in the svn book, at http://svnbook.org ."
> 
> To be honest, however, I don't think these are great suggestions. The 
> first is just impractical in our environment, and I'd assume that many 
> people would want to have a system that allows SSH access for both SVN 
> and shell.
> 
> I just don't get why this is an issue in the first place. Why does the 
> code need to read the passwd file if a user has already authenticated 
> via SSH? I thought that was the point of the SSH access method.
> 
> David says that this issue is well documented, but for the life of me I 
> couldn't find much in the way of formal documentation or mailing list 
> discussions.
> 
> Thanks.
> 
> -K
> 
> Karl Fogel wrote:
>> Kristian Kauper <kk...@au.yahoo-inc.com> writes:
>>> Does anyone know if the patch that Marcus refers to ever made it into
>>> a release?
>>>
>>> Since my original query, I've upgrade to 1.4.4, but this problem still
>>> exists.
>>
>> Can you point to the original mail in the archives, so people know what
>> you're referring to?  Thanks.
>>
>> -Karl


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: passwd file permissions with svn+ssh

Posted by Kristian Kauper <kk...@au.yahoo-inc.com>.
Greg, I'd love to tackle the patch, but probably don't have time in the 
near future. Anyone else (with probably much more knowledge of the 
system the I have!) want to give it a go?

-K

Karl Fogel wrote:
> Greg Hudson <gh...@MIT.EDU> writes:
>> Long answer: from the server's viewpoint, svnserve sees that it has been
>> executed in "tunnel mode" (-t) which means the EXTERNAL auth mechanism
>> is available to the client.  This mechanism allows the client to
>> authenticate by fiat, providing no additional credentials.  "I already
>> proved to you who I am, let's move on."  In theory, the client could
>> ignore EXTERNAL and choose to authenticate to a different user by
>> username/password.  Our client never chooses to do this, but the server
>> code allows it.  In that case, svnserve would need access to the
>> password file.  Since that's not an authentication scenario most people
>> are interested in (or one our client supports), such access is not
>> generally important.
> 
> Thank you for the clear explanation.
> 
> So, Kristian Kauper... would you like to write the patch?  It's okay to
> say no, of course, but we'd be remiss not to ask :-).  See
> 
>    http://subversion.tigris.org/hacking.html#patches
> 
> if you're not familiar with the patch submission guidelines already.
> 
> Best,
> -karl
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: passwd file permissions with svn+ssh

Posted by Karl Fogel <kf...@red-bean.com>.
Greg Hudson <gh...@MIT.EDU> writes:
> Long answer: from the server's viewpoint, svnserve sees that it has been
> executed in "tunnel mode" (-t) which means the EXTERNAL auth mechanism
> is available to the client.  This mechanism allows the client to
> authenticate by fiat, providing no additional credentials.  "I already
> proved to you who I am, let's move on."  In theory, the client could
> ignore EXTERNAL and choose to authenticate to a different user by
> username/password.  Our client never chooses to do this, but the server
> code allows it.  In that case, svnserve would need access to the
> password file.  Since that's not an authentication scenario most people
> are interested in (or one our client supports), such access is not
> generally important.

Thank you for the clear explanation.

So, Kristian Kauper... would you like to write the patch?  It's okay to
say no, of course, but we'd be remiss not to ask :-).  See

   http://subversion.tigris.org/hacking.html#patches

if you're not familiar with the patch submission guidelines already.

Best,
-karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: passwd file permissions with svn+ssh

Posted by Greg Hudson <gh...@MIT.EDU>.
On Tue, 2008-04-29 at 12:00 -0400, Karl Fogel wrote:
> But if there is a readable Subversion 'passwd' file, how should
> svn+ssh:// interact with it?  Not at all?  Or if the same username is
> present in the passwd file, then should password authentication also be
> done?

Short answer: not at all.

Long answer: from the server's viewpoint, svnserve sees that it has been
executed in "tunnel mode" (-t) which means the EXTERNAL auth mechanism
is available to the client.  This mechanism allows the client to
authenticate by fiat, providing no additional credentials.  "I already
proved to you who I am, let's move on."  In theory, the client could
ignore EXTERNAL and choose to authenticate to a different user by
username/password.  Our client never chooses to do this, but the server
code allows it.  In that case, svnserve would need access to the
password file.  Since that's not an authentication scenario most people
are interested in (or one our client supports), such access is not
generally important.



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: passwd file permissions with svn+ssh

Posted by Karl Fogel <kf...@red-bean.com>.
Greg Hudson <gh...@MIT.EDU> writes:
> I agree with Kristian here, and this is probably an oversight on my part
> when I wrote the code (although it's been a while).  If the passwd file
> is unreadable, svnserve should just fail to authenticate anyone with
> passwords, so that the same repository can be used with svn+ssh and
> svnserve.

(I'm assuming "unreadable" would include "doesn't exist".)  Kristian's
http://subversion.tigris.org/servlets/ReadMsg?list=dev&msgNo=121502
makes sense to me.

But if there is a readable Subversion 'passwd' file, how should
svn+ssh:// interact with it?  Not at all?  Or if the same username is
present in the passwd file, then should password authentication also be
done?

-Karl

> On Tue, 2008-04-29 at 17:55 +1000, Kristian Kauper wrote:
>> Sorry! I thought I was replying to the original message thread. Here's 
>> Marcus's response, with links to my original email:
>> 
>> http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=121507
>> 
>> BTW, I also received this response from David Anderson:
>> 
>> "The password storage and shell access issues with svnserve is well
>> documented. The recommended course of action (if apache+mod_dav_svn is
>> not an option) is to configure ssh to allow svn committer users no
>> access to the machine other than execution of /usr/bin/svnserve. This
>> forces all their interaction with the repository to go through the
>> svnserve interface, thus preventing them from accessing anything
>> directly (and certainly from getting at the configuration). Another
>> recommended policy is to disallow users to set their own password for
>> svnserve, but rather to autogenerate it and use the client caching
>> mechanism to not have to remember it. This password then offers access
>> to nothing other than the version control repository, where any
>> unauthorized change can be reverted easily. All these options, and
>> more, are discussed in the svn book, at http://svnbook.org ."
>> 
>> To be honest, however, I don't think these are great suggestions. The 
>> first is just impractical in our environment, and I'd assume that many 
>> people would want to have a system that allows SSH access for both SVN 
>> and shell.
>> 
>> I just don't get why this is an issue in the first place. Why does the 
>> code need to read the passwd file if a user has already authenticated 
>> via SSH? I thought that was the point of the SSH access method.
>> 
>> David says that this issue is well documented, but for the life of me I 
>> couldn't find much in the way of formal documentation or mailing list 
>> discussions.
>> 
>> Thanks.
>> 
>> -K
>> 
>> Karl Fogel wrote:
>> > Kristian Kauper <kk...@au.yahoo-inc.com> writes:
>> >> Does anyone know if the patch that Marcus refers to ever made it into
>> >> a release?
>> >>
>> >> Since my original query, I've upgrade to 1.4.4, but this problem still
>> >> exists.
>> > 
>> > Can you point to the original mail in the archives, so people know what
>> > you're referring to?  Thanks.
>> > 
>> > -Karl
>> > 
>> 
>> ---------------------------------------------------------------------
>> 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

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Re: passwd file permissions with svn+ssh

Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
Hyrum K. Wright wrote on Thu, 1 May 2008 at 12:48 -0500:
> Daniel Shahaf wrote:
> > Daniel Shahaf wrote on Thu, 1 May 2008 at 09:04 +0300:
> > > Kristian Kauper wrote on Thu, 1 May 2008 at 09:53 +1000:
> > > > Wow, I never expected such a quick resolution. Thanks everyone for
> > > > helping with this.
> > > > 
> > > > > Committed r30868 and nominated for backport.
> > > > 
> > > > I assume that this means it won't necessarily get into a 1.4.x line,
> > > 
> > > Correct; I nominated it for backport to 1.5.x.
> > > 
> > > > but if it does, will it be part of 1.4.7?
> > > 
> > > If this fix is backported to 1.4 before 1.4.7 is released for testing
> >                                           ^^^^^^^^^^^^^^^^^
> > *If* an 1.4.7 release is made, of course.
> 
> Has it been nominated for 1.4.x?  It would need to be if it were to be
> included in a future 1.4.7 release.
> 

I nominated it for 1.5.x, but it has not been nominated for 1.4.x.
I didn't nominate it for 1.4.x because I didn't know what kind of
changes should be nominated for 1.4.x at this point.

What are the criteria for backporting changes to a minor release line
other than the youngest such line?  (In other words, what are the
criteria for backporting to 1.4.x after 1.5.x has branched?)

Thanks,

Daniel

> I actually think we should think about doing a final release off the 1.4.x
> branch after we get 1.5.0 out the door.  I suspect a number of people will be
> using 1.4 for a while yet, and since we still claim to support it, it'd be
> good if we actually followed up on that claim.
> 
> -Hyrum
> 
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Re: passwd file permissions with svn+ssh

Posted by "Hyrum K. Wright" <hy...@mail.utexas.edu>.
Daniel Shahaf wrote:
> Daniel Shahaf wrote on Thu, 1 May 2008 at 09:04 +0300:
>> Kristian Kauper wrote on Thu, 1 May 2008 at 09:53 +1000:
>>> Wow, I never expected such a quick resolution. Thanks everyone for helping
>>> with this.
>>>
>>>> Committed r30868 and nominated for backport.
>>> I assume that this means it won't necessarily get into a 1.4.x line,
>> Correct; I nominated it for backport to 1.5.x.
>>
>>> but if it does, will it be part of 1.4.7?
>> If this fix is backported to 1.4 before 1.4.7 is released for testing
>                                           ^^^^^^^^^^^^^^^^^
> *If* an 1.4.7 release is made, of course.

Has it been nominated for 1.4.x?  It would need to be if it were to be 
included in a future 1.4.7 release.

I actually think we should think about doing a final release off the 
1.4.x branch after we get 1.5.0 out the door.  I suspect a number of 
people will be using 1.4 for a while yet, and since we still claim to 
support it, it'd be good if we actually followed up on that claim.

-Hyrum


Re: [PATCH] Re: passwd file permissions with svn+ssh

Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
Daniel Shahaf wrote on Thu, 1 May 2008 at 09:04 +0300:
> Kristian Kauper wrote on Thu, 1 May 2008 at 09:53 +1000:
> > Wow, I never expected such a quick resolution. Thanks everyone for helping
> > with this.
> > 
> > > Committed r30868 and nominated for backport.
> > 
> > I assume that this means it won't necessarily get into a 1.4.x line,
> 
> Correct; I nominated it for backport to 1.5.x.
> 
> > but if it does, will it be part of 1.4.7?
> 
> If this fix is backported to 1.4 before 1.4.7 is released for testing
                                          ^^^^^^^^^^^^^^^^^
*If* an 1.4.7 release is made, of course.

Daniel


> and signing by the committers, it will be included in the release.
> 
> > (Unfortunately, I don't control our binary packages, so I have to
> > request a new build.)
> > 
> > Thanks again.
> > 
> 
> You're welcome.
> 
> Daniel
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Re: passwd file permissions with svn+ssh

Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
Kristian Kauper wrote on Thu, 1 May 2008 at 09:53 +1000:
> Wow, I never expected such a quick resolution. Thanks everyone for helping
> with this.
> 
> > Committed r30868 and nominated for backport.
> 
> I assume that this means it won't necessarily get into a 1.4.x line,

Correct; I nominated it for backport to 1.5.x.

> but if it does, will it be part of 1.4.7?

If this fix is backported to 1.4 before 1.4.7 is released for testing
and signing by the committers, it will be included in the release.

> (Unfortunately, I don't control our binary packages, so I have to
> request a new build.)
> 
> Thanks again.
> 

You're welcome.

Daniel


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Re: passwd file permissions with svn+ssh

Posted by Kristian Kauper <kk...@au.yahoo-inc.com>.
Wow, I never expected such a quick resolution. Thanks everyone for 
helping with this.

 > Committed r30868 and nominated for backport.

I assume that this means it won't necessarily get into a 1.4.x line, but 
if it does, will it be part of 1.4.7? (Unfortunately, I don't control 
our binary packages, so I have to request a new build.)

Thanks again.

Daniel Shahaf wrote:
> Daniel Shahaf wrote on Wed, 30 Apr 2008 at 22:21 +0300:
>> Greg Hudson wrote on Wed, 30 Apr 2008 at 15:11 -0400:
>>> That looks fine, but I would add a comment.
>> I'll add a comment, run 'make check' and commit.
>>
> 
> Committed r30868 and nominated for backport.
> 
> Thanks,
> 
> Daniel
> 
>> Thanks Greg,
>>
>> Daniel
>>
>>> (Actually, I'm not sure why SVN_ERR_BAD_FILENAME is being ignored
>>> either.)
>>>
>>> Daniel Shahaf wrote:
>>>> [ Kristian, since you said you wouldn't have time, I went ahead and looked
>>>> into this. ]
>>>>
>>>> Greg Hudson wrote on Tue, 29 Apr 2008 at 11:27 -0400:
>>>>
>>>>> On Tue, 2008-04-29 at 17:55 +1000, Kristian Kauper wrote:
>>>>>
>>>>>> I just don't get why this is an issue in the first place. Why does the
>>>>>> code need to read the passwd file if a user has already authenticated
>>>>>> via SSH? I thought that was the point of the SSH access method.
>>>>>>
>>>>> I agree with Kristian here, and this is probably an oversight on my part
>>>>> when I wrote the code (although it's been a while).  If the passwd file
>>>>> is unreadable, svnserve should just fail to authenticate anyone with
>>>>> passwords, so that the same repository can be used with svn+ssh and
>>>>> svnserve.
>>>>>
>>>> Does this change look correct?
>>>>
>>>> Index: subversion/svnserve/serve.c
>>>> ===================================================================
>>>> --- subversion/svnserve/serve.c   (revision 30863)
>>>> +++ subversion/svnserve/serve.c   (working copy)
>>>> @@ -236,7 +236,9 @@ svn_error_t *load_configs(svn_config_t **cfg,
>>>>            if (server)
>>>>              /* Called by listening server; log error no matter what it is.
>>>> */
>>>>              log_server_error(err, server, conn, pool);
>>>> -          if (err->apr_err != SVN_ERR_BAD_FILENAME)
>>>> +
>>>> +          if (err->apr_err != SVN_ERR_BAD_FILENAME
>>>> +              && ! APR_STATUS_IS_EACCES(err->apr_err))
>>>>              {
>>>>                if (server)
>>>>                  {
>>>>
>>>> Here is the effect.  In the examples, svnserve is run in --tunnel mode.
>>>>
>>>>   0:% chmod 0 repos/conf/passwd
>>>>   0:% grep anon-access repos/conf/svnserve.conf
>>>>   anon-access = write
>>>>
>>>>   # current trunk
>>>>   0:% svn co svn+trunk://`pwd`/repos wc
>>>>   subversion/svnserve/serve.c:248: (apr_err=215004)
>>>>   svn: Authentication failed
>>>>
>>>>   # with the patch
>>>>   0:% svn co svn+patched://`pwd`/repos wc | tail -1
>>>>   Checked out revision 1.
>>>>
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Re: passwd file permissions with svn+ssh

Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
Daniel Shahaf wrote on Wed, 30 Apr 2008 at 22:21 +0300:
> Greg Hudson wrote on Wed, 30 Apr 2008 at 15:11 -0400:
> > That looks fine, but I would add a comment.
> 
> I'll add a comment, run 'make check' and commit.
> 

Committed r30868 and nominated for backport.

Thanks,

Daniel

> Thanks Greg,
> 
> Daniel
> 
> > (Actually, I'm not sure why SVN_ERR_BAD_FILENAME is being ignored
> > either.)
> > 
> > Daniel Shahaf wrote:
> > > [ Kristian, since you said you wouldn't have time, I went ahead and looked
> > > into this. ]
> > > 
> > > Greg Hudson wrote on Tue, 29 Apr 2008 at 11:27 -0400:
> > >   
> > > > On Tue, 2008-04-29 at 17:55 +1000, Kristian Kauper wrote:
> > > >     
> > > > > I just don't get why this is an issue in the first place. Why does the
> > > > > code need to read the passwd file if a user has already authenticated
> > > > > via SSH? I thought that was the point of the SSH access method.
> > > > >       
> > > > I agree with Kristian here, and this is probably an oversight on my part
> > > > when I wrote the code (although it's been a while).  If the passwd file
> > > > is unreadable, svnserve should just fail to authenticate anyone with
> > > > passwords, so that the same repository can be used with svn+ssh and
> > > > svnserve.
> > > >     
> > > 
> > > Does this change look correct?
> > > 
> > > Index: subversion/svnserve/serve.c
> > > ===================================================================
> > > --- subversion/svnserve/serve.c	(revision 30863)
> > > +++ subversion/svnserve/serve.c	(working copy)
> > > @@ -236,7 +236,9 @@ svn_error_t *load_configs(svn_config_t **cfg,
> > >            if (server)
> > >              /* Called by listening server; log error no matter what it is.
> > > */
> > >              log_server_error(err, server, conn, pool);
> > > -          if (err->apr_err != SVN_ERR_BAD_FILENAME)
> > > +
> > > +          if (err->apr_err != SVN_ERR_BAD_FILENAME
> > > +              && ! APR_STATUS_IS_EACCES(err->apr_err))
> > >              {
> > >                if (server)
> > >                  {
> > > 
> > > Here is the effect.  In the examples, svnserve is run in --tunnel mode.
> > > 
> > > 	0:% chmod 0 repos/conf/passwd
> > > 	0:% grep anon-access repos/conf/svnserve.conf
> > > 	anon-access = write
> > > 
> > > 	# current trunk
> > > 	0:% svn co svn+trunk://`pwd`/repos wc
> > > 	subversion/svnserve/serve.c:248: (apr_err=215004)
> > > 	svn: Authentication failed
> > > 
> > > 	# with the patch
> > > 	0:% svn co svn+patched://`pwd`/repos wc | tail -1
> > > 	Checked out revision 1.
> > >   
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Re: passwd file permissions with svn+ssh

Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
Greg Hudson wrote on Wed, 30 Apr 2008 at 15:11 -0400:
> That looks fine, but I would add a comment.

I'll add a comment, run 'make check' and commit.

Thanks Greg,

Daniel

> (Actually, I'm not sure why SVN_ERR_BAD_FILENAME is being ignored
> either.)
> 
> Daniel Shahaf wrote:
> > [ Kristian, since you said you wouldn't have time, I went ahead and looked
> > into this. ]
> > 
> > Greg Hudson wrote on Tue, 29 Apr 2008 at 11:27 -0400:
> >   
> > > On Tue, 2008-04-29 at 17:55 +1000, Kristian Kauper wrote:
> > >     
> > > > I just don't get why this is an issue in the first place. Why does the
> > > > code need to read the passwd file if a user has already authenticated
> > > > via SSH? I thought that was the point of the SSH access method.
> > > >       
> > > I agree with Kristian here, and this is probably an oversight on my part
> > > when I wrote the code (although it's been a while).  If the passwd file
> > > is unreadable, svnserve should just fail to authenticate anyone with
> > > passwords, so that the same repository can be used with svn+ssh and
> > > svnserve.
> > >     
> > 
> > Does this change look correct?
> > 
> > Index: subversion/svnserve/serve.c
> > ===================================================================
> > --- subversion/svnserve/serve.c	(revision 30863)
> > +++ subversion/svnserve/serve.c	(working copy)
> > @@ -236,7 +236,9 @@ svn_error_t *load_configs(svn_config_t **cfg,
> >            if (server)
> >              /* Called by listening server; log error no matter what it is.
> > */
> >              log_server_error(err, server, conn, pool);
> > -          if (err->apr_err != SVN_ERR_BAD_FILENAME)
> > +
> > +          if (err->apr_err != SVN_ERR_BAD_FILENAME
> > +              && ! APR_STATUS_IS_EACCES(err->apr_err))
> >              {
> >                if (server)
> >                  {
> > 
> > Here is the effect.  In the examples, svnserve is run in --tunnel mode.
> > 
> > 	0:% chmod 0 repos/conf/passwd
> > 	0:% grep anon-access repos/conf/svnserve.conf
> > 	anon-access = write
> > 
> > 	# current trunk
> > 	0:% svn co svn+trunk://`pwd`/repos wc
> > 	subversion/svnserve/serve.c:248: (apr_err=215004)
> > 	svn: Authentication failed
> > 
> > 	# with the patch
> > 	0:% svn co svn+patched://`pwd`/repos wc | tail -1
> > 	Checked out revision 1.
> >   

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [PATCH] Re: passwd file permissions with svn+ssh

Posted by Greg Hudson <gh...@MIT.EDU>.
That looks fine, but I would add a comment.  (Actually, I'm not sure why 
SVN_ERR_BAD_FILENAME is being ignored either.)

Daniel Shahaf wrote:
> [ Kristian, since you said you wouldn't have time, I went ahead and looked 
> into this. ]
>
> Greg Hudson wrote on Tue, 29 Apr 2008 at 11:27 -0400:
>   
>> On Tue, 2008-04-29 at 17:55 +1000, Kristian Kauper wrote:
>>     
>>> I just don't get why this is an issue in the first place. Why does the 
>>> code need to read the passwd file if a user has already authenticated 
>>> via SSH? I thought that was the point of the SSH access method.
>>>       
>> I agree with Kristian here, and this is probably an oversight on my part
>> when I wrote the code (although it's been a while).  If the passwd file
>> is unreadable, svnserve should just fail to authenticate anyone with
>> passwords, so that the same repository can be used with svn+ssh and
>> svnserve.
>>     
>
> Does this change look correct?
>
> Index: subversion/svnserve/serve.c
> ===================================================================
> --- subversion/svnserve/serve.c	(revision 30863)
> +++ subversion/svnserve/serve.c	(working copy)
> @@ -236,7 +236,9 @@ svn_error_t *load_configs(svn_config_t **cfg,
>            if (server)
>              /* Called by listening server; log error no matter what it is. */
>              log_server_error(err, server, conn, pool);
> -          if (err->apr_err != SVN_ERR_BAD_FILENAME)
> +
> +          if (err->apr_err != SVN_ERR_BAD_FILENAME
> +              && ! APR_STATUS_IS_EACCES(err->apr_err))
>              {
>                if (server)
>                  {
>
> Here is the effect.  In the examples, svnserve is run in --tunnel mode.
>
> 	0:% chmod 0 repos/conf/passwd
> 	0:% grep anon-access repos/conf/svnserve.conf
> 	anon-access = write
>
> 	# current trunk
> 	0:% svn co svn+trunk://`pwd`/repos wc
> 	subversion/svnserve/serve.c:248: (apr_err=215004)
> 	svn: Authentication failed
>
> 	# with the patch
> 	0:% svn co svn+patched://`pwd`/repos wc | tail -1
> 	Checked out revision 1.
>   

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

[PATCH] Re: passwd file permissions with svn+ssh

Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
[ Kristian, since you said you wouldn't have time, I went ahead and looked 
into this. ]

Greg Hudson wrote on Tue, 29 Apr 2008 at 11:27 -0400:
> On Tue, 2008-04-29 at 17:55 +1000, Kristian Kauper wrote:
> > I just don't get why this is an issue in the first place. Why does the 
> > code need to read the passwd file if a user has already authenticated 
> > via SSH? I thought that was the point of the SSH access method.
> 
> I agree with Kristian here, and this is probably an oversight on my part
> when I wrote the code (although it's been a while).  If the passwd file
> is unreadable, svnserve should just fail to authenticate anyone with
> passwords, so that the same repository can be used with svn+ssh and
> svnserve.

Does this change look correct?

Index: subversion/svnserve/serve.c
===================================================================
--- subversion/svnserve/serve.c	(revision 30863)
+++ subversion/svnserve/serve.c	(working copy)
@@ -236,7 +236,9 @@ svn_error_t *load_configs(svn_config_t **cfg,
           if (server)
             /* Called by listening server; log error no matter what it is. */
             log_server_error(err, server, conn, pool);
-          if (err->apr_err != SVN_ERR_BAD_FILENAME)
+
+          if (err->apr_err != SVN_ERR_BAD_FILENAME
+              && ! APR_STATUS_IS_EACCES(err->apr_err))
             {
               if (server)
                 {

Here is the effect.  In the examples, svnserve is run in --tunnel mode.

	0:% chmod 0 repos/conf/passwd
	0:% grep anon-access repos/conf/svnserve.conf
	anon-access = write

	# current trunk
	0:% svn co svn+trunk://`pwd`/repos wc
	subversion/svnserve/serve.c:248: (apr_err=215004)
	svn: Authentication failed

	# with the patch
	0:% svn co svn+patched://`pwd`/repos wc | tail -1
	Checked out revision 1.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: passwd file permissions with svn+ssh

Posted by Greg Hudson <gh...@MIT.EDU>.
I agree with Kristian here, and this is probably an oversight on my part
when I wrote the code (although it's been a while).  If the passwd file
is unreadable, svnserve should just fail to authenticate anyone with
passwords, so that the same repository can be used with svn+ssh and
svnserve.

On Tue, 2008-04-29 at 17:55 +1000, Kristian Kauper wrote:
> Sorry! I thought I was replying to the original message thread. Here's 
> Marcus's response, with links to my original email:
> 
> http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=121507
> 
> BTW, I also received this response from David Anderson:
> 
> "The password storage and shell access issues with svnserve is well
> documented. The recommended course of action (if apache+mod_dav_svn is
> not an option) is to configure ssh to allow svn committer users no
> access to the machine other than execution of /usr/bin/svnserve. This
> forces all their interaction with the repository to go through the
> svnserve interface, thus preventing them from accessing anything
> directly (and certainly from getting at the configuration). Another
> recommended policy is to disallow users to set their own password for
> svnserve, but rather to autogenerate it and use the client caching
> mechanism to not have to remember it. This password then offers access
> to nothing other than the version control repository, where any
> unauthorized change can be reverted easily. All these options, and
> more, are discussed in the svn book, at http://svnbook.org ."
> 
> To be honest, however, I don't think these are great suggestions. The 
> first is just impractical in our environment, and I'd assume that many 
> people would want to have a system that allows SSH access for both SVN 
> and shell.
> 
> I just don't get why this is an issue in the first place. Why does the 
> code need to read the passwd file if a user has already authenticated 
> via SSH? I thought that was the point of the SSH access method.
> 
> David says that this issue is well documented, but for the life of me I 
> couldn't find much in the way of formal documentation or mailing list 
> discussions.
> 
> Thanks.
> 
> -K
> 
> Karl Fogel wrote:
> > Kristian Kauper <kk...@au.yahoo-inc.com> writes:
> >> Does anyone know if the patch that Marcus refers to ever made it into
> >> a release?
> >>
> >> Since my original query, I've upgrade to 1.4.4, but this problem still
> >> exists.
> > 
> > Can you point to the original mail in the archives, so people know what
> > you're referring to?  Thanks.
> > 
> > -Karl
> > 
> 
> ---------------------------------------------------------------------
> 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

Re: passwd file permissions with svn+ssh

Posted by Kristian Kauper <kk...@au.yahoo-inc.com>.
Sorry! I thought I was replying to the original message thread. Here's 
Marcus's response, with links to my original email:

http://subversion.tigris.org/servlets/ReadMsg?listName=dev&msgNo=121507

BTW, I also received this response from David Anderson:

"The password storage and shell access issues with svnserve is well
documented. The recommended course of action (if apache+mod_dav_svn is
not an option) is to configure ssh to allow svn committer users no
access to the machine other than execution of /usr/bin/svnserve. This
forces all their interaction with the repository to go through the
svnserve interface, thus preventing them from accessing anything
directly (and certainly from getting at the configuration). Another
recommended policy is to disallow users to set their own password for
svnserve, but rather to autogenerate it and use the client caching
mechanism to not have to remember it. This password then offers access
to nothing other than the version control repository, where any
unauthorized change can be reverted easily. All these options, and
more, are discussed in the svn book, at http://svnbook.org ."

To be honest, however, I don't think these are great suggestions. The 
first is just impractical in our environment, and I'd assume that many 
people would want to have a system that allows SSH access for both SVN 
and shell.

I just don't get why this is an issue in the first place. Why does the 
code need to read the passwd file if a user has already authenticated 
via SSH? I thought that was the point of the SSH access method.

David says that this issue is well documented, but for the life of me I 
couldn't find much in the way of formal documentation or mailing list 
discussions.

Thanks.

-K

Karl Fogel wrote:
> Kristian Kauper <kk...@au.yahoo-inc.com> writes:
>> Does anyone know if the patch that Marcus refers to ever made it into
>> a release?
>>
>> Since my original query, I've upgrade to 1.4.4, but this problem still
>> exists.
> 
> Can you point to the original mail in the archives, so people know what
> you're referring to?  Thanks.
> 
> -Karl
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: passwd file permissions with svn+ssh

Posted by Karl Fogel <kf...@red-bean.com>.
Kristian Kauper <kk...@au.yahoo-inc.com> writes:
> Does anyone know if the patch that Marcus refers to ever made it into
> a release?
>
> Since my original query, I've upgrade to 1.4.4, but this problem still
> exists.

Can you point to the original mail in the archives, so people know what
you're referring to?  Thanks.

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org