[S] Change in openvpn[master]: Introduce get_key_by_management_key_id helper function
"plaisthos \(Code Review\) via Openvpn-devel" <[email protected]>
| Newsgroups | gmane.network.openvpn.devel |
|---|---|
| Message-ID | <571cd685b87db4f97b8d34da33ff8df1342c6cf8-EmailReviewComments-HTML@gerrit.openvpn.net> |
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