Re: [Openvpn-devel] [PATCH] rewrite parse_hash_fingerprint()

2021-04-26 Thread Antonio Quartulli
Hi,

On 26/04/2021 19:44, Gert Doering wrote:
> The existing code was doing far too much work for too little
> gain - copying the string segment for scanf(), checking extra
> for spaces, making the result quite unreadable.
> 
> Verify each segment with (short-circuited) isxdigit() checks,
> then feed directly to scanf(), which will stop parsing on ':'
> or end-of-string.
> 
> Rewrite error message to differenciate "hash too short" (including

differenciate -> differenTiate

> number of bytes read) and "hash too long" (it did not terminate when
> we had enough bytes).
> 
> While at it, add an option printer for the resulting o->verify_hash
> list to show_settings().
> 

Thanks for this patch!
Simplifying existing code is as important as introducing new features
and this proves that the project is still well maintained. (Feature-ACK)


> Signed-off-by: Gert Doering 
> ---
>  src/openvpn/options.c | 64 +--
>  1 file changed, 32 insertions(+), 32 deletions(-)
> 
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 2a5b1393..15472878 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -1082,56 +1082,43 @@ string_substitute(const char *src, int from, int to, 
> struct gc_arena *gc)
>  static struct verify_hash_list *
>  parse_hash_fingerprint(const char *str, int nbytes, int msglevel, struct 
> gc_arena *gc)
>  {
> -int i;
> +int i=0;

spaces around the '='

>  const char *cp = str;
>  
>  struct verify_hash_list *ret;
>  ALLOC_OBJ_CLEAR_GC(ret, struct verify_hash_list, gc);
>  
> -char term = 1;
> +char term = 0;
>  int byte;
> -char bs[3];
>  
> -for (i = 0; i < nbytes; ++i)
> +while (*cp && i < nbytes)
>  {
> -if (strlen(cp) < 2)
> +/* valid segments consist of exactly two hex digits, then ':' or EOS 
> */
> +if (!isxdigit(cp[0])
> +|| !isxdigit(cp[1])

since we have a limit of 80 chars per line, how about putting the two
conditions above on the same line?

> +|| (cp[2] != ':' && cp[2] != '\0')
> +|| sscanf(cp, "%x", ) != 1)

same for this two, but less important as they are not really related.

Regarding the format string, we concluded on IRC that %hhx is part of
C99 - why not using it here then?

>  {
>  msg(msglevel, "format error in hash fingerprint: %s", str);
> +break;

We had no break here because this function is always invoked with the
FATAL bit set in the msglevel, therefore the msg() call will terminate
the program.

This said, I prefer moving away from this assumption, so I personally
welcome the break.

>  }
> -bs[0] = *cp++;
> -bs[1] = *cp++;
> -bs[2] = 0;
>  
> -/* the format string "%x" passed to sscanf will ignore any space and
> - * will still try to parse the other character. However, this is not
> - * expected format for a fingerprint, therefore explictly check for
> - * blanks in the string and error out if any is found
> - */
> -if (bs[0] == ' ' || bs[1] == ' ')
> -{
> -msg(msglevel, "format error in hash fingerprint unexpected 
> blank: %s",
> -str);
> -}
> +ret->hash[i++] = (uint8_t)byte;

This cast would go away if byte is declared as uint8_t and we use %hhx
in the sscanf().

>  
> -byte = 0;
> -if (sscanf(bs, "%x", ) != 1)
> -{
> -msg(msglevel, "format error in hash fingerprint hex byte: %s", 
> str);
> -}
> -ret->hash[i] = (uint8_t)byte;
> -term = *cp++;
> -if (term != ':' && term != 0)
> -{
> -msg(msglevel, "format error in hash fingerprint delimiter: %s", 
> str);
> -}
> -if (term == 0)
> +term = cp[2];
> +if (term == '\0')

do we really need "term" at al at this point?
we could directly use cp[2] in the if condition.

>  {
>  break;
>  }
> +cp += 3;
>  }
> -if (term != 0 || i != nbytes-1)
> +if (i < nbytes)
>  {
> -msg(msglevel, "hash fingerprint is different length than expected 
> (%d bytes): %s", nbytes, str);
> +msg(msglevel, "hash fingerprint is wrong length - expected %d bytes, 
> got %d: %s", nbytes, i, str);
> +}
> +else if (term != '\0')

same here: if we hit the else branch, it means that cp[0,1,2] are all
defined so we can again compare cp[2] to '\0'.

> +{
> +msg(msglevel, "hash fingerprint too long - expected only %d bytes: 
> %s", nbytes, str);
>  }
>  return ret;
>  }
> @@ -1791,6 +1778,19 @@ show_settings(const struct options *o)
>  }
>  }
>  SHOW_STR(remote_cert_eku);
> +if (o->verify_hash)
> +{
> +struct gc_arena gc = gc_new();
> +struct verify_hash_list *hl = o->verify_hash;
> +while (hl)
> +{
> +char *s = format_hex_ex(hl->hash, 

[Openvpn-devel] (no subject)

2021-04-26 Thread W- WEi
https://developer.android.com/about/versions/android-4.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH] rewrite parse_hash_fingerprint()

2021-04-26 Thread Gert Doering
The existing code was doing far too much work for too little
gain - copying the string segment for scanf(), checking extra
for spaces, making the result quite unreadable.

Verify each segment with (short-circuited) isxdigit() checks,
then feed directly to scanf(), which will stop parsing on ':'
or end-of-string.

Rewrite error message to differenciate "hash too short" (including
number of bytes read) and "hash too long" (it did not terminate when
we had enough bytes).

While at it, add an option printer for the resulting o->verify_hash
list to show_settings().

Signed-off-by: Gert Doering 
---
 src/openvpn/options.c | 64 +--
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 2a5b1393..15472878 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -1082,56 +1082,43 @@ string_substitute(const char *src, int from, int to, 
struct gc_arena *gc)
 static struct verify_hash_list *
 parse_hash_fingerprint(const char *str, int nbytes, int msglevel, struct 
gc_arena *gc)
 {
-int i;
+int i=0;
 const char *cp = str;
 
 struct verify_hash_list *ret;
 ALLOC_OBJ_CLEAR_GC(ret, struct verify_hash_list, gc);
 
-char term = 1;
+char term = 0;
 int byte;
-char bs[3];
 
-for (i = 0; i < nbytes; ++i)
+while (*cp && i < nbytes)
 {
-if (strlen(cp) < 2)
+/* valid segments consist of exactly two hex digits, then ':' or EOS */
+if (!isxdigit(cp[0])
+|| !isxdigit(cp[1])
+|| (cp[2] != ':' && cp[2] != '\0')
+|| sscanf(cp, "%x", ) != 1)
 {
 msg(msglevel, "format error in hash fingerprint: %s", str);
+break;
 }
-bs[0] = *cp++;
-bs[1] = *cp++;
-bs[2] = 0;
 
-/* the format string "%x" passed to sscanf will ignore any space and
- * will still try to parse the other character. However, this is not
- * expected format for a fingerprint, therefore explictly check for
- * blanks in the string and error out if any is found
- */
-if (bs[0] == ' ' || bs[1] == ' ')
-{
-msg(msglevel, "format error in hash fingerprint unexpected blank: 
%s",
-str);
-}
+ret->hash[i++] = (uint8_t)byte;
 
-byte = 0;
-if (sscanf(bs, "%x", ) != 1)
-{
-msg(msglevel, "format error in hash fingerprint hex byte: %s", 
str);
-}
-ret->hash[i] = (uint8_t)byte;
-term = *cp++;
-if (term != ':' && term != 0)
-{
-msg(msglevel, "format error in hash fingerprint delimiter: %s", 
str);
-}
-if (term == 0)
+term = cp[2];
+if (term == '\0')
 {
 break;
 }
+cp += 3;
 }
-if (term != 0 || i != nbytes-1)
+if (i < nbytes)
 {
-msg(msglevel, "hash fingerprint is different length than expected (%d 
bytes): %s", nbytes, str);
+msg(msglevel, "hash fingerprint is wrong length - expected %d bytes, 
got %d: %s", nbytes, i, str);
+}
+else if (term != '\0')
+{
+msg(msglevel, "hash fingerprint too long - expected only %d bytes: 
%s", nbytes, str);
 }
 return ret;
 }
@@ -1791,6 +1778,19 @@ show_settings(const struct options *o)
 }
 }
 SHOW_STR(remote_cert_eku);
+if (o->verify_hash)
+{
+struct gc_arena gc = gc_new();
+struct verify_hash_list *hl = o->verify_hash;
+while (hl)
+{
+char *s = format_hex_ex(hl->hash, sizeof(hl->hash), 0,
+1, ":", );
+SHOW_PARM(verify_hash, s, "%s");
+hl = hl->next;
+}
+gc_free();
+}
 SHOW_INT(ssl_flags);
 
 SHOW_INT(tls_timeout);
-- 
2.26.3



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


[Openvpn-devel] [PATCH v4] Add documentation on EVENT_READ/EVENT_WRITE constants

2021-04-26 Thread Antonio Quartulli
From: Antonio Quartulli 

Changes from v3:
- re-introduce READ/WRITE_SHIFT because they are different from EVENT_READ/WRITE
- define also EVENT_READ/WRITE using READ/WRITE_SHIFT

Changes from v2:
- moved event definitions to event.h
- removed READ/WRITE_SHIFT and use EVENT_READ/WRITE
- removed ifdefs around *_SHIFTS definitions in event.h

Changes from v1:
- fixed typ0s
- extended comment
- moved *_SHIFT definition to openvpn.h
- made READ/WRITE events dependant on _SHIFT definition with a macro

Signed-off-by: Antonio Quartulli 
---
 src/openvpn/event.h   | 44 +--
 src/openvpn/forward.c | 14 --
 src/openvpn/openvpn.h | 12 +---
 3 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/src/openvpn/event.h b/src/openvpn/event.h
index 4af6371e..bfc3d51f 100644
--- a/src/openvpn/event.h
+++ b/src/openvpn/event.h
@@ -32,9 +32,49 @@
  * rwflags passed to event_ctl and returned by
  * struct event_set_return.
  */
+#define READ_SHIFT 0
+#define WRITE_SHIFT1
+
 #define EVENT_UNDEF4
-#define EVENT_READ (1<<0)
-#define EVENT_WRITE(1<<1)
+#define EVENT_READ (1 << READ_SHIFT)
+#define EVENT_WRITE(1 << WRITE_SHIFT)
+
+/* event flags returned by io_wait.
+ *
+ * All these events are defined as bits in a bitfield.
+ * Each event type owns two bits in the bitfield: one for the READ
+ * event and one for the WRITE event.
+ *
+ * For this reason, the specific event bit is calculated by adding
+ * the event type identifier (always a multiple of 2, as defined
+ * below) to 0 for READ and 1 for WRITE.
+ *
+ * E.g.
+ * MANAGEMENT_SHIFT = 6;  < event type identifier
+ * MANAGEMENT_READ = (1 << (6 + 0)),  < READ event
+ * MANAGEMENT_WRITE = (1 << (6 + 1))  < WRITE event
+ *
+ * 'error' and 'file_close' are special and use read/write for different
+ * signals.
+ */
+
+#define EVENT_SHIFT(_name, _op)   (1 << (_name ## _SHIFT + _op ## _SHIFT))
+
+#define SOCKET_SHIFT0
+#define SOCKET_READ EVENT_SHIFT(SOCKET, READ)
+#define SOCKET_WRITEEVENT_SHIFT(SOCKET, WRITE)
+#define TUN_SHIFT   2
+#define TUN_READEVENT_SHIFT(TUN, READ)
+#define TUN_WRITE   EVENT_SHIFT(TUN, WRITE)
+#define ERR_SHIFT   4
+#define ES_ERROREVENT_SHIFT(ERR, READ)
+#define ES_TIMEOUT  EVENT_SHIFT(ERR, WRITE)
+#define MANAGEMENT_SHIFT6
+#define MANAGEMENT_READ EVENT_SHIFT(MANAGEMENT, READ)
+#define MANAGEMENT_WRITEEVENT_SHIFT(MANAGEMENT, WRITE)
+#define FILE_SHIFT  8
+#define FILE_CLOSED EVENT_SHIFT(FILE, READ)
+
 /*
  * Initialization flags passed to event_set_init
  */
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index df96d048..e302d8e0 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -1867,15 +1867,17 @@ io_wait_dowork(struct context *c, const unsigned int 
flags)
 unsigned int tuntap = 0;
 struct event_set_return esr[4];
 
-/* These shifts all depend on EVENT_READ and EVENT_WRITE */
-static int socket_shift = 0;   /* depends on SOCKET_READ and SOCKET_WRITE 
*/
-static int tun_shift = 2;  /* depends on TUN_READ and TUN_WRITE */
-static int err_shift = 4;  /* depends on ES_ERROR */
+/* These shifts all depend on EVENT_READ (=1) and EVENT_WRITE (=2)
+ * and are added to the shift. Check openvpn.h for more details.
+ */
+static int socket_shift = SOCKET_SHIFT;
+static int tun_shift = TUN_SHIFT;
+static int err_shift = ERR_SHIFT;
 #ifdef ENABLE_MANAGEMENT
-static int management_shift = 6; /* depends on MANAGEMENT_READ and 
MANAGEMENT_WRITE */
+static int management_shift = MANAGEMENT_SHIFT;
 #endif
 #ifdef ENABLE_ASYNC_PUSH
-static int file_shift = 8; /* listening inotify events */
+static int file_shift = FILE_SHIFT;
 #endif
 
 /*
diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
index a15ff08d..6e4651cf 100644
--- a/src/openvpn/openvpn.h
+++ b/src/openvpn/openvpn.h
@@ -232,17 +232,7 @@ struct context_2
 int event_set_max;
 bool event_set_owned;
 
-/* event flags returned by io_wait */
-#define SOCKET_READ   (1<<0)
-#define SOCKET_WRITE  (1<<1)
-#define TUN_READ  (1<<2)
-#define TUN_WRITE (1<<3)
-#define ES_ERROR  (1<<4)
-#define ES_TIMEOUT(1<<5)
-#define MANAGEMENT_READ  (1<<6)
-#define MANAGEMENT_WRITE (1<<7)
-#define FILE_CLOSED   (1<<8)
-
+/* bitmask for event status. Check event.h for possible values */
 unsigned int event_set_status;
 
 struct link_socket *link_socket; /* socket used for TCP/UDP connection 
to remote */
-- 
2.26.3



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


[Openvpn-devel] [PATCH applied] Re: options: check for blanks in fingerprints and reject string if found

2021-04-26 Thread Gert Doering
Your patch has been applied to the master branch.

That said, I find this function pretty insane, tbh.  Parsing a sequence
of double-digit hex bytes should be doable in half the lines of code,
and without needing to copy bytes to temp buffers to pass to scanf()...
(https://stackoverflow.com/questions/10156409/convert-hex-string-char-to-int)

commit 9e71cf13134c10bb7e952e3dadbcc79884f60b93
Author: Antonio Quartulli
Date:   Thu Apr 22 01:49:08 2021 +0200

 options: check for blanks in fingerprints and reject string if found

 Signed-off-by: Antonio Quartulli 
 Acked-by: Arne Schwabe 
 Message-Id: <20210421234908.12817-...@unstable.cc>
 URL: 
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg22182.html
 Signed-off-by: Gert Doering 


--
kind regards,

Gert Doering



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


Re: [Openvpn-devel] [PATCH v3] Add documentation on EVENT_READ/EVENT_WRITE constants

2021-04-26 Thread Antonio Quartulli
Hi,

unfortunately something is not right.
Additional tests showed that something broke after applying this patch.

Will debug and provide a new version.

Regards,

On 26/04/2021 15:26, Antonio Quartulli wrote:
> From: Arne Schwabe 
> 
> Changes from v2:
> - moved event definitions to event.h
> - removed READ/WRITE_SHIFT and use EVENT_READ/WRITE
> - removed ifdefs around *_SHIFTS definitions in event.h
> 
> Changes from v1:
> - fixed typ0s
> - extended comment
> - moved *_SHIFT definition to openvpn.h
> - made READ/WRITE events dependant on _SHIFT definition with a macro
> 
> Signed-off-by: Arne Schwabe 
> Signed-off-by: Antonio Quartulli 
> ---
>  src/openvpn/event.h   | 37 +
>  src/openvpn/forward.c | 14 --
>  src/openvpn/openvpn.h | 12 +---
>  3 files changed, 46 insertions(+), 17 deletions(-)
> 
> diff --git a/src/openvpn/event.h b/src/openvpn/event.h
> index 4af6371e..67baea3e 100644
> --- a/src/openvpn/event.h
> +++ b/src/openvpn/event.h
> @@ -35,6 +35,43 @@
>  #define EVENT_UNDEF4
>  #define EVENT_READ (1<<0)
>  #define EVENT_WRITE(1<<1)
> +
> +/* event flags returned by io_wait.
> + *
> + * All these events are defined as bits in a bitfield.
> + * Each event type owns two bits in the bitfield: one for the READ
> + * event and one for the WRITE event.
> + *
> + * For this reason, the specific event bit is calculated by adding
> + * the event type identifier (always a multiple of 2, as defined
> + * below) to 0 for READ and 1 for WRITE.
> + *
> + * E.g.
> + * MANAGEMENT_SHIFT = 6;  < event type identifier
> + * MANAGEMENT_READ = (1 << (6 + 0)),  < READ event
> + * MANAGEMENT_WRITE = (1 << (6 + 1))  < WRITE event
> + *
> + * 'error' and 'file_close' are special and use read/write for different
> + * signals.
> + */
> +
> +#define EVENT_SHIFT(_name, _op)   (1 << (_name ## _SHIFT + EVENT_ ## _op))
> +
> +#define SOCKET_SHIFT0
> +#define SOCKET_READ EVENT_SHIFT(SOCKET, READ)
> +#define SOCKET_WRITEEVENT_SHIFT(SOCKET, WRITE)
> +#define TUN_SHIFT   2
> +#define TUN_READEVENT_SHIFT(TUN, READ)
> +#define TUN_WRITE   EVENT_SHIFT(TUN, WRITE)
> +#define ERR_SHIFT   4
> +#define ES_ERROREVENT_SHIFT(ERR, READ)
> +#define ES_TIMEOUT  EVENT_SHIFT(ERR, WRITE)
> +#define MANAGEMENT_SHIFT6
> +#define MANAGEMENT_READ EVENT_SHIFT(MANAGEMENT, READ)
> +#define MANAGEMENT_WRITEEVENT_SHIFT(MANAGEMENT, WRITE)
> +#define FILE_SHIFT  8
> +#define FILE_CLOSED EVENT_SHIFT(FILE, READ)
> +
>  /*
>   * Initialization flags passed to event_set_init
>   */
> diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
> index df96d048..e302d8e0 100644
> --- a/src/openvpn/forward.c
> +++ b/src/openvpn/forward.c
> @@ -1867,15 +1867,17 @@ io_wait_dowork(struct context *c, const unsigned int 
> flags)
>  unsigned int tuntap = 0;
>  struct event_set_return esr[4];
>  
> -/* These shifts all depend on EVENT_READ and EVENT_WRITE */
> -static int socket_shift = 0;   /* depends on SOCKET_READ and 
> SOCKET_WRITE */
> -static int tun_shift = 2;  /* depends on TUN_READ and TUN_WRITE */
> -static int err_shift = 4;  /* depends on ES_ERROR */
> +/* These shifts all depend on EVENT_READ (=1) and EVENT_WRITE (=2)
> + * and are added to the shift. Check openvpn.h for more details.
> + */
> +static int socket_shift = SOCKET_SHIFT;
> +static int tun_shift = TUN_SHIFT;
> +static int err_shift = ERR_SHIFT;
>  #ifdef ENABLE_MANAGEMENT
> -static int management_shift = 6; /* depends on MANAGEMENT_READ and 
> MANAGEMENT_WRITE */
> +static int management_shift = MANAGEMENT_SHIFT;
>  #endif
>  #ifdef ENABLE_ASYNC_PUSH
> -static int file_shift = 8; /* listening inotify events */
> +static int file_shift = FILE_SHIFT;
>  #endif
>  
>  /*
> diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
> index a15ff08d..6e4651cf 100644
> --- a/src/openvpn/openvpn.h
> +++ b/src/openvpn/openvpn.h
> @@ -232,17 +232,7 @@ struct context_2
>  int event_set_max;
>  bool event_set_owned;
>  
> -/* event flags returned by io_wait */
> -#define SOCKET_READ   (1<<0)
> -#define SOCKET_WRITE  (1<<1)
> -#define TUN_READ  (1<<2)
> -#define TUN_WRITE (1<<3)
> -#define ES_ERROR  (1<<4)
> -#define ES_TIMEOUT(1<<5)
> -#define MANAGEMENT_READ  (1<<6)
> -#define MANAGEMENT_WRITE (1<<7)
> -#define FILE_CLOSED   (1<<8)
> -
> +/* bitmask for event status. Check event.h for possible values */
>  unsigned int event_set_status;
>  
>  struct link_socket *link_socket; /* socket used for TCP/UDP 
> connection to remote */
> 

-- 
Antonio Quartulli


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


Re: [Openvpn-devel] [PATCH v3] Add documentation on EVENT_READ/EVENT_WRITE constants

2021-04-26 Thread Arne Schwabe
Am 26.04.21 um 15:26 schrieb Antonio Quartulli:
> From: Arne Schwabe 
> 
> Changes from v2:
> - moved event definitions to event.h
> - removed READ/WRITE_SHIFT and use EVENT_READ/WRITE
> - removed ifdefs around *_SHIFTS definitions in event.h
> 
> Changes from v1:
> - fixed typ0s
> - extended comment
> - moved *_SHIFT definition to openvpn.h
> - made READ/WRITE events dependant on _SHIFT definition with a macro
> 
> Signed-off-by: Arne Schwabe 
> Signed-off-by: Antonio Quartulli 

Thanks for picking it up and getting it in shape. It probably should now
say From: Antonio Quartulli  as that is now most of the
code. That also looks better on the commit log as it doesn't look like I
acked my own patch.

Acked-By: Arne Schwabe 


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


[Openvpn-devel] [PATCH v3] Add documentation on EVENT_READ/EVENT_WRITE constants

2021-04-26 Thread Antonio Quartulli
From: Arne Schwabe 

Changes from v2:
- moved event definitions to event.h
- removed READ/WRITE_SHIFT and use EVENT_READ/WRITE
- removed ifdefs around *_SHIFTS definitions in event.h

Changes from v1:
- fixed typ0s
- extended comment
- moved *_SHIFT definition to openvpn.h
- made READ/WRITE events dependant on _SHIFT definition with a macro

Signed-off-by: Arne Schwabe 
Signed-off-by: Antonio Quartulli 
---
 src/openvpn/event.h   | 37 +
 src/openvpn/forward.c | 14 --
 src/openvpn/openvpn.h | 12 +---
 3 files changed, 46 insertions(+), 17 deletions(-)

diff --git a/src/openvpn/event.h b/src/openvpn/event.h
index 4af6371e..67baea3e 100644
--- a/src/openvpn/event.h
+++ b/src/openvpn/event.h
@@ -35,6 +35,43 @@
 #define EVENT_UNDEF4
 #define EVENT_READ (1<<0)
 #define EVENT_WRITE(1<<1)
+
+/* event flags returned by io_wait.
+ *
+ * All these events are defined as bits in a bitfield.
+ * Each event type owns two bits in the bitfield: one for the READ
+ * event and one for the WRITE event.
+ *
+ * For this reason, the specific event bit is calculated by adding
+ * the event type identifier (always a multiple of 2, as defined
+ * below) to 0 for READ and 1 for WRITE.
+ *
+ * E.g.
+ * MANAGEMENT_SHIFT = 6;  < event type identifier
+ * MANAGEMENT_READ = (1 << (6 + 0)),  < READ event
+ * MANAGEMENT_WRITE = (1 << (6 + 1))  < WRITE event
+ *
+ * 'error' and 'file_close' are special and use read/write for different
+ * signals.
+ */
+
+#define EVENT_SHIFT(_name, _op)   (1 << (_name ## _SHIFT + EVENT_ ## _op))
+
+#define SOCKET_SHIFT0
+#define SOCKET_READ EVENT_SHIFT(SOCKET, READ)
+#define SOCKET_WRITEEVENT_SHIFT(SOCKET, WRITE)
+#define TUN_SHIFT   2
+#define TUN_READEVENT_SHIFT(TUN, READ)
+#define TUN_WRITE   EVENT_SHIFT(TUN, WRITE)
+#define ERR_SHIFT   4
+#define ES_ERROREVENT_SHIFT(ERR, READ)
+#define ES_TIMEOUT  EVENT_SHIFT(ERR, WRITE)
+#define MANAGEMENT_SHIFT6
+#define MANAGEMENT_READ EVENT_SHIFT(MANAGEMENT, READ)
+#define MANAGEMENT_WRITEEVENT_SHIFT(MANAGEMENT, WRITE)
+#define FILE_SHIFT  8
+#define FILE_CLOSED EVENT_SHIFT(FILE, READ)
+
 /*
  * Initialization flags passed to event_set_init
  */
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index df96d048..e302d8e0 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -1867,15 +1867,17 @@ io_wait_dowork(struct context *c, const unsigned int 
flags)
 unsigned int tuntap = 0;
 struct event_set_return esr[4];
 
-/* These shifts all depend on EVENT_READ and EVENT_WRITE */
-static int socket_shift = 0;   /* depends on SOCKET_READ and SOCKET_WRITE 
*/
-static int tun_shift = 2;  /* depends on TUN_READ and TUN_WRITE */
-static int err_shift = 4;  /* depends on ES_ERROR */
+/* These shifts all depend on EVENT_READ (=1) and EVENT_WRITE (=2)
+ * and are added to the shift. Check openvpn.h for more details.
+ */
+static int socket_shift = SOCKET_SHIFT;
+static int tun_shift = TUN_SHIFT;
+static int err_shift = ERR_SHIFT;
 #ifdef ENABLE_MANAGEMENT
-static int management_shift = 6; /* depends on MANAGEMENT_READ and 
MANAGEMENT_WRITE */
+static int management_shift = MANAGEMENT_SHIFT;
 #endif
 #ifdef ENABLE_ASYNC_PUSH
-static int file_shift = 8; /* listening inotify events */
+static int file_shift = FILE_SHIFT;
 #endif
 
 /*
diff --git a/src/openvpn/openvpn.h b/src/openvpn/openvpn.h
index a15ff08d..6e4651cf 100644
--- a/src/openvpn/openvpn.h
+++ b/src/openvpn/openvpn.h
@@ -232,17 +232,7 @@ struct context_2
 int event_set_max;
 bool event_set_owned;
 
-/* event flags returned by io_wait */
-#define SOCKET_READ   (1<<0)
-#define SOCKET_WRITE  (1<<1)
-#define TUN_READ  (1<<2)
-#define TUN_WRITE (1<<3)
-#define ES_ERROR  (1<<4)
-#define ES_TIMEOUT(1<<5)
-#define MANAGEMENT_READ  (1<<6)
-#define MANAGEMENT_WRITE (1<<7)
-#define FILE_CLOSED   (1<<8)
-
+/* bitmask for event status. Check event.h for possible values */
 unsigned int event_set_status;
 
 struct link_socket *link_socket; /* socket used for TCP/UDP connection 
to remote */
-- 
2.26.3



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


Re: [Openvpn-devel] [PATCH] options: check for blanks in fingerprints and reject string if found

2021-04-26 Thread Arne Schwabe
Am 22.04.21 um 01:49 schrieb Antonio Quartulli:
> From: Antonio Quartulli 
> 
> A fingerprint is not expected to contains any blank (white space),
> howeveri, the parser routine will still attempt parsing the octect
> and ignore the space.
> 
> This means that a fingerprint like
> 5 
> :F0:A8:75:70:46:6E:0B:A2:31:53:88:0B:0E:8C:E4:8A:5E:BF:1E:08:16:16:41:63:2C:B5:F4:D2:73:9F:E5
> will be parsed successfully.
> 
> Explcitly check for spaces in the various octects, before conversion,
> and error out if any is found.

Explicitly

> 
> Signed-off-by: Antonio Quartulli 
> ---
>  src/openvpn/options.c | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 817a1533..264fe383 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -1099,6 +1099,18 @@ parse_hash_fingerprint(const char *str, int nbytes, 
> int msglevel, struct gc_aren
>  bs[0] = *cp++;
>  bs[1] = *cp++;
>  bs[2] = 0;
> +
> +/* the format string "%x" passed to sscanf will ignore any space and
> + * will still try to parse the other character. However, this is not
> + * expected format for a fingerprint, therefore explcitly check for

explicitly

> + * blanks in the string and error out if any is found
> + */
> +if (bs[0] == ' ' || bs[1] == ' ')
> +{
> +msg(msglevel, "format error in hash fingerprint unexpected 
> blank: %s",
> +str);
> +}
> +
>  byte = 0;
>  if (sscanf(bs, "%x", ) != 1)
>  {
> 

I would not have spend the time to fix this but since Antonio done it:

Acked-By: Arne Schwabe 


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