You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ben Collins-Sussman <su...@collab.net> on 2001/09/15 16:00:10 UTC

authentication storage question

I'm in the middle of rewriting the client-side authentication system,
and people are right: it's much simpler and cleaner to have "RA pull
info from client when challenged", rather than "client preemptively
push data at RA".  Many problems drop away, and code gets smaller.

I have one design problem, though.

Here's a typical code flow:

  1. client calls

      session_baton = RA->open (URL, vtable_of_callbacks)


  2. client calls

      RA->do_some_network_things (session_baton)
 

      * the RA layer is challenged by Apache

      * the RA layer uses the callback vtable to get information from
        the client.  (the client has its own logic for getting info
        from argv[], or files, or user-prompts.)

      * authentication suceeds, so RA finishes its work.  or
        authentication fails, and user sees immediate error.


  3.  client repeats step 2 until finished, then calls 

      RA->close (session_baton)


Here's the missing feature:  if authentication was successful, it's
very likely that the auth info needs to be stored somewhere on the
client.  (IOW, cached for next time.  We're already doing this in our
current auth system.)

How does the client know if info needs to be stored?  Or which info
needs to be stored?

- One possibility is that each RA->do_something() call return this
  information.

- Another possibility is that RA->close() is the only call to return
  this information.  

- Another idea is to have RA do the auth storage itself, via another
  vtable callback.  Each RA routine would have to remember to do this
  before returning.

Thoughts?  I think the third idea is probably best.


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

Re: authentication storage question

Posted by Ben Collins-Sussman <su...@collab.net>.
Branko =?ISO-8859-2?Q?=C8ibej?= <br...@xbc.nu> writes:

> 
>     * open_temp_file (open a temporary file, returns apr_file_t)
>     * close_temp_file (close and delete temporary file)
>     * get_auth_data (pull authentication data from the client, in some
>       generic format)
>     * store_auth_data (archive the authentication data)
> 

That's almost exactly what my vtable looks like right now!

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

Re: authentication storage question

Posted by Branko Čibej <br...@xbc.nu>.
Ben Collins-Sussman wrote:

>I'm in the middle of rewriting the client-side authentication system,
>and people are right: it's much simpler and cleaner to have "RA pull
>info from client when challenged", rather than "client preemptively
>push data at RA".  Many problems drop away, and code gets smaller.
>
>I have one design problem, though.
>
>Here's a typical code flow:
>
>  1. client calls
>
>      session_baton = RA->open (URL, vtable_of_callbacks)
>
>
>  2. client calls
>
>      RA->do_some_network_things (session_baton)
> 
>
>      * the RA layer is challenged by Apache
>
>      * the RA layer uses the callback vtable to get information from
>        the client.  (the client has its own logic for getting info
>        from argv[], or files, or user-prompts.)
>
>      * authentication suceeds, so RA finishes its work.  or
>        authentication fails, and user sees immediate error.
>
>
>  3.  client repeats step 2 until finished, then calls 
>
>      RA->close (session_baton)
>
>
>Here's the missing feature:  if authentication was successful, it's
>very likely that the auth info needs to be stored somewhere on the
>client.  (IOW, cached for next time.  We're already doing this in our
>current auth system.)
>
>How does the client know if info needs to be stored?  Or which info
>needs to be stored?
>
>- One possibility is that each RA->do_something() call return this
>  information.
>
>- Another possibility is that RA->close() is the only call to return
>  this information.  
>
>- Another idea is to have RA do the auth storage itself, via another
>  vtable callback.  Each RA routine would have to remember to do this
>  before returning.
>
>Thoughts?  I think the third idea is probably best.
>
I think so, too. Right now, the client stores auth data if the RA 
operation is successful -- which means that if, e.g., a checkout fails, 
the auth data will not be cached, even if the authentication itself is 
O.K. I ran into this while I was fixin co on Windows, and got mighty 
bored by the username prompts.

So: cache the auth data as soon as authentication succeeds, and since a) 
only the RA layer knows when that happens, and b) only the client knows 
how to archive the data, the third idea is the only one that works.


