Re: svn commit: r1926227 - in /serf/branches/user-defined-authn/auth: auth.c auth.h auth_basic.c auth_digest.c auth_spnego.c auth_user_defined.c

Mon, 09 Jun 2025 05:58:31 -0700 

On 9. 6. 25 09:27, Peter Balogh wrote:

Hi,
I took a peak at this, but I'm not sure, how this interface is supposed to be used Where, when do I call serf_authn_register_scheme to register a new scheme, where I have a baton? Is the implementation supposed to call a callback like with the serf_config_credentials_callback or is subversion going to supply the callbacks as well over time? Can I help move this forward, or should I wait until this is ironed out to a working version? 

Best regards,
Peter
The interface is incomplete. As you see, Serf's internal authn providers register four callbacks. Similarly, a user-defined scheme will provide equivalent callbacks, but such that they won't have access to Serf's internal structures and data, only to the published interfaces. I haven't worked out yet what those callbacks should look like.
The idea is that the user of the Serf library registers its custom authn schemes at startup, at the same time configures Serf; Serf then calls the callbacks as needed during request and response processing. 

In the case of Subversion, to implement 2FA, you'd have to:

 * implement a new auth provider in Subversion that provides the
   necessary callbacks, just like the its current auth providers do;
 * this provider would then register itself into Serf's authentication
   flow.
Right now, I'm working on just the Serf side, so that when this is implemented, it won't be necessary to modify Serf in order to add new authentication schemes. 

-- Brane



On 2025. 06. 08. 3:33, br...@apache.org wrote:

Author: brane
Date: Sun Jun  8 01:33:45 2025
New Revision: 1926227

URL: http://svn.apache.org/viewvc?rev=1926227&view=rev
Log:
On the user-defined-auth branch: Don't "subclass" a private struct.
Next time, save some of those overactiv neurons for actual thinking.

* auth/auth.h
   (serf__user_authn_scheme_t): Remove. Move its contents to ...
   (serf__authn_scheme_t): ... here.
* auth/auth.c
   (serf_authn_register_scheme): Deal with the struct merge.

* auth/auth_basic.c,
   auth/auth_spnego.c,
   auth/auth_spnego.c: Explicitly initialize serf__authn_scheme_t::magic, 
    even though it's not really necessary for static objects.

* auth/auth_user_defined.c
   (validate_user_authn): New; replaces safe_cast_scheme.
   (serf__authn_user__init_conn,
    serf__authn_user__handler,
    serf__authn_user__setup_request,
    serf__authn_user__validate_response): Use validate_user_authn.

Modified:
     serf/branches/user-defined-authn/auth/auth.c
     serf/branches/user-defined-authn/auth/auth.h
     serf/branches/user-defined-authn/auth/auth_basic.c
     serf/branches/user-defined-authn/auth/auth_digest.c
     serf/branches/user-defined-authn/auth/auth_spnego.c
     serf/branches/user-defined-authn/auth/auth_user_defined.c

Modified: serf/branches/user-defined-authn/auth/auth.c
URL: http://svn.apache.org/viewvc/serf/branches/user-defined-authn/auth/auth.c?rev=1926227&r1=1926226&r2=1926227&view=diff ============================================================================== 
--- serf/branches/user-defined-authn/auth/auth.c (original)
+++ serf/branches/user-defined-authn/auth/auth.c Sun Jun  8 01:33:45 2025 
@@ -627,7 +627,7 @@ apr_status_t serf_authn_register_scheme(
                                          apr_pool_t *result_pool,
                                          int *type)
  {
-    serf__user_authn_scheme_t *user_scheme;
+    serf__authn_scheme_t *authn_scheme;
      apr_status_t lock_status;
      apr_status_t status;
      unsigned int scheme_type;
@@ -636,9 +636,7 @@ apr_status_t serf_authn_register_scheme(
      int index;
        *type = SERF_AUTHN_NONE;
-    user_scheme = apr_palloc(result_pool, sizeof(*user_scheme));
-    user_scheme->magic = serf__authn_user__magic;
-    user_scheme->baton = baton;
+    authn_scheme = apr_palloc(result_pool, sizeof(*authn_scheme));
        /* Generate a lower-case key for the scheme. */
      key = cp = apr_pstrdup(result_pool, name);
@@ -646,13 +644,17 @@ apr_status_t serf_authn_register_scheme(
          *cp = apr_tolower(*cp);
          ++cp;
      }
-    user_scheme->authn_scheme.name = apr_pstrdup(result_pool, name);
-    user_scheme->authn_scheme.key = key;
-    /* user_scheme->authn_scheme.type = ?; Will be updated later, under lock. */ -    user_scheme->authn_scheme.init_conn_func = serf__authn_user__init_conn; 
-    user_scheme->authn_scheme.handle_func = serf__authn_user__handler;
-    user_scheme->authn_scheme.setup_request_func = serf__authn_user__setup_request; -    user_scheme->authn_scheme.validate_response_func = serf__authn_user__validate_response; 
+    authn_scheme->name = apr_pstrdup(result_pool, name);
+    authn_scheme->key = key;
+    /* user_scheme->type = ?; Will be updated later, under lock. */
+    authn_scheme->init_conn_func = serf__authn_user__init_conn;
+    authn_scheme->handle_func = serf__authn_user__handler;
+    authn_scheme->setup_request_func = serf__authn_user__setup_request;
+    authn_scheme->validate_response_func = serf__authn_user__validate_response; 
+
+    /* User-defined scheme data. */
+    authn_scheme->magic = serf__authn_user__magic;
+    authn_scheme->baton = baton;
        lock_status = lock_autn_schemes(NULL /* TODO: whence cometh config? */); 
      if (lock_status)
@@ -687,8 +689,8 @@ apr_status_t serf_authn_register_scheme(
      }
        /* Insert into the slot, and add the sentinel. */
-    user_scheme->authn_scheme.type = scheme_type;
-    serf_authn_schemes[index] = &user_scheme->authn_scheme;
+    authn_scheme->type = scheme_type;
+    serf_authn_schemes[index] = authn_scheme;
      serf_authn_schemes[index + 1] = NULL;
      *type = scheme_type;

Modified: serf/branches/user-defined-authn/auth/auth.h
URL: http://svn.apache.org/viewvc/serf/branches/user-defined-authn/auth/auth.h?rev=1926227&r1=1926226&r2=1926227&view=diff ============================================================================== 
--- serf/branches/user-defined-authn/auth/auth.h (original)
+++ serf/branches/user-defined-authn/auth/auth.h Sun Jun  8 01:33:45 2025 
@@ -108,6 +108,16 @@ struct serf__authn_scheme_t {
        /* Function to validate the authentication header of a response */ 
      serf__validate_response_func_t validate_response_func;
+
+    /*
+     * Additional data for user-defined authentication schemes.
+     */
+
+    /* The magic number that helps verify the user-defined scheme data. */ 
+    apr_uint64_t magic;
+
+    /* The baton used by the callbacks.  */
+    void *baton;
  };
    @@ -141,22 +151,6 @@ extern const serf__authn_scheme_t serf__
    /** User-defined authentication scheme handlers */
  -/* This struct extends serf__authn_scheme_t with info needed for
-   the user-defined scheme implementation. It's essentially a subclass;
-   per C semantics, the address of the struct is also the address of
-   its first member, so we can safely put a pointer to this struct
-   into serf_authn_schemes. */
-typedef struct serf__user_authn_scheme_t serf__user_authn_scheme_t;
-struct serf__user_authn_scheme_t {
-    serf__authn_scheme_t authn_scheme;
-
-    /* The magic number that helps identify this struct. */
-    apr_uint64_t magic;
-
-    /* The baton used by the callbacks.  */
-    void *baton;
-};
-
  #ifndef SERF__AUTHN__HAVE_UNREGISTER
  /* Declare the prototype for the internal unregister implementation */
  apr_status_t serf__authn__unregister_scheme(int type,

Modified: serf/branches/user-defined-authn/auth/auth_basic.c
URL: http://svn.apache.org/viewvc/serf/branches/user-defined-authn/auth/auth_basic.c?rev=1926227&r1=1926226&r2=1926227&view=diff ============================================================================== 
--- serf/branches/user-defined-authn/auth/auth_basic.c (original)
+++ serf/branches/user-defined-authn/auth/auth_basic.c Sun Jun 8 01:33:45 2025 
@@ -198,4 +198,6 @@ const serf__authn_scheme_t serf__basic_a
      serf__handle_basic_auth,
      serf__setup_request_basic_auth,
      validate_response_func,
+
+    0                           /* user-defined scheme magic */
  };

Modified: serf/branches/user-defined-authn/auth/auth_digest.c
URL: http://svn.apache.org/viewvc/serf/branches/user-defined-authn/auth/auth_digest.c?rev=1926227&r1=1926226&r2=1926227&view=diff ============================================================================== 
--- serf/branches/user-defined-authn/auth/auth_digest.c (original)
+++ serf/branches/user-defined-authn/auth/auth_digest.c Sun Jun 8 01:33:45 2025 
@@ -566,4 +566,6 @@ const serf__authn_scheme_t serf__digest_
      serf__handle_digest_auth,
      serf__setup_request_digest_auth,
      serf__validate_response_digest_auth,
+
+    0                           /* user-defined scheme magic */
  };

Modified: serf/branches/user-defined-authn/auth/auth_spnego.c
URL: http://svn.apache.org/viewvc/serf/branches/user-defined-authn/auth/auth_spnego.c?rev=1926227&r1=1926226&r2=1926227&view=diff ============================================================================== 
--- serf/branches/user-defined-authn/auth/auth_spnego.c (original)
+++ serf/branches/user-defined-authn/auth/auth_spnego.c Sun Jun 8 01:33:45 2025 
@@ -663,6 +663,8 @@ const serf__authn_scheme_t serf__spnego_
      serf__handle_spnego_auth,
      serf__setup_request_spnego_auth,
      serf__validate_response_spnego_auth,
+
+    0                           /* user-defined scheme magic */
  };
    #ifdef WIN32
