subject:"\[Openvpn\-devel\] \[PATCH 2\/8\] Make key_state\->authenticated more state machine like"

Re: [Openvpn-devel] [PATCH 2/8] Make key_state->authenticated more state machine like

2020-07-09 Thread tincanteksup


typo x3

On 09/07/2020 11:15, Arne Schwabe wrote:

This order the states from unauthenticated to authenticated and also
changes the comparison for KS_AUTH_FALSE from != to >

It also add comments and documents part using the state machine
better.

Remove a now obsolete comment and two obsolete ifdefs. While
keeping the ifdef in ssl_verify would save a few bytes of code,
this is too minor to justify keeping the ifdef

Signed-off-by: Arne Schwabe 
---
  src/openvpn/multi.c  | 12 +---
  src/openvpn/ssl.c|  7 ---
  src/openvpn/ssl_common.h | 22 --
  src/openvpn/ssl_verify.c | 18 +++---
  4 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index f1ced9b7..f1332c8d 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2352,12 +2352,12 @@ multi_process_post(struct multi_context *m, struct 
multi_instance *mi, const uns
  if (!IS_SIG(&mi->context) && ((flags & MPP_PRE_SELECT) || ((flags & 
MPP_CONDITIONAL_PRE_SELECT) && !ANY_OUT(&mi->context
  {
  #if defined(ENABLE_ASYNC_PUSH) && defined(ENABLE_DEF_AUTH)
-bool was_authenticated = false;
+bool was_unauthenticated = true;
  struct key_state *ks = NULL;
  if (mi->context.c2.tls_multi)
  {
  ks = 
&mi->context.c2.tls_multi->session[TM_ACTIVE].key[KS_PRIMARY];
-was_authenticated = ks->authenticated;
+was_unauthenticated = (ks->authenticated == KS_AUTH_FALSE);
  }
  #endif
  
@@ -2366,7 +2366,13 @@ multi_process_post(struct multi_context *m, struct multi_instance *mi, const uns

  pre_select(&mi->context);
  
  #if defined(ENABLE_ASYNC_PUSH) && defined(ENABLE_DEF_AUTH)

-if (ks && ks->auth_control_file && ks->auth_deferred && 
!was_authenticated)
+/*
+ * if we see the state transition from unauthenticated to deferred
+ * and a auth_control_file, we assume it got just added and add


and aN auth_control_file



+ * inotify watch to that file
+ */
+if (ks && ks->auth_control_file && was_unauthenticated
+&& (ks->authenticated == KS_AUTH_DEFERRED))
  {
  /* watch acf file */
  long watch_descriptor = inotify_add_watch(m->top.c2.inotify_fd, 
ks->auth_control_file, IN_CLOSE_WRITE | IN_ONESHOT);
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 9df7552d..f3fe0ecf 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -2466,7 +2466,7 @@ key_method_2_write(struct buffer *buf, struct tls_session 
*session)
  if (session->opt->server && !(session->opt->ncp_enabled
&& session->opt->mode == MODE_SERVER && 
ks->key_id <= 0))
  {
-if (ks->authenticated != KS_AUTH_FALSE)
+if (ks->authenticated > KS_AUTH_FALSE)
  {
  if (!tls_session_generate_data_channel_keys(session))
  {
@@ -2659,7 +2659,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi 
*multi, struct tls_sessio
  secure_memzero(up, sizeof(*up));
  
  /* Perform final authentication checks */

-if (ks->authenticated != KS_AUTH_FALSE)
+if (ks->authenticated > KS_AUTH_FALSE)
  {
  verify_final_auth_checks(multi, session);
  }
@@ -2684,7 +2684,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi 
*multi, struct tls_sessio
   * Call OPENVPN_PLUGIN_TLS_FINAL plugin if defined, for final
   * veto opportunity over authentication decision.
   */
-if ((ks->authenticated != KS_AUTH_FALSE)
+if ((ks->authenticated > KS_AUTH_FALSE)
  && plugin_defined(session->opt->plugins, OPENVPN_PLUGIN_TLS_FINAL))
  {
  key_state_export_keying_material(&ks->ks_ssl, session);
@@ -2715,6 +2715,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi 
*multi, struct tls_sessio
  return true;
  
  error:

+ks->authenticated = KS_AUTH_FALSE;
  secure_memzero(ks->key_src, sizeof(*ks->key_src));
  if (up)
  {
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index fdf589b5..dd82b77d 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -127,10 +127,23 @@ struct key_source2 {
  struct key_source server;   /**< Random provided by server. */
  };
  
+

+/**
+ * This refelects the (server side) state auf authentication after the TLS
+ * session  has been established and key_method_2_read is called. If async auth
+ * is enabled the state will first move to KS_AUTH_DEFERRED before eventually
+ * being set to KS_AUTH_TRUE or KS_AUTH_FALSE
+ * Only KS_AUTH_TRUE is fully authenticated
+ */
  enum ks_auth_state {
-  KS_AUTH_FALSE,
-  KS_AUTH_TRUE,
-  KS_AUTH_DEFERRED
+  KS_AUTH_FALSE,  /**< Key state is not authenticated  */
+  KS_AUTH_DEFERRED,   /**< Key state authentication is being deferred,
+* by async auth */
+  KS_AUTH_TRUE/**< Key sta

Re: [Openvpn-devel] [PATCH 2/8] Make key_state->authenticated more state machine like

2020-07-09 Thread Antonio Quartulli

Hi,

thanks for bearing with me and for adding the requested comment. I
believe the status enum is now easier to grasp and to extend.

Code was basically already reviewed ad this revision did not bring any
functional change with it. "It still looks good".

Tested only on the client side and it did not break anything visible (as
expected).

Acked-by: Antonio Quartulli 

On 09/07/2020 12:15, Arne Schwabe wrote:
> This order the states from unauthenticated to authenticated and also
> changes the comparison for KS_AUTH_FALSE from != to >
> 
> It also add comments and documents part using the state machine
> better.
> 
> Remove a now obsolete comment and two obsolete ifdefs. While
> keeping the ifdef in ssl_verify would save a few bytes of code,
> this is too minor to justify keeping the ifdef
> 
> Signed-off-by: Arne Schwabe 
> ---
>  src/openvpn/multi.c  | 12 +---
>  src/openvpn/ssl.c|  7 ---
>  src/openvpn/ssl_common.h | 22 --
>  src/openvpn/ssl_verify.c | 18 +++---
>  4 files changed, 36 insertions(+), 23 deletions(-)
> 
> diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
> index f1ced9b7..f1332c8d 100644
> --- a/src/openvpn/multi.c
> +++ b/src/openvpn/multi.c
> @@ -2352,12 +2352,12 @@ multi_process_post(struct multi_context *m, struct 
> multi_instance *mi, const uns
>  if (!IS_SIG(&mi->context) && ((flags & MPP_PRE_SELECT) || ((flags & 
> MPP_CONDITIONAL_PRE_SELECT) && !ANY_OUT(&mi->context
>  {
>  #if defined(ENABLE_ASYNC_PUSH) && defined(ENABLE_DEF_AUTH)
> -bool was_authenticated = false;
> +bool was_unauthenticated = true;
>  struct key_state *ks = NULL;
>  if (mi->context.c2.tls_multi)
>  {
>  ks = 
> &mi->context.c2.tls_multi->session[TM_ACTIVE].key[KS_PRIMARY];
> -was_authenticated = ks->authenticated;
> +was_unauthenticated = (ks->authenticated == KS_AUTH_FALSE);
>  }
>  #endif
>  
> @@ -2366,7 +2366,13 @@ multi_process_post(struct multi_context *m, struct 
> multi_instance *mi, const uns
>  pre_select(&mi->context);
>  
>  #if defined(ENABLE_ASYNC_PUSH) && defined(ENABLE_DEF_AUTH)
> -if (ks && ks->auth_control_file && ks->auth_deferred && 
> !was_authenticated)
> +/*
> + * if we see the state transition from unauthenticated to deferred
> + * and a auth_control_file, we assume it got just added and add
> + * inotify watch to that file
> + */
> +if (ks && ks->auth_control_file && was_unauthenticated
> +&& (ks->authenticated == KS_AUTH_DEFERRED))
>  {
>  /* watch acf file */
>  long watch_descriptor = inotify_add_watch(m->top.c2.inotify_fd, 
> ks->auth_control_file, IN_CLOSE_WRITE | IN_ONESHOT);
> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
> index 9df7552d..f3fe0ecf 100644
> --- a/src/openvpn/ssl.c
> +++ b/src/openvpn/ssl.c
> @@ -2466,7 +2466,7 @@ key_method_2_write(struct buffer *buf, struct 
> tls_session *session)
>  if (session->opt->server && !(session->opt->ncp_enabled
>&& session->opt->mode == MODE_SERVER && 
> ks->key_id <= 0))
>  {
> -if (ks->authenticated != KS_AUTH_FALSE)
> +if (ks->authenticated > KS_AUTH_FALSE)
>  {
>  if (!tls_session_generate_data_channel_keys(session))
>  {
> @@ -2659,7 +2659,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi 
> *multi, struct tls_sessio
>  secure_memzero(up, sizeof(*up));
>  
>  /* Perform final authentication checks */
> -if (ks->authenticated != KS_AUTH_FALSE)
> +if (ks->authenticated > KS_AUTH_FALSE)
>  {
>  verify_final_auth_checks(multi, session);
>  }
> @@ -2684,7 +2684,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi 
> *multi, struct tls_sessio
>   * Call OPENVPN_PLUGIN_TLS_FINAL plugin if defined, for final
>   * veto opportunity over authentication decision.
>   */
> -if ((ks->authenticated != KS_AUTH_FALSE)
> +if ((ks->authenticated > KS_AUTH_FALSE)
>  && plugin_defined(session->opt->plugins, OPENVPN_PLUGIN_TLS_FINAL))
>  {
>  key_state_export_keying_material(&ks->ks_ssl, session);
> @@ -2715,6 +2715,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi 
> *multi, struct tls_sessio
>  return true;
>  
>  error:
> +ks->authenticated = KS_AUTH_FALSE;
>  secure_memzero(ks->key_src, sizeof(*ks->key_src));
>  if (up)
>  {
> diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
> index fdf589b5..dd82b77d 100644
> --- a/src/openvpn/ssl_common.h
> +++ b/src/openvpn/ssl_common.h
> @@ -127,10 +127,23 @@ struct key_source2 {
>  struct key_source server;   /**< Random provided by server. */
>  };
>  
> +
> +/**
> + * This refelects the (server side) state auf authentication after the TLS
> + * session  has been established and key_method_2_read is called. If asyn

Re: [Openvpn-devel] [PATCH 2/8] Make key_state->authenticated more state machine like

2020-07-09 Thread tincanteksup


Typo

On 09/07/2020 11:15, Arne Schwabe wrote:

This order the states from unauthenticated to authenticated and also
changes the comparison for KS_AUTH_FALSE from != to >

It also add comments and documents part using the state machine
better.

Remove a now obsolete comment and two obsolete ifdefs. While
keeping the ifdef in ssl_verify would save a few bytes of code,
this is too minor to justify keeping the ifdef

Signed-off-by: Arne Schwabe 
---
  src/openvpn/multi.c  | 12 +---
  src/openvpn/ssl.c|  7 ---
  src/openvpn/ssl_common.h | 22 --
  src/openvpn/ssl_verify.c | 18 +++---
  4 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index f1ced9b7..f1332c8d 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2352,12 +2352,12 @@ multi_process_post(struct multi_context *m, struct 
multi_instance *mi, const uns
  if (!IS_SIG(&mi->context) && ((flags & MPP_PRE_SELECT) || ((flags & 
MPP_CONDITIONAL_PRE_SELECT) && !ANY_OUT(&mi->context
  {
  #if defined(ENABLE_ASYNC_PUSH) && defined(ENABLE_DEF_AUTH)
-bool was_authenticated = false;
+bool was_unauthenticated = true;
  struct key_state *ks = NULL;
  if (mi->context.c2.tls_multi)
  {
  ks = 
&mi->context.c2.tls_multi->session[TM_ACTIVE].key[KS_PRIMARY];
-was_authenticated = ks->authenticated;
+was_unauthenticated = (ks->authenticated == KS_AUTH_FALSE);
  }
  #endif
  
@@ -2366,7 +2366,13 @@ multi_process_post(struct multi_context *m, struct multi_instance *mi, const uns

  pre_select(&mi->context);
  
  #if defined(ENABLE_ASYNC_PUSH) && defined(ENABLE_DEF_AUTH)

-if (ks && ks->auth_control_file && ks->auth_deferred && 
!was_authenticated)
+/*
+ * if we see the state transition from unauthenticated to deferred
+ * and a auth_control_file, we assume it got just added and add
+ * inotify watch to that file
+ */
+if (ks && ks->auth_control_file && was_unauthenticated
+&& (ks->authenticated == KS_AUTH_DEFERRED))
  {
  /* watch acf file */
  long watch_descriptor = inotify_add_watch(m->top.c2.inotify_fd, 
ks->auth_control_file, IN_CLOSE_WRITE | IN_ONESHOT);
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 9df7552d..f3fe0ecf 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -2466,7 +2466,7 @@ key_method_2_write(struct buffer *buf, struct tls_session 
*session)
  if (session->opt->server && !(session->opt->ncp_enabled
&& session->opt->mode == MODE_SERVER && 
ks->key_id <= 0))
  {
-if (ks->authenticated != KS_AUTH_FALSE)
+if (ks->authenticated > KS_AUTH_FALSE)
  {
  if (!tls_session_generate_data_channel_keys(session))
  {
@@ -2659,7 +2659,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi 
*multi, struct tls_sessio
  secure_memzero(up, sizeof(*up));
  
  /* Perform final authentication checks */

-if (ks->authenticated != KS_AUTH_FALSE)
+if (ks->authenticated > KS_AUTH_FALSE)
  {
  verify_final_auth_checks(multi, session);
  }
@@ -2684,7 +2684,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi 
*multi, struct tls_sessio
   * Call OPENVPN_PLUGIN_TLS_FINAL plugin if defined, for final
   * veto opportunity over authentication decision.
   */
-if ((ks->authenticated != KS_AUTH_FALSE)
+if ((ks->authenticated > KS_AUTH_FALSE)
  && plugin_defined(session->opt->plugins, OPENVPN_PLUGIN_TLS_FINAL))
  {
  key_state_export_keying_material(&ks->ks_ssl, session);
@@ -2715,6 +2715,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi 
*multi, struct tls_sessio
  return true;
  
  error:

+ks->authenticated = KS_AUTH_FALSE;
  secure_memzero(ks->key_src, sizeof(*ks->key_src));
  if (up)
  {
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index fdf589b5..dd82b77d 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -127,10 +127,23 @@ struct key_source2 {
  struct key_source server;   /**< Random provided by server. */
  };
  
+

+/**
+ * This refelects the (server side) state auf authentication after the TLS


refelects -> reflects




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

[Openvpn-devel] [PATCH 2/8] Make key_state->authenticated more state machine like

2020-07-09 Thread Arne Schwabe

This order the states from unauthenticated to authenticated and also
changes the comparison for KS_AUTH_FALSE from != to >

It also add comments and documents part using the state machine
better.

Remove a now obsolete comment and two obsolete ifdefs. While
keeping the ifdef in ssl_verify would save a few bytes of code,
this is too minor to justify keeping the ifdef

Signed-off-by: Arne Schwabe 
---
 src/openvpn/multi.c  | 12 +---
 src/openvpn/ssl.c|  7 ---
 src/openvpn/ssl_common.h | 22 --
 src/openvpn/ssl_verify.c | 18 +++---
 4 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index f1ced9b7..f1332c8d 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -2352,12 +2352,12 @@ multi_process_post(struct multi_context *m, struct 
multi_instance *mi, const uns
 if (!IS_SIG(&mi->context) && ((flags & MPP_PRE_SELECT) || ((flags & 
MPP_CONDITIONAL_PRE_SELECT) && !ANY_OUT(&mi->context
 {
 #if defined(ENABLE_ASYNC_PUSH) && defined(ENABLE_DEF_AUTH)
-bool was_authenticated = false;
+bool was_unauthenticated = true;
 struct key_state *ks = NULL;
 if (mi->context.c2.tls_multi)
 {
 ks = &mi->context.c2.tls_multi->session[TM_ACTIVE].key[KS_PRIMARY];
-was_authenticated = ks->authenticated;
+was_unauthenticated = (ks->authenticated == KS_AUTH_FALSE);
 }
 #endif
 
@@ -2366,7 +2366,13 @@ multi_process_post(struct multi_context *m, struct 
multi_instance *mi, const uns
 pre_select(&mi->context);
 
 #if defined(ENABLE_ASYNC_PUSH) && defined(ENABLE_DEF_AUTH)
-if (ks && ks->auth_control_file && ks->auth_deferred && 
!was_authenticated)
+/*
+ * if we see the state transition from unauthenticated to deferred
+ * and a auth_control_file, we assume it got just added and add
+ * inotify watch to that file
+ */
+if (ks && ks->auth_control_file && was_unauthenticated
+&& (ks->authenticated == KS_AUTH_DEFERRED))
 {
 /* watch acf file */
 long watch_descriptor = inotify_add_watch(m->top.c2.inotify_fd, 
ks->auth_control_file, IN_CLOSE_WRITE | IN_ONESHOT);
diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
index 9df7552d..f3fe0ecf 100644
--- a/src/openvpn/ssl.c
+++ b/src/openvpn/ssl.c
@@ -2466,7 +2466,7 @@ key_method_2_write(struct buffer *buf, struct tls_session 
*session)
 if (session->opt->server && !(session->opt->ncp_enabled
   && session->opt->mode == MODE_SERVER && 
ks->key_id <= 0))
 {
-if (ks->authenticated != KS_AUTH_FALSE)
+if (ks->authenticated > KS_AUTH_FALSE)
 {
 if (!tls_session_generate_data_channel_keys(session))
 {
@@ -2659,7 +2659,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi 
*multi, struct tls_sessio
 secure_memzero(up, sizeof(*up));
 
 /* Perform final authentication checks */
-if (ks->authenticated != KS_AUTH_FALSE)
+if (ks->authenticated > KS_AUTH_FALSE)
 {
 verify_final_auth_checks(multi, session);
 }
@@ -2684,7 +2684,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi 
*multi, struct tls_sessio
  * Call OPENVPN_PLUGIN_TLS_FINAL plugin if defined, for final
  * veto opportunity over authentication decision.
  */
-if ((ks->authenticated != KS_AUTH_FALSE)
+if ((ks->authenticated > KS_AUTH_FALSE)
 && plugin_defined(session->opt->plugins, OPENVPN_PLUGIN_TLS_FINAL))
 {
 key_state_export_keying_material(&ks->ks_ssl, session);
@@ -2715,6 +2715,7 @@ key_method_2_read(struct buffer *buf, struct tls_multi 
*multi, struct tls_sessio
 return true;
 
 error:
+ks->authenticated = KS_AUTH_FALSE;
 secure_memzero(ks->key_src, sizeof(*ks->key_src));
 if (up)
 {
diff --git a/src/openvpn/ssl_common.h b/src/openvpn/ssl_common.h
index fdf589b5..dd82b77d 100644
--- a/src/openvpn/ssl_common.h
+++ b/src/openvpn/ssl_common.h
@@ -127,10 +127,23 @@ struct key_source2 {
 struct key_source server;   /**< Random provided by server. */
 };
 
+
+/**
+ * This refelects the (server side) state auf authentication after the TLS
+ * session  has been established and key_method_2_read is called. If async auth
+ * is enabled the state will first move to KS_AUTH_DEFERRED before eventually
+ * being set to KS_AUTH_TRUE or KS_AUTH_FALSE
+ * Only KS_AUTH_TRUE is fully authenticated
+ */
 enum ks_auth_state {
-  KS_AUTH_FALSE,
-  KS_AUTH_TRUE,
-  KS_AUTH_DEFERRED
+  KS_AUTH_FALSE,  /**< Key state is not authenticated  */
+  KS_AUTH_DEFERRED,   /**< Key state authentication is being deferred,
+* by async auth */
+  KS_AUTH_TRUE/**< Key state is authenticated. TLS and 
user/pass
+* succeeded. This include AUTH_PENDING/OOB
+

Re: [Openvpn-devel] [PATCH 2/8] Make key_state->authenticated more state machine like

Re: [Openvpn-devel] [PATCH 2/8] Make key_state->authenticated more state machine like

Re: [Openvpn-devel] [PATCH 2/8] Make key_state->authenticated more state machine like

[Openvpn-devel] [PATCH 2/8] Make key_state->authenticated more state machine like

4 matches

Site Navigation

Mail list logo

Footer information