Re: [Openvpn-devel] [PATCH 3/3] Set DCO_NOT_INSTALLED also for keys not in the get_key_scan range

2022-12-13 Thread Antonio Quartulli

Hi,

On 13/12/2022 23:54, Arne Schwabe wrote:

We have 6 key slots but normally only consider 3 of them to be
active/valid keys. Especially the secondary key of TM_LAME_DUCK can
in rare corner cases have a key that is still installed in the kernel.

While this should not cause any issues since I do not see way for this
key to become active ever again, it is better to keep the state correctly.

Signed-off-by: Arne Schwabe 


I am fine with this because it is just "safer".
But I truly dislike that we have to "Defend ourselves" from conditions 
which we don't even know when they may happen

Still, this is a rant for another patch/cleanup.

This patch makes sense
Acked-by: Antonio Quartulli 


---
  src/openvpn/dco.c | 14 +-
  1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 36bfbf10a..20196fe5d 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -221,13 +221,17 @@ dco_update_keys(dco_context_t *dco, struct tls_multi 
*multi)
  multi->dco_keys_installed = 1;
  }
  
-/* all keys that are not installed are set to NOT installed */

-for (int i = 0; i < KEY_SCAN_SIZE; ++i)
+/* all keys that are not installed are set to NOT installed. Include also
+ * keys that might even be considered as active keys to be sure*/
+for (int i = 0; i < TM_SIZE; ++i)
  {
-struct key_state *ks = get_key_scan(multi, i);
-if (ks != primary && ks != secondary)
+for (int j = 0; j < KS_SIZE; j++)
  {
-ks->dco_status = DCO_NOT_INSTALLED;
+struct key_state *ks = >session[i].key[j];
+if (ks != primary && ks != secondary)
+{
+ks->dco_status = DCO_NOT_INSTALLED;
+}
  }
  }
  return true;


--
Antonio Quartulli


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH 3/3] Set DCO_NOT_INSTALLED also for keys not in the get_key_scan range

2022-12-13 Thread Arne Schwabe
We have 6 key slots but normally only consider 3 of them to be
active/valid keys. Especially the secondary key of TM_LAME_DUCK can
in rare corner cases have a key that is still installed in the kernel.

While this should not cause any issues since I do not see way for this
key to become active ever again, it is better to keep the state correctly.

Signed-off-by: Arne Schwabe 
---
 src/openvpn/dco.c | 14 +-
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/openvpn/dco.c b/src/openvpn/dco.c
index 36bfbf10a..20196fe5d 100644
--- a/src/openvpn/dco.c
+++ b/src/openvpn/dco.c
@@ -221,13 +221,17 @@ dco_update_keys(dco_context_t *dco, struct tls_multi 
*multi)
 multi->dco_keys_installed = 1;
 }
 
-/* all keys that are not installed are set to NOT installed */
-for (int i = 0; i < KEY_SCAN_SIZE; ++i)
+/* all keys that are not installed are set to NOT installed. Include also
+ * keys that might even be considered as active keys to be sure*/
+for (int i = 0; i < TM_SIZE; ++i)
 {
-struct key_state *ks = get_key_scan(multi, i);
-if (ks != primary && ks != secondary)
+for (int j = 0; j < KS_SIZE; j++)
 {
-ks->dco_status = DCO_NOT_INSTALLED;
+struct key_state *ks = >session[i].key[j];
+if (ks != primary && ks != secondary)
+{
+ks->dco_status = DCO_NOT_INSTALLED;
+}
 }
 }
 return true;
-- 
2.37.1 (Apple Git-137.1)



___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel