Re: Thinking about MFA infrastructure

[Branko Čibej <br...@apache.org>]

On 8. 7. 25 17:26, Branko Čibej wrote:
> On 26. 6. 25 08:16, Branko Čibej wrote:
>> On 24. 6. 25 07:52, Branko Čibej wrote:
>>> On 14. 6. 25 21:23, Branko Čibej wrote:
>>>> Hi all,
>>>>
>>>> I've been digging into Serf's support for various authentication 
>>>> schemes and I notices something that looks like a bit of a limitation.
>>>>
>>>> Unless I'm much mistaken, there's space for only one authentication 
>>>> baton in Serf's context. It would seem that this is rather a 
>>>> blocker for implementing multi-factor authentication flows, for 
>>>> example, Basic + OTP, where the server would first require basic 
>>>> credentials and then, if those were correct, go on to issue an OTP 
>>>> challenge.
>>>>
>>>> It seems to me that a simple solution for that would be to store an 
>>>> authn baton per scheme, but I know on the close order of nothing 
>>>> about the possible side effects.
>>>>
>>>> Yeah, I'm starting small, I have no wish to implement OAuth2 flow 
>>>> any time soon. Still, a bit of insight from the knowledgeable would 
>>>> be welcome.
>>>
>>>
>>> With r1926674, I think the user-defined-authn branch is ready for 
>>> review. Here's a short summary of the changes on the branch:
>>>
>>>  * The (private) struct serf__authn_scheme_t, defined in auth/auth.h,
>>>    grew a number of new members to support user-defined schemes, among
>>>    them five new user-visible callback functions. These, along with
>>>    their interactions, are described in serf.h. The built-in
>>>    authentication schemes don't need or use these new members.
>>>
>>>  * The (private) table serf_authn_schemes, defined in auth/auth.c, has
>>>    been extended to have room for as many schemes as there are bits in
>>>    serf__authn_scheme_t::type. Locking was added to serialize access,
>>>    because new schemes can be registered at runtime.
>>>
>>>  * Private callbacks were added to implement the user-visible
>>>    authentication flow.
>>>
>>>  * Two new public APIs take care of registering and unregistering
>>>    authentication schemes. The latter is tricky because we don't track
>>>    references to the schemes in contexts and connections, only in the
>>>    serf_authn_schemes table; this issue is documented.
>>>
>>>  * A number of tests were added for all this new stuff.
>>>
>>>
>>> Please take a look if you have time.
>>>
>>>    svn diff --internal-diff \
>>> https://svn.apache.org/repos/asf/serf/trunk  \
>>> https://svn.apache.org/repos/asf/serf/branches/user-defined-authn \
>>>         | colordiff --color=always \
>>>         | less -R
>>>
>>
>> There are a couple more changes on that branch now:
>>
>>  * The parameters in the authentication challenge (headers
>>    WWW-Authenticate and Proxy-Authenticate) are now parsed into a
>>    dictionary, using the rules described in RFC 9110.
>>
>>  * The Authentication-Info and Proxy-Authentication-Info response
>>    headers are now handled and their values parsed as above.
>>
>> These two changes should make life much easier for authn scheme 
>> implementers. I decided to add this once I saw how many times we have 
>> copy-pasted not-quite-compliant parsers throughout the auth code...
>>
>> (Note that these changes do not stop implementers from doing whatever 
>> wrong-headed, non-compliant horror they can think of, as all the raw 
>> information and headers are still available in the callbacks.)
>
>
> Any takers? It's been a couple of weeks now. I'd sure like to get 
> another pair of eyes on these changes before merging them to trunk, 
> but I'd rather not have the branch fall victim to bitrot, either.

After a month of waiting for feedback, I decided to merge this branch to 
trunk in r1927627. All my testing on several platforms has not shown any 
problems.

-- Brane