Hi Selva,
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 <http://context.c2.es>);
}
#endif
I do not see why this requires --management-client-auth or any
management related options to be in use.
management is set to NULL unless a management address is defined
(--management), so the above is not called for a plugin or script
bool
open_management(struct context *c)
{
/* initialize management layer */
if (management)
{
if (c->options.management_addr) <----
{
....
{
else
{
close_management(); <----
}
I'm not trying to be resistant in finding another way here to avoid this
extra state you guys don't want, this is a real bug in re-auth/reneg
that we have seen occur multiple times in various setups that we want to
see fixed, and it effects management, script and plugin authentication,
however I have had a good walk through the code and I can't see another way.
The other alternative here is to refactor up the call chain so the
context_2 is passed to each of these functions instead of just multi &
session, however this is about a 500 line refactor that I really don't
want to do.
Cheers,
Eric
---
Eric Thorpe
SparkLabs Developer
https://www.sparklabs.com
https://twitter.com/sparklabs
supp...@sparklabs.com
On 25/08/2020 12:25 am, Selva Nair wrote:
On Mon, Aug 24, 2020 at 3:49 AM Eric Thorpe <e...@sparklabs.com
<mailto: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 <http://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 Developer
https://www.sparklabs.com <https://www.sparklabs.com>
https://twitter.com/sparklabs <https://twitter.com/sparklabs>
supp...@sparklabs.com <mailto:supp...@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
<mailto: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
