On Wed, Oct 22, 2008 at 09:30:08AM +0200, Daniel Stenberg wrote:
> On Wed, 22 Oct 2008, Josef Wolf wrote:
> >2. The library part: This part should detect the 401, call a callback
> > into the application to retrieve the credentials, and finally retry
> > the request with the supplied credentials. The callback would have
> > a signature like this:
> >
> > int curl_get_credentials (char *realm, char *url, int isproxy)

I have to correct myself here. The signature should look something
like:

  int curl_get_credentials (char *user, char *passwd, /* return values */
                            const char *realm, const char *url,
                            int isproxy)

maybe with additional parameters telling the size of the buffers for
the returned values.

> Why would a callback be needed for this? Why not simply return back to let
> the application detect the situation, ask the user, set the credentials and
> call libcurl again?

For sure, this can be done without a callback. But I think the callback
solution would be the better solution for several reasons:

1. It separates protocol handling (which, IMHO, belongs into the lib)
   from user-handling (which clearly belongs into the application).

2. I expect the callback method to avoid code duplication on three levels:

   a. Protocol and cache handling is not duplicated in every application
      that uses libcurl. This avoids the situation that every application
      introduces its own pile of bugs/vulnerabilities when they try
      to handle protocol/cache.

   b. Response parsing (which is already done in libcurl) is not
      duplicated in the application. Applications would have plenty
      of chance to introduce bugs here, too.

   c. I guess even some code from CURLOPT_FOLLOWLOCATION could be
      reused.
      After all, the overall handling for both situations is
      identical: repeat the request with slightly modified parameters.
      The only difference is that in case of 401/407, the additional
      parameters need to be retrieved from the application instead
      of the response header.
      I don't see the point in handling one situation within the
      library but requiring the application to handle the other
      (almost identical) situation by itself.

3. It is (IMHO) a cleaner design than letting the application do
   everything by itself.

4. Adding support for the new functionality into an application is much
   easier: simply supply an additional callback for credential retrieval
   instead of messing around with the (probably already complex) error
   handling, making it even more complex.
   As we all know, complexity kills security ;-)

   Actually, I have stolen the callback idea from LWP::UserAgent. It
   really makes credential handling a trivial no-brainer from the
   application point of view.

5. I guess, there might be even the opportunity to simplify libcurl
   by decoupling current user/password handling from the core library.

> >BTW: I thought curl-library list was not only about internals of the
> > library but also about how to use the library?
>
> It certainly is for that as well. My refereal to curl-users was for
> potential discussions about how curl the tool should or should not
> react on 401 (and 407 I would presume) responses.

Ah, OK.

And yes, you're right: 407 should also be covered here. That's why the
callback should have the isproxy parameter.
Received on 2008-10-22