On Mon, Aug 24, 2020 at 3:49 AM Eric Thorpe <e...@sparklabs.com> wrote:
> Hi Selva,
>
> my suggestion would be to make
> this conditional on MANAGEMNET_DEF_AUTH so that we can
> then get it from session->opt->mda_context just as we do it when
> auth is done via the management. In practice, that would cover
> most builds where this is really useful.
>
> Unfortunately this doesn't help. The mda_context flags are only set when
> --management-client-auth is in use, meaning this patch would not cover
> plugin or script authentication, which are the more commonly used, and this
> patch set specifically addresses plugin authentication.
>
Are you sure of that?
In multi_connection_estableished, we have
#ifdef MANAGEMENT_DEF_AUTH
if (management)
{
management_connection_established(management,
&mi->context.c2.mda_context, mi->
context.c2.es);
}
#endif
management_connection_established will set DAF_CONENCTION_ESATBLISHED in
mdac->flags and that's what's used to determine REAUTH.
I do not see why this requires --management-client-auth or any management
related options to be in use. Sure, management support and deferred auth
support have to be enabled but restricting the usefulness of your patch to
those cases is not really a limitation.
What am I missing?
Selva
---
> Eric Thorpe
> SparkLabs
> Developerhttps://www.sparklabs.comhttps://twitter.com/sparklabssupp...@sparklabs.com
>
> On 23/08/2020 12:13 pm, Selva Nair wrote:
>
> Hi,
>
> On Thu, Aug 13, 2020 at 4:37 AM Eric Thorpe <e...@sparklabs.com> wrote:
>
>> Hi Arne,
>>
>> The issue is your state is not accessible from where that boolean needs
>> to be used unless I am missing something? Please advise if I'm mistaken
>> or of another route.
>>
>
> I agree with Arne that duplicating a state machine variable is
> not a good approach. But we have to somehow get the REAUTH (reneg)
> info in here.
>
> This has stalled for too long, so my suggestion would be to make
> this conditional on MANAGEMNET_DEF_AUTH so that we can
> then get it from session->opt->mda_context just as we do it when
> auth is done via the management. In practice, that would cover
> most builds where this is really useful.
>
> In fact, I think we should always enable MANAGEMENT_DEF_AUTH
> when management is enabled. That also gets rid of a lot of IFDEFs and
> allow the use of useful bits like CID more widely in the code. I see
> no compelling reason for such fine-grained build options.
>
> A marginal increase in code size is of little consequence all but
> embedded devices which can continue to cope without this
> as they do now.
>
> Selva
>
>
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
