[Openvpn-devel,2/7] compat-mode: allow user to specify version to be compatible with

  This changes introduces the basic inbfrastructure required
to allow the user to specify a specific OpenVPN version to be
compatible with.

Following changes will modify defaults to more modern and safer
values, while allowing backwards-compatible behaviour on demand.

The backwards-compatible behaviour is intructed via the config
knob '--compat-mode' implemented in this patch.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Signed-off-by: Antonio Quartulli <a@unstable.cc>
---
 Changes.rst                          |  6 +++++
 doc/man-sections/generic-options.rst |  9 +++++++
 src/openvpn/options.c                | 37 ++++++++++++++++++++++++++++
 src/openvpn/options.h                |  4 +++
 4 files changed, 56 insertions(+)
  

On Sat, Sep 4, 2021 at 9:57 AM Antonio Quartulli <a@unstable.cc> wrote:

> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 0d6b85cf..4d971a56 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -3125,6 +3125,29 @@ options_postprocess_cipher(struct options *o)
>      }
>  }
>
> +/**
> + * Returns if we want 'backwards-compatibility' up to (but not included) a
> + * certain version
> + *
> + * @param version   the oldest version that does not compatibility
>

I feel like you dropped a word in here ^^

> + *                  e.g. 20400 for all versions < 2.4.0
> + * @return          whether compatibility should be enabled
> + */
> +static bool
> +need_compatibility_before(const struct options *o, int version)
>

I feel like you want to do `unsigned int` on your version parameter here
(why would a version go negative?) ...



> +{
> +    return o->backwards_compatible != 0 && o->backwards_compatible <
> version;
> +}
> +
> +/**
> + * Changes default values so that OpenVPN can be compatible with the user
> + * specified version
> + */
> +static void
> +options_set_backwards_compatible_options(struct options *o)
> +{
> +}
> +
>  static void
>  options_postprocess_mutate(struct options *o)
>  {
> @@ -3137,6 +3160,8 @@ options_postprocess_mutate(struct options *o)
>      helper_keepalive(o);
>      helper_tcp_nodelay(o);
>
> +    options_set_backwards_compatible_options(o);
> +
>      options_postprocess_cipher(o);
>      options_postprocess_mutate_invariant(o);
>
> @@ -6698,6 +6723,18 @@ add_option(struct options *options,
>              setenv_str(es, p[1], p[2] ? p[2] : "");
>          }
>      }
> +    else if (streq(p[0], "compat-mode") && p[1] && !p[3])
> +    {
> +        unsigned int major, minor, patch;
> +        if (!(sscanf(p[1], "%u.%u.%u", &major, &minor, &patch) == 3))
> +        {
> +            msg(msglevel, "cannot parse version number for --compat-mode:
> %s",
> +                p[1]);
> +            goto err;
> +        }
> +
> +        options->backwards_compatible = major * 10000 + minor * 100 +
> patch;
> +    }
>      else if (streq(p[0], "setenv-safe") && p[1] && !p[3])
>      {
>          VERIFY_PERMISSION(OPT_P_SETENV);
> diff --git a/src/openvpn/options.h b/src/openvpn/options.h
> index b0e40cb7..98c21a2a 100644
> --- a/src/openvpn/options.h
> +++ b/src/openvpn/options.h
> @@ -225,6 +225,10 @@ struct options
>
>      /* enable forward compatibility for post-2.1 features */
>      bool forward_compatible;
> +    /** What version we should try to be compatible with as major * 10000
> +
> +      * minor * 100 + patch, e.g. 2.4.7 => 20407 */
> +    unsigned int backwards_compatible;
>

... doubly so because you use `unsigned int` (imo rightly) here.

> +
>      /* list of options that should be ignored even if unknown */
>      const char **ignore_unknown_option;
>
> --
> 2.32.0
>
>
>
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
>
<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-family:courier new,monospace"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sat, Sep 4, 2021 at 9:57 AM Antonio Quartulli &lt;a@unstable.cc&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">diff --git a/src/openvpn/options.c b/src/openvpn/options.c<br>
index 0d6b85cf..4d971a56 100644<br>
--- a/src/openvpn/options.c<br>
+++ b/src/openvpn/options.c<br>
@@ -3125,6 +3125,29 @@ options_postprocess_cipher(struct options *o)<br>
     }<br>
 }<br>