@@ -674,6 +676,8 @@ const serf__authn_scheme_t serf__ntlm_au
      serf__handle_spnego_auth,
      serf__setup_request_spnego_auth,
      serf__validate_response_spnego_auth,
+
+    0                           /* user-defined scheme magic */
  };
  #endif /* #ifdef WIN32 */

Modified: serf/branches/user-defined-authn/auth/auth_user_defined.c
URL: http://svn.apache.org/viewvc/serf/branches/user-defined-authn/auth/auth_user_defined.c?rev=1926227&r1=1926226&r2=1926227&view=diff ============================================================================== 
--- serf/branches/user-defined-authn/auth/auth_user_defined.c (original)
+++ serf/branches/user-defined-authn/auth/auth_user_defined.c Sun Jun  8 01:33:45 2025 
@@ -25,14 +25,11 @@
  #include "auth.h"
    -static const serf__user_authn_scheme_t *
-safe_cast_scheme(const serf__authn_scheme_t *scheme)
+static const bool
+validate_user_authn(const serf__authn_scheme_t *scheme)
  {
-    const serf__user_authn_scheme_t *const user_scheme = (const void *)scheme; 
-    if (scheme->type & *serf__authn_user__type_mask
-        && user_scheme->magic == serf__authn_user__magic)
-        return user_scheme;
-    return NULL;
+    return (scheme->type & *serf__authn_user__type_mask
+            && scheme->magic == serf__authn_user__magic);
  }
    apr_status_t
@@ -41,7 +38,7 @@ serf__authn_user__init_conn(const serf__
                              serf_connection_t *conn,
                              apr_pool_t *pool)
  {
-    if (!safe_cast_scheme(scheme))
+    if (!validate_user_authn(scheme))
          return APR_EINVAL;
        return APR_ENOTIMPL;
@@ -56,7 +53,7 @@ serf__authn_user__handler(const serf__au
                            const char *auth_attr,
                            apr_pool_t *pool)
  {
-    if (!safe_cast_scheme(scheme))
+    if (!validate_user_authn(scheme))
          return APR_EINVAL;
        return APR_ENOTIMPL;
@@ -72,7 +69,7 @@ serf__authn_user__setup_request(const se
                                  const char *uri,
                                  serf_bucket_t *hdrs_bkt)
  {
-    if (!safe_cast_scheme(scheme))
+    if (!validate_user_authn(scheme))
          return APR_EINVAL;
        return APR_ENOTIMPL;
@@ -87,7 +84,7 @@ serf__authn_user__validate_response(cons
                                      serf_bucket_t *response,
                                      apr_pool_t *pool)
  {
-    if (!safe_cast_scheme(scheme))
+    if (!validate_user_authn(scheme))
          return APR_EINVAL;
        return APR_ENOTIMPL;

Reply via email to