BTW, what's in the RA callback vtable? IIRC:

    * open_temp_file (open a temporary file, returns apr_file_t)
    * close_temp_file (close and delete temporary file)
    * get_auth_data (pull authentication data from the client, in some
      generic format)
    * store_auth_data (archive the authentication data)


-- 
Brane �ibej   <br...@xbc.nu>            http://www.xbc.nu/brane/




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

Re: authentication storage question

Posted by Ben Collins-Sussman <su...@collab.net>.
Greg Stein <gs...@lyra.org> writes:
> 
> So... the client-provided vtable looks something like:
> 
> * open_tempfile(client_baton, pool, &file)
> * close_tempfile(file)
> * get_authenticator(which_method, client_baton, pool, &auth_vtable, &auth_baton)
> 
> And the authenticator vtable looks like:
> 
> * authenticate(auth_baton)
> * save_authinfo(auth_baton)
> * ... functions appropriate to this auth method


I don't understand why an authenticator structure needs the
authenticate() function;  isn't that a leftover from the client
"pushing" data at RA?  Instead, I imagine this structure only needs
functions to get specific info (get_username(), get_password()), and
ones to save the data in the client.


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

Re: authentication storage question

Posted by Ben Collins-Sussman <su...@collab.net>.
Dale Thatcher <su...@dalethatcher.com> writes:

> I've been trying to find a decent place for the proxy callbacks
> without any major change to the svn_ra.h.

Look at the new svn_ra.h -- it might be better for you now.

Here's the new system, as simply as I can put it:

  * an administrator configures the svn server (apache) to require
    certain types of authentication for certain repositories.

  * the svn client attempts to do something (checkout, commit, etc.)

     * libsvn_client creates a table of callbacks that are able to
       provide auth information;  it passes this table to neon.

     * if neon is never challenged by apache:

          the callback table is ignored.

     * if neon -is- challenged by apache:

          neon uses the callback table to "pull" authentication data
          from the client.  (username, password, private key, cert,
          etc.)  

          - if authentication fails, neon returns an error immediately
            to the client app.

          - if authentication succeeds, the svn subcommand finishes,
            and neon "cleans up" by invoking a client callback to
            cache the auth info in the working copy.

So really, the old system made some erroneous assumptions -- it
assumed that the client was in control.  It assumed that the client
could wily-nily select any old authentication protocol from a menu of
choices.  It can't.

Really, apache is in total control.  Apache dictates exactly what auth
protocol will be used, end of story.

To make Subversion use a new authentication protocol called "FooAuth",
here's what would have to happen:

   * make sure apache understands the protocol, and can issue a
     FooAuth challenge when necessary

   * make sure neon knows how to respond to a FooAuth challenge

   * make sure libsvn_client knows how to provide the specific FooAuth
     information to neon, so that neon can properly respond.



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

Re: authentication storage question

Posted by Dale Thatcher <su...@dalethatcher.com>.
I've been trying to find a decent place for the proxy callbacks without any
major change to the svn_ra.h.  Passing in the proxy information would either
have to be done by changing the parameters to the get_authenticator to accept
a proxy argument (which really doesn't make sense to the ra_local stuff) or to
add callback setting functions in the returned authenticator structure.

The sort of callbacks I'm thinking of would be:

typedef char *(*svn_ra_proxy_callback) (
        svn_stringbuf_t *target_URL,
        apr_pool_t *pool);

typedef char *(*svn_ra_userpass_callback) (
        svn_stringbuf_t *target_URL,
        char **username,
        char **password,
        apr_pool_t *pool);

The userpass callback could be used in many different cases.  With a 'file'
prefix for the local disk and 'http' or 'https' for the remote proxy or
website.  The callback could also be useful for passwords that have changed or
been mistyped.  The application could try the website and if a 407 is returned
the callback could be queried again.

The actual connection code would have to be moved to the 'authenticate'
function as on many proxied networks you would not have access to DNS.  I'm
not really happy when the proxy settings going inside the authenticator
structure but don't really seen another place for them at the moment.  Have I
missed something?

thanks,

- Dale Thatcher


On Mon, Sep 17, 2001 at 07:44:34AM -0500, Ben Collins-Sussman wrote:
> Greg Stein <gs...@lyra.org> writes:
> 
> 
> > Should have posted the header before rewriting :-)
> 
> No worries.  I'm used to rewriting and rewriting.  :-)
> 
> 
> > This won't work...
> > 
> > 1) The RA layer should not do anything with prompts or echo status. That is
> >    just "out of scope" for RA -- leave it to the callback function.
> > 
> > 2) The RA layer should not fetch items as individual units. Consider the
> >    user/password situation in a GUI. The pair are fetched at the same time.
> >    The point here is that stuff is fetched as a semantic group, rather than
> >    "fetch A, then fetch B, then C".
> >    (and I dunno why you'd use an apr_array_header_t since the set of items
> >     is known in advance and static)
> > 
> > 3) Saving information is similar: based on the semantic, different things
> >    are saved. You sure wouldn't want to save the password for that SSL key,
> >    but you would for a plain-text HTTP password.
> > 
> > Each "type" of authentication is going to have a way to get the needed
> > pieces, and to save the appropriate pieces.
> 
> So in other words, even though RA is pulling info now, you're saying
> we *still* need to create specific structures for specific protocols?
> Are you sure this isn't being overly strict?
> 
> Hmmm.  Thinking about it, I suppose that even the "username" object
> can't be shared between protocols.  When ra_local pulls a username,
> the client should return the process owner.  But when ra_foo pulls a
> username, it may be trying to get it along with *all* auth info from
> some random file in ~.
> 
> You make sense, will rewrite.  :-)
> 
> ---------------------------------------------------------------------
> 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: authentication storage question

Posted by Ben Collins-Sussman <su...@collab.net>.
Greg Stein <gs...@lyra.org> writes:


> Should have posted the header before rewriting :-)

No worries.  I'm used to rewriting and rewriting.  :-)


> This won't work...
> 
> 1) The RA layer should not do anything with prompts or echo status. That is
>    just "out of scope" for RA -- leave it to the callback function.
> 
> 2) The RA layer should not fetch items as individual units. Consider the
>    user/password situation in a GUI. The pair are fetched at the same time.
>    The point here is that stuff is fetched as a semantic group, rather than
>    "fetch A, then fetch B, then C".
>    (and I dunno why you'd use an apr_array_header_t since the set of items
>     is known in advance and static)
> 
> 3) Saving information is similar: based on the semantic, different things
>    are saved. You sure wouldn't want to save the password for that SSL key,
>    but you would for a plain-text HTTP password.
> 
> Each "type" of authentication is going to have a way to get the needed
> pieces, and to save the appropriate pieces.

So in other words, even though RA is pulling info now, you're saying
we *still* need to create specific structures for specific protocols?
Are you sure this isn't being overly strict?

Hmmm.  Thinking about it, I suppose that even the "username" object
can't be shared between protocols.  When ra_local pulls a username,
the client should return the process owner.  But when ra_foo pulls a
username, it may be trying to get it along with *all* auth info from
some random file in ~.

You make sense, will rewrite.  :-)

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

Re: authentication storage question

Posted by Greg Stein <gs...@lyra.org>.
On Sun, Sep 16, 2001 at 05:46:22PM -0500, Ben Collins-Sussman wrote:
> Greg Stein <gs...@lyra.org> writes:
>...
> > And the authenticator vtable looks like:
> > 
> > * authenticate(auth_baton)
> > * save_authinfo(auth_baton)
> > * ... functions appropriate to this auth method
> 
> Authenticator?  We have no more authenticator objects.

Euh. We should...

> Here's my latest draft of svn_ra.h.  I'm almost done rewriting
> svn_client_* routines and the two RA implementations to use it.

Should have posted the header before rewriting :-)