<br>
+/**<br>
+ * Returns if we want &#39;backwards-compatibility&#39; up to (but not included) a<br>
+ * certain version<br>
+ *<br>
+ * @param version   the oldest version that does not compatibility<br>
</blockquote><div><br></div><div style="font-family:courier new,monospace" class="gmail_default">I feel like you dropped a word in here ^^</div><div style="font-family:courier new,monospace" class="gmail_default"></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+ *                  e.g. 20400 for all versions &lt; 2.4.0<br>
+ * @return          whether compatibility should be enabled<br>
+ */<br>
+static bool<br>
+need_compatibility_before(const struct options *o, int version)<br>
</blockquote><div><br></div><div><div style="font-family:courier new,monospace" class="gmail_default">I feel like you want to do `unsigned int` on your version parameter here (why would a version go negative?) ...<br></div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">+{<br>
+    return o-&gt;backwards_compatible != 0 &amp;&amp; o-&gt;backwards_compatible &lt; version;<br>
+}<br>
+<br>
+/**<br>
+ * Changes default values so that OpenVPN can be compatible with the user<br>
+ * specified version<br>
+ */<br>
+static void<br>
+options_set_backwards_compatible_options(struct options *o)<br>
+{<br>
+}<br>
+<br>
 static void<br>
 options_postprocess_mutate(struct options *o)<br>
 {<br>
@@ -3137,6 +3160,8 @@ options_postprocess_mutate(struct options *o)<br>
     helper_keepalive(o);<br>
     helper_tcp_nodelay(o);<br>
<br>
+    options_set_backwards_compatible_options(o);<br>
+<br>
     options_postprocess_cipher(o);<br>
     options_postprocess_mutate_invariant(o);<br>
<br>
@@ -6698,6 +6723,18 @@ add_option(struct options *options,<br>
             setenv_str(es, p[1], p[2] ? p[2] : &quot;&quot;);<br>
         }<br>
     }<br>
