You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Sigfred Håversen <bs...@mumak.com> on 2004/10/18 18:03:48 UTC

[PATCH] SSL layer for svnserve

I've added a SSL layer to svnserve, and I would like to have some comments
on the patch. Instructions how to run "make check" with self-signed certificate,
is below.

To start svnserve with SSL enabled :

$ svnserve --daemon --root /path/to/repo \
  --cert-file server_public_certificate \
  --key-file private_key_to_server_certificate

A client will use svn as usual. If the server certificate is
unknown, however, the user will be prompted for acceptance
of it :

$ svn co svn://localhost/repo repo


A SSL enabled svnserve will announce it's SSL functionality as a 
"ssl capability" that is exchanged during the greeting phase between
server and client. After a SSL connection has been successfully setup, 
the usual communication between svnserve and client is now encrypted.

Clients with SSL capability may connect to svnserve that has not
enabled SSL.

A few points to note for a SSL enabled svnserve :

  * svnserve will refuse clients that does not have SSL capability.
  * Since the realm is sendt as part of the greeting, this will not be
    encrypted.
  * The client must at least use version 2 of the svnserve protocol.

Example of information leak (using strings on output from tcpdump on lo0) :

sAri
8N( success ( 2 2 ( ANONYMOUS ) ( edit-pipeline ssl ) ) ) L
8N( 2 ( edit-pipeline ssl ) 43:svn://localhost/repositories/basic_tests-15 ) M
M$T3Q

To avoid sending "svn://localhost/repositories/basic_tests-15" would 
require a change in the version 2 protocoll, or that less information is
sendt upon initial greeting.


The patch uses OpenSSL, and uses a BIO pair to handle the communication
between SSL and the network :

  Subversion  |   TLS-engine
     |        |
     +----------> SSL_operations()
              |     /\    ||
              |     ||    \/
              |   BIO-pair (internal_bio)
     +----------< BIO-pair (network_bio)
     |        |
   socket     |
 
The function writebuf_output in libsvn_ra_svn/marshal.c will use SSL
operations for transferal of data. Similarly for readbuf_input.
For this to work, svn_ra_svn_conn_t has to be extended with relevant
data that the SSL operations needs for each connection.

A so-called SSL context is used when setting up the SSL connection,
and ra_svn_session_baton_t and server_baton_t has a new such member.

On the server side, the SSL setup for each connection is handled
by serve() in svnserve/serve.c. For the client side this is
handled by ra_svn_open() in libsvn_ra_svn/client.c. 

There are several other helper functions for managing the SSL
operations, as well as handling of certificates on the client side.

 
 
The patch was developed on an OpenBSD machine. So other *BSD/Linux 
should be able to build. The enabling of SSL for svnserve should be 
part of the configure, but I'm no configure guru. I quite simply replaced 
in Makefile (on my system) the "LIBS = -lintl" with 
"LIBS = -lintl -lssl -lcrypto" during development.

The "make check BASE_URL=svn://localhost" will work as usual for 
svnserve not using SSL. However, when SSL is enabled, the tests
will hang waiting for user input to accept/reject certificate.

As a temporary solution, I hacked main.py to use /tmp/svnserve_config
as parameter to --config-dir, and autoprop_tests.py to only use
svntest.main.config_dir as config_dir.

For testing using "make check", I've attached two shell scripts :

"myservecheck" generates a self-signed certificate, and
then start a SSL enabled svnserve.

"mycheck" runs "make check", but uses "svn ls" to accept the
server certificate generated by "myservecheck".

With these scripts, all tests in "make check" passes.

Regards,
Sigfred Haversen.

Re: [PATCH] SSL layer for svnserve

Posted by Sigfred Håversen <bs...@mumak.com>.
On Wednesday 20 October 2004 18.46, Garrett Rooney wrote:
> Sigfred Håversen wrote:
> > If the certificate is specified in the repo config, then svnserve does
> > not know if it has a valid certificate when a client connects. All it
> > knows is that it can handle SSL, if needed. The more tricky part is to
> > handle the SSL handshake, and that probably require more communication
> > between client and svnserve before actual SSL handshake. After the
> > greeting, svnserve can check that a certificate is indeed present in the
> > repo, and then load/verify it. At this stage the client and svnserve can
> > continue with the SSL as desribed above. Actually, with this approach
> > svnserve does not need to announce ssl capability at greeting as this
> > will be handled with further handshaking. But this does add complexity,
> > and perhaps a change in the protocoll as well.
>
> But the client already sends the URL in the greeting, and that's all you
> need to find the repository and thus find the repository config file.  I
> don't see why you can't just do that before you start the SSL handshake.
>
> Am I missing something here?
>
> -garrett
>

More with me thinking unclearly, I'm afraid. With both SSL and non-SSL access 
to repo, an extra step needed to be taken in the handshake and that has 
already been done. No need for extra steps as I suggested, as far as I can 
see. The initial announcement from server that SSL might be available is not 
needed, though. svnserve will send ssl or ssl-auth based upon info from repo 
config file.

/Sigfred

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


Re: [PATCH] SSL layer for svnserve

Posted by Travis P <sv...@castle.fastmail.fm>.
On Wednesday 20 October 2004 15.45, Garrett Rooney wrote:
> Sigfred Håversen wrote:
>> I would prefer that svnserve has it's configuration file separate from
>> the repository config file. It think it will be simpler to 
>> administrate,
>> as well as code, to let svnserve have a certificate that is shared 
>> with
>> all the repositories. This does not exclude the possibility of client
>> authentication based upon client certificates, though. The approved
>> client certificates should be per repository, and listed in the
>> repository config file, of course.
>
> I don't see why SSL configuration should be any different from
> svnserve's current auth options, which are per-repository.  I know that
> I personally have multiple repositories served up via svnserve on my
> server, and they have different sets of users and different rules
> regarding authentication.  There doesn't seem to be any reason (IMO) to
> make SSL a special case.

What if someone is serving the same repository from two different 
machines?  (Maybe serving FSFS repository off a high-performance SAN, 
would that work?)   Or two different interfaces on the same 
high-performance machine using a local disk?  (That would probably work 
today with either BDB or FSFS.)

Since server cert is tied to hostname, having this per-repos-config 
seems like it might be problematic.  Each server servicing a particular 
interface will need a different server certificate.  (It seems to me 
that the use of CN=hostname requirement for SSL on the web is limiting 
and that fundamentally, a checking of anything specifically expected 
such as a realm name by a particular CA would be a good 
alternative--just as the server expects to get in a client certificate, 
but I don't know that the available SSL libraries are written to 
support such client-cert<=>client-cert communication.)

Using Apache, we have a per-server config file for this (Apache's own 
config file), so this would work with Apache.

-Travis


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


Re: [PATCH] SSL layer for svnserve

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Sigfred Håversen wrote:

> If the certificate is specified in the repo config, then svnserve does not 
> know if it has a valid certificate when a client connects. All it knows
> is that it can handle SSL, if needed. The more tricky part is to handle
> the SSL handshake, and that probably require more communication between
> client and svnserve before actual SSL handshake. After the greeting, svnserve
> can check that a certificate is indeed present in the repo, and then 
> load/verify it. At this stage the client and svnserve can continue with the
> SSL as desribed above. Actually, with this approach svnserve does not need to
> announce ssl capability at greeting as this will be handled with further 
> handshaking. But this does add complexity, and perhaps a change in the
> protocoll as well.

But the client already sends the URL in the greeting, and that's all you 
need to find the repository and thus find the repository config file.  I 
don't see why you can't just do that before you start the SSL handshake.

Am I missing something here?

-garrett

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

Re: [PATCH] SSL layer for svnserve

Posted by Sigfred Håversen <bs...@mumak.com>.
On Wednesday 20 October 2004 15.45, Garrett Rooney wrote:
> Sigfred Håversen wrote:
> > We could allow non-SSL access for anonymous users, but all other users
> > are required to use SSL. svnserve could send this in the greeting as a
> > new capability "ssl-auth", meaning that anonymous access may or may not
> > be encrypted at the discreetion of the client.
> >
> > So svnserve will send one of two capabilities :
> >
> > ssl  All access must must SSL
> > ssl-auth  All access, except anonymous, must use SSL.
> >
> > The client will then respond with the capability "ssl" for starting the
> > SSL handshake. If no SSL is wanted by the client, then it will not send
> > the ssl capability. Note that a client may very well use SSL even for
> > anonymous access.
> >
> > With the two ssl capabilities, the setup and usage of SSL in svnserve
> > should be fairly easy to understand as well as administrate.
>
> That seems reasonable to me.
>
> >>>I also don't much like the mess of new command-line options.  I'd like
> >>>to either see these moved to the repository config file (although that
> >>>means the greeting will specify the "ssl" capability even if the server
> >>>has configured no certificates, which means the ssl handshake had better
> >>>do something intelligent even if there are no certificates), or see
> >>>svnserve grow a server configuration file before we add this many global
> >>>options.
> >>
> >>Yeah, since the remainder of the svnserve configuration stuff is
> >>
> >>>per-repository it seems like this should be as well.
> >
> > I would prefer that svnserve has it's configuration file separate from
> > the repository config file. It think it will be simpler to administrate,
> > as well as code, to let svnserve have a certificate that is shared with
> > all the repositories. This does not exclude the possibility of client
> > authentication based upon client certificates, though. The approved
> > client certificates should be per repository, and listed in the
> > repository config file, of course.
>
> I don't see why SSL configuration should be any different from
> svnserve's current auth options, which are per-repository.  I know that
> I personally have multiple repositories served up via svnserve on my
> server, and they have different sets of users and different rules
> regarding authentication.  There doesn't seem to be any reason (IMO) to
> make SSL a special case.

If the certificate is specified in the repo config, then svnserve does not 
know if it has a valid certificate when a client connects. All it knows
is that it can handle SSL, if needed. The more tricky part is to handle
the SSL handshake, and that probably require more communication between
client and svnserve before actual SSL handshake. After the greeting, svnserve
can check that a certificate is indeed present in the repo, and then 
load/verify it. At this stage the client and svnserve can continue with the
SSL as desribed above. Actually, with this approach svnserve does not need to
announce ssl capability at greeting as this will be handled with further 
handshaking. But this does add complexity, and perhaps a change in the
protocoll as well.

> If you do want the ability to have a single auth configuration that
> affects all repositories, perhaps we could add it in as a command line
> option (or a global config file or whatever) that can be overridden in
> the individual repositories, but honestly I don't think it's that
> important, the per-repository stuff seems to work just fine now.
>

I think that having two different places for SSL certificates might be 
confusing.

/Sigfred

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


Re: [PATCH] SSL layer for svnserve

Posted by Sigfred Håversen <bs...@mumak.com>.
On Saturday 23 October 2004 17.50, Greg Hudson wrote:
> On Sat, 2004-10-23 at 09:53, Sigfred Håversen wrote:
> > I'm not sure why one would run SSL enabled svnserve and not use a server
> > certificate.
>
> People won't always actively make a choice to run an svnserve built with
> SSL support.  Typically they'll be using whatever a binary packager
> built for them.  If we can make the default communication between
> svnserve and clients more secure, we should at least consider it.
>
> >  We could supply a script that the repo administrator could use
> > to make a certificate for use with svnserve. This is an approach done
> > with several other servers, like courier-imap.
>
> Then you have to get the certificate to the clients (if a client simply
> accepts what the server presents over the ra_svn connection, that's no
> better than D-H).  We're never going to accomplish true security without
> some out-of-band work on the part of the administrator, and many
> administrators aren't going to go to that work.  So it's always worth
> considering what we can do in the keyless case.
>

I'm concerned that we have always to consider two cases of SSL usage in the 
source code : with or without server certificate, along with corresponding 
cases in clients. 

If we could make it easy to the repo administrator to start using SSL with 
certificates, it will help security, even if it's not as automatic as in your 
proposal. For instance, "svnadmin create" can automatically make a 
self-signed certificate that is placed in the repo config directory. These 
pre-made certificates can now be used by svnserve. Granted, most client users 
will just accept this certificate, and not verify the certificate 
fingerprint. The repo administrators that care about this, will have made 
their certificate fingerprint available out-of-band, even if it's just 
posting the fingerprint on a html page.


/Sigfred

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


Re: [PATCH] SSL layer for svnserve

Posted by Greg Hudson <gh...@MIT.EDU>.
On Sat, 2004-10-23 at 09:53, Sigfred Håversen wrote:
> I'm not sure why one would run SSL enabled svnserve and not use a server 
> certificate.

People won't always actively make a choice to run an svnserve built with
SSL support.  Typically they'll be using whatever a binary packager
built for them.  If we can make the default communication between
svnserve and clients more secure, we should at least consider it.

>  We could supply a script that the repo administrator could use 
> to make a certificate for use with svnserve. This is an approach done with 
> several other servers, like courier-imap.

Then you have to get the certificate to the clients (if a client simply
accepts what the server presents over the ra_svn connection, that's no
better than D-H).  We're never going to accomplish true security without
some out-of-band work on the part of the administrator, and many
administrators aren't going to go to that work.  So it's always worth
considering what we can do in the keyless case.


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


Re: [PATCH] SSL layer for svnserve

Posted by Sigfred Håversen <bs...@mumak.com>.
On Wednesday 20 October 2004 22.17, Greg Hudson wrote:
> On Wed, 2004-10-20 at 09:34, Sigfred Håversen wrote:

>
> The issue is that old clients need to be able to access new servers.
> Whether to allow access (anonymous or not) without SSL is a policy
> issue, and shouldn't be hardcoded into our code base.

Understandable, but then I assume this policy will be an option either to 
svnserve, or an option specified in the repo config file.

>
> > We could allow non-SSL access for anonymous users, but all other users
> > are required to use SSL. svnserve could send this in the greeting as a
> > new capability "ssl-auth", meaning that anonymous access may or may not
> > be encrypted at the discreetion of the client.
>
> Capabilities should not be used to express policy, only what the server
> and client are capable of.  If further negotiation is necessary, it
> needs to be done through some other aspect of the protocol--either a new
> field in the greeting, or an interchange before the SSL handshake.

Agreed. 

...
> That model presents an issue for client certificate authentication,
> which needs to happen during an SSL handshake.  Can we re-handshake
> during an authentication challenge?

Yes, it is possible to renegotiate to require to client to use a valid client 
certificate. 

...
>
> In another message:
> > If the certificate is specified in the repo config, then svnserve does
> > not know if it has a valid certificate when a client connects. All it
> > knows is that it can handle SSL, if needed. The more tricky part is to
> > handle the SSL handshake, and that probably require more communication
> > between client and svnserve before actual SSL handshake.
>
> At least at a protocol level, SSL can be used without any certificates,
> in which case it provides confidentiality but no authentication.  Can
> OpenSSL do this?  It might actually be nice if, with no certificate
> configuration whatsoever, ra_svn clients and servers used
> Diffie-Hellman-protected communication, rendering casual network attacks
> impossible.

I'm not sure why one would run SSL enabled svnserve and not use a server 
certificate. We could supply a script that the repo administrator could use 
to make a certificate for use with svnserve. This is an approach done with 
several other servers, like courier-imap.

/Sigfred

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


Re: [PATCH] SSL layer for svnserve

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Sigfred Håversen wrote:

> Is the reason for not wanting SSL in all cases due to a concern for 
> consumption of CPU resources? I can understand that SSL on anonymous access 
> might be too much for a server.

There are at least two reasons I can think of.  First is CPU resources, 
second is increasing the requirements on your users.  If a person only 
wants annonymous access and never needs to commit changes there's no 
reason to require them to have openssl installed.

> We could allow non-SSL access for anonymous users, but all other users are 
> required to use SSL. svnserve could send this in the greeting as a new 
> capability "ssl-auth", meaning that anonymous access may or may not be 
> encrypted at the discreetion of the client.
> 
> So svnserve will send one of two capabilities :
> 
> ssl  All access must must SSL
> ssl-auth  All access, except anonymous, must use SSL.
> 
> The client will then respond with the capability "ssl" for starting the SSL 
> handshake. If no SSL is wanted by the client, then it will not send the ssl 
> capability. Note that a client may very well use SSL even for anonymous 
> access.
> 
> With the two ssl capabilities, the setup and usage of SSL in svnserve should 
> be fairly easy to understand as well as administrate. 

That seems reasonable to me.

>>>I also don't much like the mess of new command-line options.  I'd like
>>>to either see these moved to the repository config file (although that
>>>means the greeting will specify the "ssl" capability even if the server
>>>has configured no certificates, which means the ssl handshake had better
>>>do something intelligent even if there are no certificates), or see
>>>svnserve grow a server configuration file before we add this many global
>>>options.
>>
>>Yeah, since the remainder of the svnserve configuration stuff is
>>
>>>per-repository it seems like this should be as well.
> 
> 
> I would prefer that svnserve has it's configuration file separate from the 
> repository config file. It think it will be simpler to administrate, as well 
> as code, to let svnserve have a certificate that is shared with all the 
> repositories. This does not exclude the possibility of client authentication 
> based upon client certificates, though. The approved client certificates 
> should be per repository, and listed in the repository config file, of 
> course.

I don't see why SSL configuration should be any different from 
svnserve's current auth options, which are per-repository.  I know that 
I personally have multiple repositories served up via svnserve on my 
server, and they have different sets of users and different rules 
regarding authentication.  There doesn't seem to be any reason (IMO) to 
make SSL a special case.

If you do want the ability to have a single auth configuration that 
affects all repositories, perhaps we could add it in as a command line 
option (or a global config file or whatever) that can be overridden in 
the individual repositories, but honestly I don't think it's that 
important, the per-repository stuff seems to work just fine now.

>>>>> * The client must at least use version 2 of the svnserve protocol.
>>>>
> 
> Since the source code says that version 1 should be deprecated, I decided not 
> to implent SSL for that protocol.

Yeah, I don't see any reason to support version 1 anymore, I doubt there 
are a whole lot of people using it, and there's certainly no reason to 
support it for SSL, since anyone who can upgrade to get SSL support 
already has version 2 ;-)

-garrett

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

Re: [PATCH] SSL layer for svnserve

Posted by Greg Hudson <gh...@MIT.EDU>.
On Wed, 2004-10-20 at 09:34, Sigfred Håversen wrote:
> > >>>  * svnserve will refuse clients that does not have SSL capability.
> > >>
> > >>I'm not sure this is the way we should go on this...
> > >
> > > Clearly not.
> 
> Is the reason for not wanting SSL in all cases due to a concern for 
> consumption of CPU resources? I can understand that SSL on anonymous access 
> might be too much for a server.

The issue is that old clients need to be able to access new servers. 
Whether to allow access (anonymous or not) without SSL is a policy
issue, and shouldn't be hardcoded into our code base.

I'm fine with having the connection always use SSL if the client and
server are both capable of it.

> We could allow non-SSL access for anonymous users, but all other users are 
> required to use SSL. svnserve could send this in the greeting as a new 
> capability "ssl-auth", meaning that anonymous access may or may not be 
> encrypted at the discreetion of the client.

Capabilities should not be used to express policy, only what the server
and client are capable of.  If further negotiation is necessary, it
needs to be done through some other aspect of the protocol--either a new
field in the greeting, or an interchange before the SSL handshake.

I don't think further negotiation is necessary, though.  If the server
doesn't want to talk to a non-SSL client, it can generate an error and
close the connection.  If for whatever reason the client doesn't want to
use SSL, it can just pretend it's not capable of it.

Also, the svn client doesn't really know whether it's performing an
anonymous access or not.  It generally attempts to perform whatever
operation it's performing anonymously, and then responds to an
authentication challenge when the server asks.  That might happen as
soon as the client connects (if authentication is required for read
access), or only after the client tries to issue a write operation.

That model presents an issue for client certificate authentication,
which needs to happen during an SSL handshake.  Can we re-handshake
during an authentication challenge?

> I would prefer that svnserve has it's configuration file separate from the 
> repository config file. It think it will be simpler to administrate, as well 
> as code, to let svnserve have a certificate that is shared with all the 
> repositories. This does not exclude the possibility of client authentication 
> based upon client certificates, though. The approved client certificates 
> should be per repository, and listed in the repository config file, of 
> course.

One of my concerns is that some day, svnserve may support a more
flexible mapping of paths to repositories than the current
repository-root option.  If the new method encompasses the server name
as well as the pathname, then we may need different server certificates
depending on which repository is being accessed.

Travis raises the interesting point that the same repository might be
accessible through two hostnames, in which case repository configuration
won't do.  I don't know how common that situation will be.

I guess the way I lean right now is:

  - The server certificate should be a global option, although at some
point (possibly in the future) it should be allowed to vary depending on
the hostname specified in the URL provided by the client.

  - The approved client-certificates should be per-repository, as you
say.

In another message:

> If the certificate is specified in the repo config, then svnserve does
> not know if it has a valid certificate when a client connects. All it
> knows is that it can handle SSL, if needed. The more tricky part is to
> handle the SSL handshake, and that probably require more communication
> between client and svnserve before actual SSL handshake.

At least at a protocol level, SSL can be used without any certificates,
in which case it provides confidentiality but no authentication.  Can
OpenSSL do this?  It might actually be nice if, with no certificate
configuration whatsoever, ra_svn clients and servers used
Diffie-Hellman-protected communication, rendering casual network attacks
impossible.

(Of course, if you're running a server on an underpowered box, that
default might not work so well for you.)


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


Re: [PATCH] SSL layer for svnserve

Posted by Sigfred Håversen <bs...@mumak.com>.
On Tuesday 19 October 2004 04.58, Garrett Rooney wrote:
> Greg Hudson wrote:
> > I hope to review this stuff more closely soon.  mbk proposed creating a
> > branch for it; that way we can commit it without addressing all of the
> > build system and configuration issues immediately.
>
> That seems like a good idea.
>
> > On Mon, 2004-10-18 at 16:00, Garrett Rooney wrote:
> >>>  * svnserve will refuse clients that does not have SSL capability.
> >>
> >>I'm not sure this is the way we should go on this...
> >
> > Clearly not.

Is the reason for not wanting SSL in all cases due to a concern for 
consumption of CPU resources? I can understand that SSL on anonymous access 
might be too much for a server.

We could allow non-SSL access for anonymous users, but all other users are 
required to use SSL. svnserve could send this in the greeting as a new 
capability "ssl-auth", meaning that anonymous access may or may not be 
encrypted at the discreetion of the client.

So svnserve will send one of two capabilities :

ssl  All access must must SSL
ssl-auth  All access, except anonymous, must use SSL.

The client will then respond with the capability "ssl" for starting the SSL 
handshake. If no SSL is wanted by the client, then it will not send the ssl 
capability. Note that a client may very well use SSL even for anonymous 
access.

With the two ssl capabilities, the setup and usage of SSL in svnserve should 
be fairly easy to understand as well as administrate. 

> >
> > I also don't much like the mess of new command-line options.  I'd like
> > to either see these moved to the repository config file (although that
> > means the greeting will specify the "ssl" capability even if the server
> > has configured no certificates, which means the ssl handshake had better
> > do something intelligent even if there are no certificates), or see
> > svnserve grow a server configuration file before we add this many global
> > options.
> Yeah, since the remainder of the svnserve configuration stuff is
>> per-repository it seems like this should be as well.

I would prefer that svnserve has it's configuration file separate from the 
repository config file. It think it will be simpler to administrate, as well 
as code, to let svnserve have a certificate that is shared with all the 
repositories. This does not exclude the possibility of client authentication 
based upon client certificates, though. The approved client certificates 
should be per repository, and listed in the repository config file, of 
course.


> >>>  * Since the realm is sendt as part of the greeting, this will not be
> >>>    encrypted.
> >>
> >>That's unfortunate.  Do you see a good way to change the protocol to
> >>avoid the leakage?
> >
> > I'm not quite sure why the word "realm" is being used here.  The only
> > "realm" involved in the svn protocol is the authentication realm, which
> > is not sent as part of the initial greeting.

I meant the client URL.

> >
> > If you mean the client URL, there is actually a reason to send this in
> > plain text: you may want to use different certificates (client and
> > server) for different repositories.  Think of the client URL (or at
> > least the repository part of it) as an extension of the hostname and
> > port number.

Yes, with client certificates this would be usefull.

> > If we decide that it's more important to hide the client URL than it is
> > to allow repository-dependent SSL negotiation parameters, then we can do
> > it, but it's a little tricky.  It would go like this: the client sees
> > the "ssl" capability in the server and knows it's going to respond with
> > an "ssl" capability, so it knows there will be an SSL handshake.  So it
> > sends a placeholder in the client URL, and then sends the real client
> > URL as the first thing after the SSL security layer has been negotiated.
>
> Yeah, I can see how that could work, but I'm not sure it's worth the
> complexity.

Agreed. 

> >>>  * The client must at least use version 2 of the svnserve protocol.
> >>

Since the source code says that version 1 should be deprecated, I decided not 
to implent SSL for that protocol.

/Sigfred

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

Re: [PATCH] SSL layer for svnserve

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Greg Hudson wrote:
> I hope to review this stuff more closely soon.  mbk proposed creating a
> branch for it; that way we can commit it without addressing all of the
> build system and configuration issues immediately.

That seems like a good idea.

> On Mon, 2004-10-18 at 16:00, Garrett Rooney wrote:
> 
>>>  * svnserve will refuse clients that does not have SSL capability.
>>
>>I'm not sure this is the way we should go on this...
> 
> 
> Clearly not.
> 
> I also don't much like the mess of new command-line options.  I'd like
> to either see these moved to the repository config file (although that
> means the greeting will specify the "ssl" capability even if the server
> has configured no certificates, which means the ssl handshake had better
> do something intelligent even if there are no certificates), or see
> svnserve grow a server configuration file before we add this many global
> options.

Yeah, since the remainder of the svnserve configuration stuff is 
per-repository it seems like this should be as well.

>>>  * Since the realm is sendt as part of the greeting, this will not be
>>>    encrypted.
>>
>>That's unfortunate.  Do you see a good way to change the protocol to 
>>avoid the leakage?
> 
> 
> I'm not quite sure why the word "realm" is being used here.  The only
> "realm" involved in the svn protocol is the authentication realm, which
> is not sent as part of the initial greeting.
> 
> If you mean the client URL, there is actually a reason to send this in
> plain text: you may want to use different certificates (client and
> server) for different repositories.  Think of the client URL (or at
> least the repository part of it) as an extension of the hostname and
> port number.
> 
> If we decide that it's more important to hide the client URL than it is
> to allow repository-dependent SSL negotiation parameters, then we can do
> it, but it's a little tricky.  It would go like this: the client sees
> the "ssl" capability in the server and knows it's going to respond with
> an "ssl" capability, so it knows there will be an SSL handshake.  So it
> sends a placeholder in the client URL, and then sends the real client
> URL as the first thing after the SSL security layer has been negotiated.

Yeah, I can see how that could work, but I'm not sure it's worth the 
complexity.

>>>  * The client must at least use version 2 of the svnserve protocol.
>>
>>That's perfectly fine with me ;-)
> 
> 
> I'm not sure what this means.  If we're going to completely drop support
> for 0.32 clients, that's probably okay, but I don't know that there's a
> good reason for it.  If this just means that only version 2 clients can
> support SSL, that's obviously fine.

I'm pretty sure (judging from the code) that it does mean only version 2 
clients can support SSL.

-garrett

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

Re: [PATCH] SSL layer for svnserve

Posted by Greg Hudson <gh...@MIT.EDU>.
I hope to review this stuff more closely soon.  mbk proposed creating a
branch for it; that way we can commit it without addressing all of the
build system and configuration issues immediately.

On Mon, 2004-10-18 at 16:00, Garrett Rooney wrote:
> >   * svnserve will refuse clients that does not have SSL capability.
> 
> I'm not sure this is the way we should go on this...

Clearly not.

I also don't much like the mess of new command-line options.  I'd like
to either see these moved to the repository config file (although that
means the greeting will specify the "ssl" capability even if the server
has configured no certificates, which means the ssl handshake had better
do something intelligent even if there are no certificates), or see
svnserve grow a server configuration file before we add this many global
options.

> >   * Since the realm is sendt as part of the greeting, this will not be
> >     encrypted.
> 
> That's unfortunate.  Do you see a good way to change the protocol to 
> avoid the leakage?

I'm not quite sure why the word "realm" is being used here.  The only
"realm" involved in the svn protocol is the authentication realm, which
is not sent as part of the initial greeting.

If you mean the client URL, there is actually a reason to send this in
plain text: you may want to use different certificates (client and
server) for different repositories.  Think of the client URL (or at
least the repository part of it) as an extension of the hostname and
port number.

If we decide that it's more important to hide the client URL than it is
to allow repository-dependent SSL negotiation parameters, then we can do
it, but it's a little tricky.  It would go like this: the client sees
the "ssl" capability in the server and knows it's going to respond with
an "ssl" capability, so it knows there will be an SSL handshake.  So it
sends a placeholder in the client URL, and then sends the real client
URL as the first thing after the SSL security layer has been negotiated.

> >   * The client must at least use version 2 of the svnserve protocol.
> 
> That's perfectly fine with me ;-)

I'm not sure what this means.  If we're going to completely drop support
for 0.32 clients, that's probably okay, but I don't know that there's a
good reason for it.  If this just means that only version 2 clients can
support SSL, that's obviously fine.


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

Re: [PATCH] SSL layer for svnserve

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
I haven't had a chance to look at this patch yet, but just off the top 
of my head I had a few comments...

> A few points to note for a SSL enabled svnserve :
> 
>   * svnserve will refuse clients that does not have SSL capability.

I'm not sure this is the way we should go on this...  It would be nice 
to be able to allow people to access the repository via both non-ssl and 
ssl connections, but give the server that information so they can be 
treated differently.  For example we might want to refuse to allow 
commits over non-ssl connections, but allow them for ssl connections. 
Just refusing non-ssl connections seems a bit limiting...

>   * Since the realm is sendt as part of the greeting, this will not be
>     encrypted.

That's unfortunate.  Do you see a good way to change the protocol to 
avoid the leakage?

>   * The client must at least use version 2 of the svnserve protocol.

That's perfectly fine with me ;-)

-garrett

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

Re: [PATCH] SSL layer for svnserve

Posted by Sigfred Håversen <bs...@mumak.com>.
On Tuesday 19 October 2004 14.46, Joe Orton wrote:
> On Tue, Oct 19, 2004 at 02:12:34PM +0200, Sigfred Håversen wrote:
> > Here is the corresponding from my patch :
> >
> > +/* Format an ASN1 time to a string.
> > + * Adapted from Neon library.
>
> Your words not mine.  It looks like you've copied it then changed it.
> Or was the function name mere coincidence?  Keep the copyright notices
> obey the license if you copied the code.
>
> joe

The function name was to give credit to the Neon library for a
trivial function used to fill out the cert_info. I did not try to hide 
anything, did I? 

Is there anything else in my patch that you are unhappy about?
You do hint that there are other things as well. The place to look 
for discrepancies is in fill_server_cert_info(), as it in this code I fill out
cert_info so that it contains the same as the one produced from
indirectly from Neon. Hint, hint, look at the switch-statement. 
You'll find similar tests in the Postfix TLS patch as well.

/Sigfred



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


Re: [PATCH] SSL layer for svnserve

Posted by Sigfred Håversen <bs...@mumak.com>.
On Tuesday 19 October 2004 22.49, Joe Orton wrote:
> On Tue, Oct 19, 2004 at 08:18:01PM +0200, Sigfred Håversen wrote:
> > On Tuesday 19 October 2004 18.17, Joe Orton wrote:
> > > Sorry, I didn't mean to start a big argument.  If you say you didn't
> > > copy the code, then I shall shut up, it just look{ed,s} like you did
> > > copy it but didn't understand the implications.
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> > > For additional commands, e-mail: dev-help@subversion.tigris.org
> >
> > I asked you in another post what other part of the patch you are unhappy
> > about. You did not answer. Instead you made new insuations, despite me
> > giving you a link for the Postfix TLS patch. The link is even in my
> > patch. Several places.
>
> Please accept my apologies, Sigfred, I was not trying to insult you.  I
> looked at the patch, I saw the "Adopted from neon" comment, and I saw
> the two functions beneath it with names which are the same as the
> functions in neon, with code which looked pretty similar to the code in
> neon.  I jumped to conclusions, but clearly I was wrong and spoke out of
> turn -- sorry.
>
> joe

Thank you, and I appreciate that this misunderstanding is resolved. No hard 
feelings on my part.

Regards,
Sigfred.


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


Re: [PATCH] SSL layer for svnserve

Posted by Joe Orton <jo...@manyfish.co.uk>.
On Tue, Oct 19, 2004 at 08:18:01PM +0200, Sigfred Håversen wrote:
> On Tuesday 19 October 2004 18.17, Joe Orton wrote:
> > Sorry, I didn't mean to start a big argument.  If you say you didn't
> > copy the code, then I shall shut up, it just look{ed,s} like you did
> > copy it but didn't understand the implications.
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> > For additional commands, e-mail: dev-help@subversion.tigris.org
> 
> I asked you in another post what other part of the patch you are unhappy 
> about. You did not answer. Instead you made new insuations, despite me giving 
> you a link for the Postfix TLS patch. The link is even in my patch. Several 
> places.

Please accept my apologies, Sigfred, I was not trying to insult you.  I
looked at the patch, I saw the "Adopted from neon" comment, and I saw
the two functions beneath it with names which are the same as the
functions in neon, with code which looked pretty similar to the code in
neon.  I jumped to conclusions, but clearly I was wrong and spoke out of
turn -- sorry.

joe

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

Re: [PATCH] SSL layer for svnserve

Posted by Sigfred Håversen <bs...@mumak.com>.
On Tuesday 19 October 2004 18.17, Joe Orton wrote:
> Sorry, I didn't mean to start a big argument.  If you say you didn't
> copy the code, then I shall shut up, it just look{ed,s} like you did
> copy it but didn't understand the implications.
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org

I asked you in another post what other part of the patch you are unhappy 
about. You did not answer. Instead you made new insuations, despite me giving 
you a link for the Postfix TLS patch. The link is even in my patch. Several 
places.

You, as the author of Neon, knows very well that the svnserve SSL-patch 
approach using BIO pairs to handle the communication between Subversion/SSL 
and the network did not originate from Neon. In fact, it appears you don't 
use it at all :

snorre$ pwd
/usr/ports/net/neon/w-neon-0.24.7/neon-0.24.7/src
snorre$ grep -IR BIO_new_bio_pair .
snorre$ grep -IR SSL_set_bio .
snorre$

You know very well that the function verify_hostname() did not originate from 
Neon. Still, you claim ownership to a function that verify_hostname() uses, 
namely match_hostname (that is in the Postfix TLS patch, rewritten by me). 
The only thing in common with your match_hostname is the name. That did not 
stop you from counting the lines, though, and post it. You managed to count 
to 15, perhaps you might reduce it?

You know very well that the major SSL functions (network_biopair_interop(), 
do_ssl_operation()) did not originate from Neon. 

In fact, the entire approach of introducing SSL to svnserve is very unlike 
Neon. Please feel free to contradict me.

Despite of all this, you don't hesitate to post to mailinglist instead of 
asking me for clarification first. You purposely did a character 
assassination for this :

+/* Format an ASN1 time to a string.
+ * Adapted from Neon library. 
+ */
+static svn_boolean_t asn1time_to_string(ASN1_TIME *tm, char *buffer, 
+                                        apr_size_t len)
+{
+  int num_read;
+  svn_boolean_t OK = FALSE;
+  BIO *bio = BIO_new(BIO_s_mem());
+  if (bio) 
+    {
+      if (ASN1_TIME_print(bio, tm) && len > 1)
+        {
+          num_read = BIO_read(bio, buffer, len-1);
+          if (num_read > 1) 
+            {
+              buffer[num_read] = '\0';
+              OK = TRUE;
+            }
+          else
+            OK = FALSE;
+        }
+      BIO_free(bio);
+    }
+  return OK;
+}

in contrast to your code

/* Format an ASN1 time to a string. 'buf' must be at least of size
 * 'NE_SSL_VDATELEN'. */
static void asn1time_to_string(ASN1_TIME *tm, char *buf)
{
    BIO *bio;

    strncpy(buf, _("[invalid date]"), NE_SSL_VDATELEN-1);

    bio = BIO_new(BIO_s_mem());
    if (bio) {
        if (ASN1_TIME_print(bio, tm))
            BIO_read(bio, buf, NE_SSL_VDATELEN-1);
        BIO_free(bio);
    }
}


/Sigfred



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

Re: [PATCH] SSL layer for svnserve

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
Sigfred Håversen wrote:

> I really don't understand your attitude. Are you mad because I did not use 
> Neon?

He's mad because there are several functions in your patch that seem to 
have originated in Neon (you even say that in the comments).  You're 
trying to contribute them to Subversion, which is licensed under a 
BSDish type license and copyright CollabNet, where in Neon they are 
under the LGPL and the copyright is held by Joe.  You can't just take 
code from an LGPL project and relicense it without asking permission 
first, which is what you seem to have done.  Just leaving comments in 
the code that says "this originated in Neon" is not enough.

-garrett

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

Re: [PATCH] SSL layer for svnserve

Posted by Joe Orton <jo...@manyfish.co.uk>.
Sorry, I didn't mean to start a big argument.  If you say you didn't
copy the code, then I shall shut up, it just look{ed,s} like you did
copy it but didn't understand the implications.

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

Re: [PATCH] SSL layer for svnserve

Posted by Sigfred Håversen <bs...@mumak.com>.
On Tuesday 19 October 2004 17.38, Joe Orton wrote:
> On Tue, Oct 19, 2004 at 11:15:14AM -0400, Greg Hudson wrote:
> > On Tue, 2004-10-19 at 08:46, Joe Orton wrote:
> > > On Tue, Oct 19, 2004 at 02:12:34PM +0200, Sigfred Håversen wrote:
> > > > Here is the corresponding from my patch :
> > > >
> > > > +/* Format an ASN1 time to a string.
> > > > + * Adapted from Neon library.
> > >
> > > Your words not mine.  It looks like you've copied it then changed it.
> > > Or was the function name mere coincidence?  Keep the copyright notices
> > > obey the license if you copied the code.
> >
> > Maybe there's more code at issue than you've brought up, but I don't
> > think the five lines of neon's asn1time_to_string() really rise to the
> > level of work where copyright law, or the associated ethical
> > considerations, come into play.  It's like saying you have a copyright
>
> The presence of that and a match_hostname function, with the "adapted
> from neon" comment above looks like a tacit admission when the code was
> copied and munged without bothering with the whole copyright business.
>
> > (I'm not a lawyer.  But note that, to the best of my knowledge, the FSF
> > doesn't require copyright assignments for patches smaller than 20
> > lines.)
>
> 10 lines.
>

Do you have some trademark on the function name "match_hostname"? Did you not 
understand my other postings to the list? 

Let me spell it out for you once again. The base for my patch using OpenSSL, 
is the Postfix TLS patch by Lutz Jänicke that you can download at
http://www.aet.tu-cottbus.de/personen/jaenicke/postfix_tls/

In that patch there is a function called match_hostname. Go check for 
yourself. Go read that patch, and then compare. I re-implemented that 
functionality of wild card matching. There are functions for that in apr.

I really don't understand your attitude. Are you mad because I did not use 
Neon?

/Sigfred


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


Re: [PATCH] SSL layer for svnserve

Posted by Jani Averbach <ja...@jaa.iki.fi>.
On 2004-10-19 16:38+0100, Joe Orton wrote:
>
> copied and munged without bothering with the whole copyright business.
>

The case is that Subversion itself is breaking Neon's LGLP, isn't it?
http://www.gnu.org/copyleft/lgpl.html

5. ...  However, linking a "work that uses the Library" with the
   Library creates an executable that is a derivative of the Library
   (because it contains portions of the Library), rather than a "work
   that uses the library". The executable is therefore covered by this
   License. Section 6 states terms for distribution of such
   executables. ...

6. ... If the work during execution displays copyright notices, you
   must include the copyright notice for the Library among them, as
   well as a reference directing the user to the copy of this
   License. Also, you must do one of these things: ...

Similar thing happens with OpenSSL:
http://www.openssl.org/source/license.html

 * 6. Redistributions of any form whatsoever must retain the following
 *    acknowledgment:
 *    "This product includes software developed by the OpenSSL Project
 *    for use in the OpenSSL Toolkit (http://www.openssl.org/)"

But I don't know hows resposibility is it to produce that, should there be a
hook in the neon, which will print that out, if neon is compiled with
OpenSSL?

svn --version
svn, version 1.2.0 (dev build)
   compiled Oct 18 2004, 06:54:11

Copyright (C) 2000-2004 CollabNet.
Subversion is open source software, see http://subversion.tigris.org/
This product includes software developed by CollabNet (http://www.Collab.Net/).

The following repository access (RA) modules are available:

* ra_dav : Module for accessing a repository via WebDAV (DeltaV)
  protocol.
  - handles 'http' schema
  - handles 'https' schema
* ra_local : Module for accessing a repository on local disk.
  - handles 'file' schema
* ra_svn : Module for accessing a repository using the svn network
  protocol.
  - handles 'svn' schema

I think this should be fixed, because we are using our own advertise
clause in the CollabNet license, and we are printing it with
svn --version.

BR, Jani

-- 
Jani Averbach

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

Re: [PATCH] SSL layer for svnserve

Posted by Joe Orton <jo...@manyfish.co.uk>.
On Tue, Oct 19, 2004 at 04:38:10PM +0100, Joe Orton wrote:
> On Tue, Oct 19, 2004 at 11:15:14AM -0400, Greg Hudson wrote:
> > On Tue, 2004-10-19 at 08:46, Joe Orton wrote:
> > (I'm not a lawyer.  But note that, to the best of my knowledge, the FSF
> > doesn't require copyright assignments for patches smaller than 20
> > lines.)
> 
> 10 lines.

On average we were right :) "around 15"...

http://www.gnu.org/prep/maintain/html_node/Legally-Significant.html#Legally-Significant

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

Re: [PATCH] SSL layer for svnserve

Posted by Joe Orton <jo...@manyfish.co.uk>.
On Tue, Oct 19, 2004 at 11:15:14AM -0400, Greg Hudson wrote:
> On Tue, 2004-10-19 at 08:46, Joe Orton wrote:
> > On Tue, Oct 19, 2004 at 02:12:34PM +0200, Sigfred Håversen wrote:
> > > Here is the corresponding from my patch :
> > > 
> > > +/* Format an ASN1 time to a string.
> > > + * Adapted from Neon library. 
> > 
> > Your words not mine.  It looks like you've copied it then changed it. 
> > Or was the function name mere coincidence?  Keep the copyright notices
> > obey the license if you copied the code.
> 
> Maybe there's more code at issue than you've brought up, but I don't
> think the five lines of neon's asn1time_to_string() really rise to the
> level of work where copyright law, or the associated ethical
> considerations, come into play.  It's like saying you have a copyright

The presence of that and a match_hostname function, with the "adapted
from neon" comment above looks like a tacit admission when the code was
copied and munged without bothering with the whole copyright business.

> (I'm not a lawyer.  But note that, to the best of my knowledge, the FSF
> doesn't require copyright assignments for patches smaller than 20
> lines.)

10 lines.

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

Re: [PATCH] SSL layer for svnserve

Posted by Greg Hudson <gh...@MIT.EDU>.
On Tue, 2004-10-19 at 08:46, Joe Orton wrote:
> On Tue, Oct 19, 2004 at 02:12:34PM +0200, Sigfred Håversen wrote:
> > Here is the corresponding from my patch :
> > 
> > +/* Format an ASN1 time to a string.
> > + * Adapted from Neon library. 
> 
> Your words not mine.  It looks like you've copied it then changed it. 
> Or was the function name mere coincidence?  Keep the copyright notices
> obey the license if you copied the code.

Maybe there's more code at issue than you've brought up, but I don't
think the five lines of neon's asn1time_to_string() really rise to the
level of work where copyright law, or the associated ethical
considerations, come into play.  It's like saying you have a copyright
on the five lines of code to initialize an XML parser and feed it some
data.  Even if you have a strong indication that someone looked at your
code to find out how to do it, that doesn't mean you have a complaint.

(I'm not a lawyer.  But note that, to the best of my knowledge, the FSF
doesn't require copyright assignments for patches smaller than 20
lines.)


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


Re: [PATCH] SSL layer for svnserve

Posted by Joe Orton <jo...@manyfish.co.uk>.
On Tue, Oct 19, 2004 at 02:12:34PM +0200, Sigfred Håversen wrote:
> Here is the corresponding from my patch :
> 
> +/* Format an ASN1 time to a string.
> + * Adapted from Neon library. 

Your words not mine.  It looks like you've copied it then changed it. 
Or was the function name mere coincidence?  Keep the copyright notices
obey the license if you copied the code.

joe

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

Re: [PATCH] SSL layer for svnserve

Posted by Sigfred Håversen <bs...@mumak.com>.
On Tuesday 19 October 2004 11.54, Joe Orton wrote:
> On Mon, Oct 18, 2004 at 08:03:48PM +0200, Sigfred Håversen wrote:
> > I've added a SSL layer to svnserve, and I would like to have some
> > comments on the patch. Instructions how to run "make check" with
> > self-signed certificate, is below.
>
> Bits of this code do look like they have been copied from neon e.g.
> asn1time_to_string, but you have stripped the copyright notices.
>
> joe
>

I thought I was I giving due credit as to whose approach/code I used.

Here is the code from ne_openssl.c :

/* Format an ASN1 time to a string. 'buf' must be at least of size
 * 'NE_SSL_VDATELEN'. */
static void asn1time_to_string(ASN1_TIME *tm, char *buf)
{
    BIO *bio;

    strncpy(buf, _("[invalid date]"), NE_SSL_VDATELEN-1);

    bio = BIO_new(BIO_s_mem());
    if (bio) {
        if (ASN1_TIME_print(bio, tm))
            BIO_read(bio, buf, NE_SSL_VDATELEN-1);
        BIO_free(bio);
    }
}

Here is the corresponding from my patch :

+/* Format an ASN1 time to a string.
+ * Adapted from Neon library. 
+ */
+static svn_boolean_t asn1time_to_string(ASN1_TIME *tm, char *buffer, 
+                                        apr_size_t len)
+{
+  int num_read;
+  svn_boolean_t OK = FALSE;
+  BIO *bio = BIO_new(BIO_s_mem());
+  if (bio) 
+    {
+      if (ASN1_TIME_print(bio, tm) && len > 1)
+        {
+          num_read = BIO_read(bio, buffer, len-1);
+          if (num_read > 1) 
+            {
+              buffer[num_read] = '\0';
+              OK = TRUE;
+            }
+          else
+            OK = FALSE;
+        }
+      BIO_free(bio);
+    }
+  return OK;
+}

So, from Neon's asn1time_to_string() I saw that I could use a BIO_s_mem to
actually convert the time to ascii format. The code uses Neon's approach, but with
error handling added. Actually, using a BIO to convert ASN1_TIME to a string is
commonly used, as a bit of Googling shows :

http://archives.seul.org/mixminion/cvs/Apr-2004/msg00019.html
http://www.mail-archive.com/openssl-users@openssl.org/msg37340.html

In this I thought I gave due credit to Neon. Do you still think
this is not the case?

You will also note other similarities with Neon in fill_server_cert_info().
But how can it not be when using SSL functions to get dates, or
fill out the other fields in svn_auth_ssl_server_cert_info_t *cert_info ?
Even using a switch statement after 
"verify_result = SSL_get_verify_result(sess->conn->ssl);" are common,
and in this I again choosed to test for the same values as in Neon, for
compatability reasons.

Now, for the non-trivial parts, I use the Postfix TLS patch by  Lutz Jaenicke
(http://www.aet.tu-cottbus.de/personen/jaenicke/postfix_tls/) as a model of
how to work with BIO pairs. As it is, he uses a free license :

"License
This software is free. You can do with it whatever you want. I would however
kindly ask you to acknowledge the use of this package, if you are going use it 
in your software, which you might be going to distribute. I would also like to 
receive a note if you are a satisfied user :-)"

I did send an e-mail :-) And acknowledge it this way, even though
I've mostly rewritten it.

"
+ * Adapted from network_biopair_interop() in postfixtls patch by Lutz Jaenicke
+ * at http://www.aet.tu-cottbus.de/personen/jaenicke/postfix_tls/
+ */
"

/Sigfred

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


Re: [PATCH] SSL layer for svnserve

Posted by kf...@collab.net.
Sigfred Håversen <bs...@mumak.com> writes:
> that uses Neon, even though I credit the Neon library. Then he makes
> more insuations that he can't back up, but now he appears to retract
> some of them.
> 
> All in all, I feel quite disgusted.

Sigfred, it sounds like Joe just misunderstood what you did -- but he
gracefully apologized once it was clarified.  That's all a person can
do.

I think the right reaction would be to return the favor, instead of
escalating :-).

Just my $0.02,
-Karl

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


Re: [PATCH] SSL layer for svnserve

Posted by "C. Michael Pilato" <cm...@collab.net>.
Sigfred Håversen <bs...@mumak.com> writes:

> The author of Neon knows very well that the svnserve SSL patch is
> not based upon Neon. The entire approach to SSL communication
> between Subversion/SSL and network is different. Despite this, the
> first thing he does is to post to a mailinglist (and just CC me)
> over stripping copyright from a trivial function for interopability
> with dav code that uses Neon, even though I credit the Neon
> library. Then he makes more insuations that he can't back up, but
> now he appears to retract some of them.
> 
> All in all, I feel quite disgusted.

