>
> Hi
>
> I post this here since I want to discuss this patch on the list where everyone
> can join in.
>
> (People, Peter host his patch set here: http://www.edelweb.fr/EdelKey/ and
> I've just reviewed his patch with the name in the Subject, that is available
> from this page, or directly at
> http://www.edelweb.fr/EdelKey/files/curl-7.12.1+srp-beta.patch.gz)
>
> General:
>
> I certainly want libcurl to be able to speak SRP and I appreciate your work
> on this!
Thanks.

> Remarks:
>
> o I don't like to mix the SRP stuff in the defines and variable used for
> HTTP authentication. Using -u for user and password for the command line
> tool could possibly be OK, but extending CURLOPT_HTTPAUTH is more than I
> can take. What if you want SRP for FTPS connections. Should we require
> users to set CURLOPT_HTTPAUTH for it? It would be really confusing.

I share your opinion about CURLOPT_HTTPAUTH.
The reason for this was to have a first minimal addition
to get a proof of concept (i.e. without adding new options to the setopt).

There is indeed already for FTP and TELNET a mode and implementation available
for clients and servers that use SRP for authentication.

I see a small problem in the user friendliness of the interface to
curl: If -u is used for both basic auth and srp auth, this may result
in a srp password transferred in clear if the user forgets to specify
--srp, etc.

I tend to prefer simply --srpuser user --srppass password

and to have two new options in setopt.

> Besides that, SRP doesn't prevent the HTTP server to ask for user and
> password which will make it simply not work. I can't see how you can add
> user and password for the transport layer without using new options for it,
> or break something.
>
> o Would it be possible to reverse the OPENSSL_NO_SRP define? I find
> constructs with double negations like #ifndef OPENSSL_NO_SRP very hard to
> read.

I agree with you that it is hard to read.
It follows the normal openssl logic e.g., OPENSSL_NO_ENGINE

>
> o Please make the code use less than 80 columns per line
That
 can
 be
 done.

>
> o Your code writes a zero byte to the user+password string where it finds a
> colon. It will prevent the same string to be used multiple times, which we
> indeed want to!

It reverts the zero to the initial value. The string is an internal copy, can
it be used concurrently by several threads (multi??).

As indicated I'll propose that I remove the overloading
of CURLOPT_HTTPAUTH, and add two parameters,
one for a user one for a password.

>
> Again: thanks for your contribution!
I am very "lazy" person, adding <1% of code to some existing is easier work than
writing everything yourself. :-)

Any comments are welcome. (After today, I may not answer before next Wednesday).

Peter
Received on 2004-10-01