>...
> /* A structure that represents an RA layer's desire to pull
>    authentication information from the client; this information can be
>    retrieved either by prompting the user, or by retrieving it from a
>    file, etc.  It's up to the client to decide how to retrieve the
>    info. */
> typedef struct svn_ra_get_info_t
> {
>   /* A flag which identifies the exact info being asked for.  Should
>      be one of the #defines above. */
>   apr_uint64_t info_kind;
> 
>   /* A prompt to display to the user (if necessary). */
>   const char *prompt;
> 
>   /* If prompting user, whether to echo typing or not. */
>   svn_boolean_t no_echo;
> 
>   /* The desired information, filled in by the client. */
>   char *info;
> 
> } svn_ra_get_info_t;
>
>... reordered ...
> 
>   /* Pull a list of information from the client.
> 
>      INFOS is a list of svn_ra_get_info_t structures;  the client's
>      responses will be returned in each structure's `info' field,
>      allocated in POOL. */
>   svn_error_t *(*get_informations) (apr_array_header_t *infos,
>                                     void *info_baton,
>                                     apr_pool_t *pool);
> 
>   /* Store auth info in the client.
> 
>      Assuming authentication was successful, ask the client to store
>      each svn_ra_get_info_t in INFOS -- so that the info will be
>      cached for next time.  */
>   svn_error_t *(*set_informations) (apr_array_header_t *infos,
>                                     void *info_baton,
>                                     apr_pool_t *pool);

This won't work...

1) The RA layer should not do anything with prompts or echo status. That is
   just "out of scope" for RA -- leave it to the callback function.

2) The RA layer should not fetch items as individual units. Consider the
   user/password situation in a GUI. The pair are fetched at the same time.
   The point here is that stuff is fetched as a semantic group, rather than
   "fetch A, then fetch B, then C".
   (and I dunno why you'd use an apr_array_header_t since the set of items
    is known in advance and static)

3) Saving information is similar: based on the semantic, different things
   are saved. You sure wouldn't want to save the password for that SSL key,
   but you would for a plain-text HTTP password.

Each "type" of authentication is going to have a way to get the needed
pieces, and to save the appropriate pieces.

This is why I suggested a per-auth-type vtable that had "authenticate" and
"save" in it. Each type can then do everything needed. Each per-type
authenticator can return the appropriate needed information to the RA layer.

>...
>   /* -- Authentication Callbacks -- */
> 
>   /* A context the client may need for getting/setting auth info. */
>   void *info_baton;

Keep this out of the structure. This kind of thing was in the original
authenticator stuff, and it was a pain in the ass to get rid of. It means
that people cannot use static structures for the vtables. And that just
means it is a real bitch to construct and pass these things around. Pass it
separately.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

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

Re: authentication storage question

Posted by Ben Collins-Sussman <su...@collab.net>.
Greg Stein <gs...@lyra.org> writes:

> So... the client-provided vtable looks something like:
> 
> * open_tempfile(client_baton, pool, &file)
> * close_tempfile(file)
> * get_authenticator(which_method, client_baton, pool, &auth_vtable, &auth_baton)
> 
> [ we may not need close_tempfile; APR files already have a "delete on close"
>   and there is also a patch floating for APR to create temp files as
>   appropriate for the platform at hand (e.g. TMP/TEMP/TMPDIR on Windows, or
>   mkstemp() on unixen, etc) ]

That would be swell.  :-)

> 
> And the authenticator vtable looks like:
> 
> * authenticate(auth_baton)
> * save_authinfo(auth_baton)
> * ... functions appropriate to this auth method


Authenticator?  We have no more authenticator objects.  

Here's my latest draft of svn_ra.h.  I'm almost done rewriting
svn_client_* routines and the two RA implementations to use it.


/*** Authentication ***/

/* This is the 2nd draft of client-side authentication; in the first
   draft, the client presumed to be in control of selecting auth
   methods and then "pushing" data at the RA layer.  In this draft,
   the RA layer is presumed to be in control;  as server challenges
   are made, it "pulls" data from the client via callbacks. */


/* Every known type of info that an RA layer may want to fetch from
   the client.  */
#define SVN_RA_AUTH_USERNAME                       0x0001
#define SVN_RA_AUTH_PASSWORD                       0x0002

/* ### someday add PRIVATE_KEY, CERT, etc. */


/* A structure that represents an RA layer's desire to pull
   authentication information from the client; this information can be
   retrieved either by prompting the user, or by retrieving it from a
   file, etc.  It's up to the client to decide how to retrieve the
   info. */
typedef struct svn_ra_get_info_t
{
  /* A flag which identifies the exact info being asked for.  Should
     be one of the #defines above. */
  apr_uint64_t info_kind;

  /* A prompt to display to the user (if necessary). */
  const char *prompt;

  /* If prompting user, whether to echo typing or not. */
  svn_boolean_t no_echo;

  /* The desired information, filled in by the client. */
  char *info;

} svn_ra_get_info_t;


/* A collection of callbacks that allows an RA layer to "pull"
   information from the client application, or possibly store
   information. */
typedef struct svn_ra_callbacks_t
{
  /* -- Miscellaneous Callbacks -- */

  /* Open a temporary file FILENAME in the working copy. 
     Arguments are identical to apr_file_open(), except that FILENAME
     is presumed to be a basename (instead of a whole path.) */
  svn_error_t *(*open_tmp_file) (apr_file_t **fp,
                                 const char *filename,
                                 apr_int32_t flag,
                                 apr_fileperms_t perm,
                                 apr_pool_t *pool);
  
  /* Delete a temporary file FILENAME, which previously created in
     working copy.  Again, FILENAME is assumed to be a basename. */
  svn_error_t *(*delete_tmp_file) (const char *filename);


  /* -- Authentication Callbacks -- */

  /* A context the client may need for getting/setting auth info. */
  void *info_baton;

  /* Pull a list of information from the client.

     INFOS is a list of svn_ra_get_info_t structures;  the client's
     responses will be returned in each structure's `info' field,
     allocated in POOL. */
  svn_error_t *(*get_informations) (apr_array_header_t *infos,
                                    void *info_baton,
                                    apr_pool_t *pool);

  /* Store auth info in the client.

     Assuming authentication was successful, ask the client to store
     each svn_ra_get_info_t in INFOS -- so that the info will be
     cached for next time.  */
  svn_error_t *(*set_informations) (apr_array_header_t *infos,
                                    void *info_baton,
                                    apr_pool_t *pool);

} svn_ra_callbacks_t



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

Re: authentication storage question

Posted by Greg Stein <gs...@lyra.org>.
On Sat, Sep 15, 2001 at 11:00:10AM -0500, Ben Collins-Sussman wrote:
>...
>   1. client calls
> 
>       session_baton = RA->open (URL, vtable_of_callbacks)

Note that there should probably be a baton associated with that vtable.

>   2. client calls
> 
>       RA->do_some_network_things (session_baton)
>  
> 
>       * the RA layer is challenged by Apache
> 
>       * the RA layer uses the callback vtable to get information from
>         the client.  (the client has its own logic for getting info
>         from argv[], or files, or user-prompts.)

Data comes via the baton, right? :-)

>...
> - Another idea is to have RA do the auth storage itself, via another
>   vtable callback.  Each RA routine would have to remember to do this
>   before returning.

The latter. When RA fetches an authenticator, it should get a baton. In the
authenticator vtable, there is a "store auth info" entry.

So... the client-provided vtable looks something like:

* open_tempfile(client_baton, pool, &file)
* close_tempfile(file)
* get_authenticator(which_method, client_baton, pool, &auth_vtable, &auth_baton)

[ we may not need close_tempfile; APR files already have a "delete on close"
  and there is also a patch floating for APR to create temp files as
  appropriate for the platform at hand (e.g. TMP/TEMP/TMPDIR on Windows, or
  mkstemp() on unixen, etc) ]

And the authenticator vtable looks like:

* authenticate(auth_baton)
* save_authinfo(auth_baton)
* ... functions appropriate to this auth method


Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

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