+    else if (streq(p[0], &quot;compat-mode&quot;) &amp;&amp; p[1] &amp;&amp; !p[3])<br>
+    {<br>
+        unsigned int major, minor, patch;<br>
+        if (!(sscanf(p[1], &quot;%u.%u.%u&quot;, &amp;major, &amp;minor, &amp;patch) == 3))<br>
+        {<br>
+            msg(msglevel, &quot;cannot parse version number for --compat-mode: %s&quot;,<br>
+                p[1]);<br>
+            goto err;<br>
+        }<br>
+<br>
+        options-&gt;backwards_compatible = major * 10000 + minor * 100 + patch;<br>
+    }<br>
     else if (streq(p[0], &quot;setenv-safe&quot;) &amp;&amp; p[1] &amp;&amp; !p[3])<br>
     {<br>
         VERIFY_PERMISSION(OPT_P_SETENV);<br>
diff --git a/src/openvpn/options.h b/src/openvpn/options.h<br>
index b0e40cb7..98c21a2a 100644<br>
--- a/src/openvpn/options.h<br>
+++ b/src/openvpn/options.h<br>
@@ -225,6 +225,10 @@ struct options<br>
<br>
     /* enable forward compatibility for post-2.1 features */<br>
     bool forward_compatible;<br>
+    /** What version we should try to be compatible with as major * 10000 +<br>
+      * minor * 100 + patch, e.g. 2.4.7 =&gt; 20407 */<br>
+    unsigned int backwards_compatible;<br></blockquote><div><br></div><div style="font-family:courier new,monospace" class="gmail_default">... doubly so because you use `unsigned int` (imo rightly) here.</div><div style="font-family:courier new,monospace" class="gmail_default"></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
     /* list of options that should be ignored even if unknown */<br>
     const char **ignore_unknown_option;<br>
<br>
-- <br>
2.32.0<br>
<br>
<br>
<br>
_______________________________________________<br>
Openvpn-devel mailing list<br>
<a href="mailto:Openvpn-devel@lists.sourceforge.net" target="_blank">Openvpn-devel@lists.sourceforge.net</a><br>
<a href="https://lists.sourceforge.net/lists/listinfo/openvpn-devel" rel="noreferrer" target="_blank">https://lists.sourceforge.net/lists/listinfo/openvpn-devel</a><br>
</blockquote></div></div>
  

Am 04.09.21 um 11:56 schrieb Antonio Quartulli:
> This changes introduces the basic inbfrastructure required
                                    typo

> to allow the user to specify a specific OpenVPN version to be
> compatible with.
> 
> Following changes will modify defaults to more modern and safer
> values, while allowing backwards-compatible behaviour on demand.
> 
> The backwards-compatible behaviour is intructed via the config
> knob '--compat-mode' implemented in this patch.
> 
> Signed-off-by: Arne Schwabe <arne@rfc2549.org>
> Signed-off-by: Antonio Quartulli <a@unstable.cc>
> ---
>  Changes.rst                          |  6 +++++
>  doc/man-sections/generic-options.rst |  9 +++++++
>  src/openvpn/options.c                | 37 ++++++++++++++++++++++++++++
>  src/openvpn/options.h                |  4 +++
>  4 files changed, 56 insertions(+)
> 
> diff --git a/Changes.rst b/Changes.rst
> index 0323a7f7..f55b0e3e 100644
> --- a/Changes.rst
> +++ b/Changes.rst
> @@ -45,6 +45,12 @@ Pending auth support for plugins and scripts
>  
>      See ``sample/sample-scripts/totpauth.py`` for an example.
>  
> +Compatibility mode (``--compat-mode``)
> +    The modernisation of defaults can impact the compatibility of OpenVPN 2.6.0
> +    with older peers. The options ``--compat-mode`` allows UIs to provide users
> +    with an easy way to still connect to older servers.
> +
> +
>  Deprecated features
>  -------------------
>  ``inetd`` has been removed
> diff --git a/doc/man-sections/generic-options.rst b/doc/man-sections/generic-options.rst
> index db39f6e2..63c6227c 100644
> --- a/doc/man-sections/generic-options.rst
> +++ b/doc/man-sections/generic-options.rst
> @@ -52,6 +52,15 @@ which mode OpenVPN is configured as.
>    BSDs implement a getrandom() or getentropy() syscall that removes the
>    need for /dev/urandom to be available.
>  
> +--compat-mode version
> +  This option provides a way to alter the default of OpenVPN to be more
> +  compatible with the version ``version`` specified. All of the changes
> +  this option does can also be achieved using individual configuration
> +  options.
> +
> +  Note: Using this option reverts defaults to no longer recommended
> +  values and should be avoided if possible.
> +
>  --config file
>    Load additional config options from ``file`` where each line corresponds
>    to one command line option, but with the leading '--' removed.
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 0d6b85cf..4d971a56 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -3125,6 +3125,29 @@ options_postprocess_cipher(struct options *o)
>      }
>  }
>  
> +/**
> + * Returns if we want 'backwards-compatibility' up to (but not included) a
> + * certain version
> + *
> + * @param version   the oldest version that does not compatibility
> + *                  e.g. 20400 for all versions < 2.4.0
> + * @return          whether compatibility should be enabled
> + */
> +static bool
> +need_compatibility_before(const struct options *o, int version)
> +{
> +    return o->backwards_compatible != 0 && o->backwards_compatible < version;
> +}
> +
> +/**
> + * Changes default values so that OpenVPN can be compatible with the user
> + * specified version
> + */
> +static void
> +options_set_backwards_compatible_options(struct options *o)
> +{
> +}
> +
>  static void
>  options_postprocess_mutate(struct options *o)
>  {
> @@ -3137,6 +3160,8 @@ options_postprocess_mutate(struct options *o)
>      helper_keepalive(o);
>      helper_tcp_nodelay(o);
>  
> +    options_set_backwards_compatible_options(o);
> +
>      options_postprocess_cipher(o);
>      options_postprocess_mutate_invariant(o);
>  
> @@ -6698,6 +6723,18 @@ add_option(struct options *options,
>              setenv_str(es, p[1], p[2] ? p[2] : "");
>          }
>      }
> +    else if (streq(p[0], "compat-mode") && p[1] && !p[3])
> +    {
> +        unsigned int major, minor, patch;
> +        if (!(sscanf(p[1], "%u.%u.%u", &major, &minor, &patch) == 3))
> +        {
> +            msg(msglevel, "cannot parse version number for --compat-mode: %s",
> +                p[1]);
> +            goto err;
> +        }
> +
> +        options->backwards_compatible = major * 10000 + minor * 100 + patch;
> +    }
>      else if (streq(p[0], "setenv-safe") && p[1] && !p[3])
>      {
>          VERIFY_PERMISSION(OPT_P_SETENV);
> diff --git a/src/openvpn/options.h b/src/openvpn/options.h
> index b0e40cb7..98c21a2a 100644
> --- a/src/openvpn/options.h
> +++ b/src/openvpn/options.h
> @@ -225,6 +225,10 @@ struct options
>  
>      /* enable forward compatibility for post-2.1 features */
>      bool forward_compatible;
> +    /** What version we should try to be compatible with as major * 10000 +
> +      * minor * 100 + patch, e.g. 2.4.7 => 20407 */
> +    unsigned int backwards_compatible;
> +
>      /* list of options that should be ignored even if unknown */
>      const char **ignore_unknown_option;

Splitting this into its own patch makes sense.

Acked-By: Arne Schwabe <arne@rfc2549.org>