Am 22.02.19 um 03:42 schrieb Eric Thorpe: > Thanks for doing this one Arne, this has been on my bucket list for a > while. I've given this a reasonable test now and it's working as I'd > expect. A few comments from my testing: > > On 23/01/2019 2:03 am, Arne Schwabe wrote: > >> + /* Accept session tokens that not expired are in the acceptable range >> + * for renogiations */ >> + bool in_renog_time = now >= timestamp >> + && now < timestamp + 2 * >> session->opt->renegotiate_seconds; > I'd like to see the valid time of an auth-token have it's own value, > however I understand why you've done this. I can't find a nice way to > get through the last active time of a session through to auth without a > reasonable refactor. I'd like to see the auth token have the option > "--auth-gen-token [inactive timeout] [total timeout]" or something along > those lines. So while this isn't an ideal solution, it's good enough.
I am not really sure what talking about. There are two lifetimes for
auth token.
- the total max lifetime of an auth-token session (also specified in the
config)
- the max lifetime of an individual auth-token.
The second one is dervived from renegotiate_seconds as setting this
lower than this time will break renogiation.
The reason that I did not do any refactoring is a client with auth-token
can switch to another server and that server needs to verify that
auth-token with the information from the client and its config alone.
>> + /* We could still have a client that does not update
>> + * its auth-token, so also allow the initial auth-token */
>> + bool initialtoken = multi->auth_token_initial
>> + && memcmp_constant_time(up->password,
>> multi->auth_token_initial,
>> +
>> strlen(multi->auth_token_initial)) == 0;
> I don't agree with this being in place, only the most recently generated
> token should be valid imo.
The reality is that we don't want to exclude all the older OpenVPN3
clients that do not update their token. Without this special, after
2*renogiation time, the clinets will fail.
> When an auth-token is authenticated, the server log will print out:
>
>> TLS: Username/auth-token authentication succeeded for username is still
>> displayed.
>> TLS: Username/Password authentication succeeded for username is still
>> displayed.
> The second line seems redundant and might cause some confusion. "if
> (skip_auth)" above this log line is probably enough I think?
>
> Finally, the patch won't build under MSVC without the following change:
>
>> + struct push_list push_list = {};
> to
>
>> + struct push_list push_list = {0};
>
> auth_token.c and auth_token.h will need to be added to the VS solution
> as well however I'm happy to submit that one myself once this gets acked
> to save you the trouble.
I will look into it.
Arne
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