There seem to be some mails crossing each other on the wires.  At any
rate, as a disinterested third party, I'm seeing nothing but
disconnect here.  

May I suggest that all parties involved take a step away from their
computers for an hour or so, and when they return, read all mails in
all related threads without responding to any of them until they've
all been consumed and weighed?

I suspect what's going on here is that a simple mistake (or two, or
ten) has been made, and that none of it really justifies either the
level of bandwidth consumed, or the elevation of temperatures, present
on this list today.

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


Re: [PATCH] SSL layer for svnserve

Posted by Sigfred Håversen <bs...@mumak.com>.
On Tuesday 19 October 2004 20.17, Justin Erenkrantz wrote:
> --On Tuesday, October 19, 2004 6:49 PM +0200 Sigfred Håversen
>
> <bs...@mumak.com> wrote:
> > Subversion uses the Apache license, while Neon is essensially a LGPL
> > layer on  top of OpenSSL. This was a major reason for not using Neon in
> > my patch, as  well as not introduce an extra library (Neon, in this
> > case). Besides, using a  BIO pair to handle the communication between
> > svnserve/SSL and the network  made implementation much easier.
>
> But, we already depend on Neon for ra_dav support on the client end.  So,
> the svnserve SSL layer might only be enabled when neon is found.  That
> seems reasonable to me instead of playing games with the copyright
> restrictions Joe has on neon.  -- justin
>

The author of Neon knows very well that the svnserve SSL patch is not based 
upon Neon. The entire approach to SSL communication between Subversion/SSL 
and network is different. Despite this, the first thing he does is to post to 
a mailinglist (and just CC me) over stripping copyright from a trivial 
function for interopability with dav code that uses Neon, even though I 
credit the Neon library. Then he makes more insuations that he can't back up, 
but now he appears to retract some of them. 

All in all, I feel quite disgusted.

/Sigfred


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


Re: [PATCH] SSL layer for svnserve

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Tuesday, October 19, 2004 6:49 PM +0200 Sigfred Håversen 
<bs...@mumak.com> wrote:

> Subversion uses the Apache license, while Neon is essensially a LGPL
> layer on  top of OpenSSL. This was a major reason for not using Neon in
> my patch, as  well as not introduce an extra library (Neon, in this
> case). Besides, using a  BIO pair to handle the communication between
> svnserve/SSL and the network  made implementation much easier.

But, we already depend on Neon for ra_dav support on the client end.  So, 
the svnserve SSL layer might only be enabled when neon is found.  That 
seems reasonable to me instead of playing games with the copyright 
restrictions Joe has on neon.  -- justin

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


Re: [PATCH] SSL layer for svnserve

Posted by Sigfred Håversen <bs...@mumak.com>.
On Tuesday 19 October 2004 18.00, Mark Benedetto King wrote:
> On Tue, Oct 19, 2004 at 10:54:18AM +0100, Joe Orton wrote:
> > On Mon, Oct 18, 2004 at 08:03:48PM +0200, Sigfred H??versen wrote:
> > > I've added a SSL layer to svnserve, and I would like to have some
> > > comments on the patch. Instructions how to run "make check" with
> > > self-signed certificate, is below.
> >
> > Bits of this code do look like they have been copied from neon e.g.
> > asn1time_to_string, but you have stripped the copyright notices.
> >
> > joe
>
> In reviewing this patch, it appears that several pieces of
> its functionality overlap with neon's SSL functionality.
>
> I think it would be best for Subversion to have only one
> implementation of things like asn1time_to_string, hostname/certificate
> matching, etc, so that they can be performed consistently.
>
> What do you think the best way to accomplish that would be?
>
>
> --ben
>

Subversion uses the Apache license, while Neon is essensially a LGPL layer on 
top of OpenSSL. This was a major reason for not using Neon in my patch, as 
well as not introduce an extra library (Neon, in this case). Besides, using a 
BIO pair to handle the communication between svnserve/SSL and the network 
made implementation much easier.

/Sigfred


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

Re: [PATCH] SSL layer for svnserve

Posted by Mark Benedetto King <mb...@lowlatency.com>.
On Tue, Oct 19, 2004 at 10:54:18AM +0100, Joe Orton wrote:
> On Mon, Oct 18, 2004 at 08:03:48PM +0200, Sigfred H??versen wrote:
> > I've added a SSL layer to svnserve, and I would like to have some comments
> > on the patch. Instructions how to run "make check" with self-signed certificate,
> > is below.
> 
> Bits of this code do look like they have been copied from neon e.g. 
> asn1time_to_string, but you have stripped the copyright notices.
> 
> joe
> 

In reviewing this patch, it appears that several pieces of
its functionality overlap with neon's SSL functionality.

I think it would be best for Subversion to have only one
implementation of things like asn1time_to_string, hostname/certificate
matching, etc, so that they can be performed consistently.

What do you think the best way to accomplish that would be?


--ben


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

Re: Copied code report [was: Re: [PATCH] SSL layer for svnserve]

Posted by Greg Hudson <gh...@MIT.EDU>.
On Tue, 2004-10-19 at 06:36, Daniel Patterson wrote:
> Found 33 duplicate lines in the following files:
>   Between lines 608 and 672 in subversion/libsvn_repos/delta.c
>   Between lines 420 and 483 in subversion/libsvn_repos/reporter.c
> 
>    looks like someone moved the function but forgot to remove it.

The code in reporter.c is kind of a generalization of the code in
delta.c, but it also removed a bit of functionality (you can't use the
reporter.c code to diff a transaction against a transaction, only a
revision against a report-of-revisions).  So it will take some tricky
design work to be able to combine them again.  Sure, we could turn
compare_files() into a shared function since it's identical, but that
would be a drop in the bucket.


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

Copied code report [was: Re: [PATCH] SSL layer for svnserve]

Posted by Daniel Patterson <da...@danpat.net>.
Joe Orton wrote:
> 
> Bits of this code do look like they have been copied from neon e.g. 

   Speaking of copied code, I ran across a tool called Simian [1]
   the other day which does code similarity checking.

   I've attached the results (gzipped, hope it's not too big for
   this list (18k)), most of the interesting matches are near the
   end of the report.  Unsurprisingly, there are lots of long
   matches in the fs_fs vs bdb areas.  There are some other
   rather suspect looking entries though:

Found 33 duplicate lines in the following files:
  Between lines 608 and 672 in subversion/libsvn_repos/delta.c
  Between lines 420 and 483 in subversion/libsvn_repos/reporter.c

   looks like someone moved the function but forgot to remove it.

   Make of it what you will.

daniel

[1] http://www.redhillconsulting.com.au/products/simian/

Re: [PATCH] SSL layer for svnserve

Posted by Joe Orton <jo...@manyfish.co.uk>.
On Mon, Oct 18, 2004 at 08:03:48PM +0200, Sigfred Håversen wrote:
> I've added a SSL layer to svnserve, and I would like to have some comments
> on the patch. Instructions how to run "make check" with self-signed certificate,
> is below.

Bits of this code do look like they have been copied from neon e.g. 
asn1time_to_string, but you have stripped the copyright notices.

joe

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