Attention is currently required from: flichtenheld.

plaisthos has posted comments on this change by plaisthos. ( 
http://gerrit.openvpn.net/c/openvpn/+/1856?usp=email )

Change subject: Introduce get_key_by_management_key_id helper function
......................................................................


Patch Set 6:

(1 comment)

File src/openvpn/ssl_common.h:

http://gerrit.openvpn.net/c/openvpn/+/1856/comment/af028c42_ec4c2b57?usp=email :
PS6, Line 775:             return ks;
> The old code did not abort on first match but always checked all three key 
> states. […]
So each key_state has its own unique mda_key_id so there is only one valid 
key_state with mda_key_id, so the code does the same. But valid does a bit of 
heavy lifting here:

If state is S_UNDEF, mda_key_id is 0 and 0 is also a valid key id for the very 
first client. So we need to check for S_UNDEF to determine if the client 
actually the one with key-id 0 or just not initialised.

As alternative we can change the initialisation of context_2  or assignment of 
mda_key_id_counter to start with 1 instead of 0.

E.g.

session->opt->mda_context->mda_key_id_counter++;
    ks->mda_key_id =

E.g. something like:

    index de9303b30..478e4d314 100644
    --- a/src/openvpn/manage.h
    +++ b/src/openvpn/manage.h
    @@ -69,6 +69,8 @@ struct man_def_auth_context
     #define DAF_INITIAL_AUTH           (1u << 2)
         unsigned int flags;

    +    /** Identify the key id in the management interface, always 1 or
    +     * or greater if valid */
         unsigned int mda_key_id_counter;
     };

    diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
    index 82d7daeb2..3495f9dd7 100644
    --- a/src/openvpn/ssl.c
    +++ b/src/openvpn/ssl.c
    @@ -870,7 +870,10 @@ key_state_init(struct tls_session *session, struct 
key_state *ks)
         ks->crypto_options.pid_persist = NULL;

     #ifdef ENABLE_MANAGEMENT
    -    ks->mda_key_id = session->opt->mda_context->mda_key_id_counter++;
    +    /* Increment key ID before assigning to always have a key id >= 1 to
    +     * make 0 signify an invalid key id */
    +    session->opt->mda_context->mda_key_id_counter++;
    +    ks->mda_key_id = session->opt->mda_context->mda_key_id_counter;
     #endif

         /*
    diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
    index 3b326a7c1..0270c23f2 100644
    --- a/src/openvpn/ssl_common.h
    +++ b/src/openvpn/ssl_common.h
    @@ -764,7 +764,7 @@ get_primary_key(const struct tls_multi *multi)

     #ifdef ENABLE_MANAGEMENT
     /**
    - * Gets the \c key_state  object that belong to the management key id or
    + * Gets the \c key_state object that belongs to the management key id or
      * return NULL if not found.
      */
     static inline struct key_state *

 But that is a much bigger/more invasive change but I change the commit to do 
that as well to avoid this check here.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1856?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I42d8785959c24bf688190965e58b9b98251b8557
Gerrit-Change-Number: 1856
Gerrit-PatchSet: 6
Gerrit-Owner: plaisthos <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
Gerrit-Comment-Date: Wed, 26 Aug 2026 16:23:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: flichtenheld <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to