[Openvpn-devel] Require at least 20MB of mlock()-able memory if --mlock is used.

Message ID 20210307161547.25130-1-gert@greenie.muc.de
State Changes Requested
Headers show
Series
  • [Openvpn-devel] Require at least 20MB of mlock()-able memory if --mlock is used.
Related show

Commit Message

Gert Doering March 7, 2021, 4:15 p.m.
If --mlock is used, the amount of memory OpenVPN can use is guarded
by the RLIMIT_MEMLOCK value (see mlockall(2)).  The OS default for this
is usually 64 Kbyte, which is enough for OpenVPN to initialize, but
as soon as the first TLS handshake comes it, OpenVPN will crash due
to "ouf of memory", and might even end up in a crash loop.

Steady-state OpenVPN requires between 8 MB and 30-50 MB (servers with
many concurrent clients) of memory.

So: with this patch, we check if getrlimit() is available, and if yes,
log the amount of mlock'able memory.  If the amount is below 20 MB,
which is an arbitrary value "large enough for most smaller deployments",
we abort.

Trac: #1390

Signed-off-by: Gert Doering <gert@greenie.muc.de>
---
 configure.ac                         |  2 +-
 doc/man-sections/generic-options.rst |  6 ++++++
 src/openvpn/platform.c               | 18 ++++++++++++++++++
 src/openvpn/platform.h               |  4 ++++
 4 files changed, 29 insertions(+), 1 deletion(-)

Comments

Selva Nair March 7, 2021, 5:22 p.m. | #1
Hi,

On Sun, Mar 7, 2021 at 11:31 AM Gert Doering <gert@greenie.muc.de> wrote:

> If --mlock is used, the amount of memory OpenVPN can use is guarded
> by the RLIMIT_MEMLOCK value (see mlockall(2)).  The OS default for this
> is usually 64 Kbyte, which is enough for OpenVPN to initialize, but
> as soon as the first TLS handshake comes it, OpenVPN will crash due
> to "ouf of memory", and might even end up in a crash loop.
>
> Steady-state OpenVPN requires between 8 MB and 30-50 MB (servers with
> many concurrent clients) of memory.
>
> So: with this patch, we check if getrlimit() is available, and if yes,
> log the amount of mlock'able memory.  If the amount is below 20 MB,
> which is an arbitrary value "large enough for most smaller deployments",
> we abort.
>

This is required only if privileges are dropped, isn't it? Could be made
conditional
on o->username is set.


> Trac: #1390
>
> Signed-off-by: Gert Doering <gert@greenie.muc.de>
> ---
>  configure.ac                         |  2 +-
>  doc/man-sections/generic-options.rst |  6 ++++++
>  src/openvpn/platform.c               | 18 ++++++++++++++++++
>  src/openvpn/platform.h               |  4 ++++
>  4 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index 1ab8fe59..c65df3e2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -645,7 +645,7 @@ AC_FUNC_FORK
>
>  AC_CHECK_FUNCS([ \
>         daemon chroot getpwnam setuid nice system getpid dup dup2 \
> -       getpass syslog openlog mlockall getgrnam setgid \
> +       getpass syslog openlog mlockall getrlimit getgrnam setgid \
>         setgroups stat flock readv writev time gettimeofday \
>         ctime memset vsnprintf strdup \
>         setsid chdir putenv getpeername unlink \
> diff --git a/doc/man-sections/generic-options.rst
> b/doc/man-sections/generic-options.rst
> index d5f08839..0a7d3caf 100644
> --- a/doc/man-sections/generic-options.rst
> +++ b/doc/man-sections/generic-options.rst
> @@ -237,6 +237,12 @@ which mode OpenVPN is configured as.
>    likely fail. The limit can be increased using ulimit or systemd
>    directives depending on how OpenVPN is started.
>
> +  If the platform has the getrlimit(2) system call, OpenVPN will check
> +  for the amount of mlock-able memory before calling mlockall(2), and


white space at end of line


>
> +  abort if less than 20 Mb are available.  20 Mb is somewhat arbitrary -
>

Mb -> MB


> +  it is enough for a moderately-sized OpenVPN deployment, but the memory
> +  usage will go beyond that if the number of concurrent clients is high.
> +
>  --nice n
>    Change process priority after initialization (``n`` greater than 0 is
>    lower priority, ``n`` less than zero is higher priority).
> diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c
> index ef688c23..67a69748 100644
> --- a/src/openvpn/platform.c
> +++ b/src/openvpn/platform.c
> @@ -193,6 +193,24 @@ void
>  platform_mlockall(bool print_msg)
>  {
>  #ifdef HAVE_MLOCKALL
> +
> +#ifdef HAVE_GETRLIMIT
> +    struct rlimit rl;
> +    if (getrlimit(RLIMIT_MEMLOCK,&rl)<0)
>

spaces after "," and around "<"  :)


> +    {
> +        msg(M_WARN | M_ERRNO, "WARNING: getrlimit(RLIMIT_MEMLOCK)
> failed");
> +    }
> +    else
> +    {
> +       msg(M_INFO, "mlock: MEMLOCK limit: soft=%ldkb, hard=%dkb",
>

%dkb --> %ldkb


> +           ((long int) rl.rlim_cur)/1024, ((long int) rl.rlim_max)/1024);
>

Some may argue that our style is to put space around "/"


> +       if (rl.rlim_cur < 20*1024*1024)
> +       {
> +           msg(M_FATAL, "mlock: RLIMIT_MEMLOCK < 20 MByte, increase
> limit");
>

Mbyte -> MB or megabytes


> +       }
> +    }
> +#endif
> +
>      if (mlockall(MCL_CURRENT | MCL_FUTURE))
>      {
>          msg(M_WARN | M_ERRNO, "WARNING: mlockall call failed");
> diff --git a/src/openvpn/platform.h b/src/openvpn/platform.h
> index 01f3200c..02c23e38 100644
> --- a/src/openvpn/platform.h
> +++ b/src/openvpn/platform.h
> @@ -48,6 +48,10 @@
>  #include <stdio.h>
>  #endif
>
> +#ifdef HAVE_GETRLIMIT
> +#include <sys/resource.h>
> +#endif
> +
>  #include "basic.h"
>  #include "buffer.h"
>

 Selva
<div dir="ltr"><div dir="ltr"><br></div>Hi,<div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Mar 7, 2021 at 11:31 AM Gert Doering &lt;<a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a>&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">If --mlock is used, the amount of memory OpenVPN can use is guarded<br>
by the RLIMIT_MEMLOCK value (see mlockall(2)).  The OS default for this<br>
is usually 64 Kbyte, which is enough for OpenVPN to initialize, but<br>
as soon as the first TLS handshake comes it, OpenVPN will crash due<br>
to &quot;ouf of memory&quot;, and might even end up in a crash loop.<br>
<br>
Steady-state OpenVPN requires between 8 MB and 30-50 MB (servers with<br>
many concurrent clients) of memory.<br>
<br>
So: with this patch, we check if getrlimit() is available, and if yes,<br>
log the amount of mlock&#39;able memory.  If the amount is below 20 MB,<br>
which is an arbitrary value &quot;large enough for most smaller deployments&quot;,<br>
we abort.<br></blockquote><div><br></div><div>This is required only if privileges are dropped, isn&#39;t it? Could be made conditional </div><div>on o-&gt;username is set.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Trac: #1390<br>
<br>
Signed-off-by: Gert Doering &lt;<a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a>&gt;<br>
---<br>
 <a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a>                         |  2 +-<br>
 doc/man-sections/generic-options.rst |  6 ++++++<br>
 src/openvpn/platform.c               | 18 ++++++++++++++++++<br>
 src/openvpn/platform.h               |  4 ++++<br>
 4 files changed, 29 insertions(+), 1 deletion(-)<br>
<br>
diff --git a/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> b/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
index 1ab8fe59..c65df3e2 100644<br>
--- a/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
+++ b/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
@@ -645,7 +645,7 @@ AC_FUNC_FORK<br>
<br>
 AC_CHECK_FUNCS([ \<br>
        daemon chroot getpwnam setuid nice system getpid dup dup2 \<br>
-       getpass syslog openlog mlockall getgrnam setgid \<br>
+       getpass syslog openlog mlockall getrlimit getgrnam setgid \<br>
        setgroups stat flock readv writev time gettimeofday \<br>
        ctime memset vsnprintf strdup \<br>
        setsid chdir putenv getpeername unlink \<br>
diff --git a/doc/man-sections/generic-options.rst b/doc/man-sections/generic-options.rst<br>
index d5f08839..0a7d3caf 100644<br>
--- a/doc/man-sections/generic-options.rst<br>
+++ b/doc/man-sections/generic-options.rst<br>
@@ -237,6 +237,12 @@ which mode OpenVPN is configured as.<br>
   likely fail. The limit can be increased using ulimit or systemd<br>
   directives depending on how OpenVPN is started.<br>
<br>
+  If the platform has the getrlimit(2) system call, OpenVPN will check<br>
+  for the amount of mlock-able memory before calling mlockall(2), and</blockquote><div><br></div><div>white space at end of line</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>
+  abort if less than 20 Mb are available.  20 Mb is somewhat arbitrary -<br></blockquote><div><br></div><div>Mb -&gt; MB </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">+  it is enough for a moderately-sized OpenVPN deployment, but the memory<br>
+  usage will go beyond that if the number of concurrent clients is high.<br>
+<br>
 --nice n<br>
   Change process priority after initialization (``n`` greater than 0 is<br>
   lower priority, ``n`` less than zero is higher priority).<br>
diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c<br>
index ef688c23..67a69748 100644<br>
--- a/src/openvpn/platform.c<br>
+++ b/src/openvpn/platform.c<br>
@@ -193,6 +193,24 @@ void<br>
 platform_mlockall(bool print_msg)<br>
 {<br>
 #ifdef HAVE_MLOCKALL<br>
+<br>
+#ifdef HAVE_GETRLIMIT<br>
+    struct rlimit rl;<br>
+    if (getrlimit(RLIMIT_MEMLOCK,&amp;rl)&lt;0)<br></blockquote><div><br></div><div>spaces after &quot;,&quot; and around &quot;&lt;&quot;  :)</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>
+        msg(M_WARN | M_ERRNO, &quot;WARNING: getrlimit(RLIMIT_MEMLOCK) failed&quot;);<br>
+    }<br>
+    else<br>
+    {<br>
+       msg(M_INFO, &quot;mlock: MEMLOCK limit: soft=%ldkb, hard=%dkb&quot;,<br>
</blockquote><div><br></div><div>%dkb --&gt; %ldkb</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">+           ((long int) rl.rlim_cur)/1024, ((long int) rl.rlim_max)/1024);<br></blockquote><div><br></div><div>Some may argue that our style is to put space around &quot;/&quot;</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">
+       if (rl.rlim_cur &lt; 20*1024*1024)<br>
+       {<br>
+           msg(M_FATAL, &quot;mlock: RLIMIT_MEMLOCK &lt; 20 MByte, increase limit&quot;);<br></blockquote><div><br></div><div>Mbyte -&gt; MB or megabytes</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>
+    }<br>
+#endif<br>
+<br>
     if (mlockall(MCL_CURRENT | MCL_FUTURE))<br>
     {<br>
         msg(M_WARN | M_ERRNO, &quot;WARNING: mlockall call failed&quot;);<br>
diff --git a/src/openvpn/platform.h b/src/openvpn/platform.h<br>
index 01f3200c..02c23e38 100644<br>
--- a/src/openvpn/platform.h<br>
+++ b/src/openvpn/platform.h<br>
@@ -48,6 +48,10 @@<br>
 #include &lt;stdio.h&gt;<br>
 #endif<br>
<br>
+#ifdef HAVE_GETRLIMIT<br>
+#include &lt;sys/resource.h&gt;<br>
+#endif<br>
+<br>
 #include &quot;basic.h&quot;<br>
 #include &quot;buffer.h&quot;<br></blockquote><div><br></div><div> Selva</div></div></div></div>
Gert Doering March 7, 2021, 6:10 p.m. | #2
Hi,

thanks for the review. 

On Sun, Mar 07, 2021 at 12:22:32PM -0500, Selva Nair wrote:
> On Sun, Mar 7, 2021 at 11:31 AM Gert Doering <gert@greenie.muc.de> wrote:
> 
> > If --mlock is used, the amount of memory OpenVPN can use is guarded
> > by the RLIMIT_MEMLOCK value (see mlockall(2)).  The OS default for this
> > is usually 64 Kbyte, which is enough for OpenVPN to initialize, but
> > as soon as the first TLS handshake comes it, OpenVPN will crash due
> > to "ouf of memory", and might even end up in a crash loop.
> >
> > Steady-state OpenVPN requires between 8 MB and 30-50 MB (servers with
> > many concurrent clients) of memory.
> >
> > So: with this patch, we check if getrlimit() is available, and if yes,
> > log the amount of mlock'able memory.  If the amount is below 20 MB,
> > which is an arbitrary value "large enough for most smaller deployments",
> > we abort.
> >
> 
> This is required only if privileges are dropped, isn't it? Could be made
> conditional
> on o->username is set.

"I'm not sure", TBH.  rlimit handling in unix is a bit of an unknown
territory for me.

What I understand is that root can *increment* the rlimit at will, but
I'd assume that the rlimit value "in existance right now" (specifically,
the soft limit) applies to root processes as well.  Sort of a voluntary
protection against processes running away.

But I can test this.  Just malloc() until it does...

[..]
> > @@ -237,6 +237,12 @@ which mode OpenVPN is configured as.
> >    likely fail. The limit can be increased using ulimit or systemd
> >    directives depending on how OpenVPN is started.
> >
> > +  If the platform has the getrlimit(2) system call, OpenVPN will check
> > +  for the amount of mlock-able memory before calling mlockall(2), and
> 
> white space at end of line

Yeah, the whitespace-and-typing force wasn't strong with me today :-)

gert
Selva Nair March 7, 2021, 6:36 p.m. | #3
On Sun, Mar 7, 2021 at 1:10 PM Gert Doering <gert@greenie.muc.de> wrote:

> Hi,
>
> thanks for the review.
>
> On Sun, Mar 07, 2021 at 12:22:32PM -0500, Selva Nair wrote:
> > On Sun, Mar 7, 2021 at 11:31 AM Gert Doering <gert@greenie.muc.de>
> wrote:
> >
> > > If --mlock is used, the amount of memory OpenVPN can use is guarded
> > > by the RLIMIT_MEMLOCK value (see mlockall(2)).  The OS default for this
> > > is usually 64 Kbyte, which is enough for OpenVPN to initialize, but
> > > as soon as the first TLS handshake comes it, OpenVPN will crash due
> > > to "ouf of memory", and might even end up in a crash loop.
> > >
> > > Steady-state OpenVPN requires between 8 MB and 30-50 MB (servers with
> > > many concurrent clients) of memory.
> > >
> > > So: with this patch, we check if getrlimit() is available, and if yes,
> > > log the amount of mlock'able memory.  If the amount is below 20 MB,
> > > which is an arbitrary value "large enough for most smaller
> deployments",
> > > we abort.
> > >
> >
> > This is required only if privileges are dropped, isn't it? Could be made
> > conditional
> > on o->username is set.
>
> "I'm not sure", TBH.  rlimit handling in unix is a bit of an unknown
> territory for me.
>
> What I understand is that root can *increment* the rlimit at will, but
> I'd assume that the rlimit value "in existance right now" (specifically,
> the soft limit) applies to root processes as well.  Sort of a voluntary
> protection against processes running away.
>

On modern linux kernels (since some 2.6.x..) RLIMIT_MEMLOCK applies only to
unprivileged processes -- privileged processes allowed to lock "unlimited"
amount of memory as documented in man mlock. We updated the man page based
on that sometime ago.

We could also consider using setrlimit to increase the limit before
dropping privileges.

But I haven't checked how other "unix"-like OSes behave.

Selva
<div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Mar 7, 2021 at 1:10 PM Gert Doering &lt;<a href="mailto:gert@greenie.muc.de">gert@greenie.muc.de</a>&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">Hi,<br>
<br>
thanks for the review. <br>
<br>
On Sun, Mar 07, 2021 at 12:22:32PM -0500, Selva Nair wrote:<br>
&gt; On Sun, Mar 7, 2021 at 11:31 AM Gert Doering &lt;<a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a>&gt; wrote:<br>
&gt; <br>
&gt; &gt; If --mlock is used, the amount of memory OpenVPN can use is guarded<br>
&gt; &gt; by the RLIMIT_MEMLOCK value (see mlockall(2)).  The OS default for this<br>
&gt; &gt; is usually 64 Kbyte, which is enough for OpenVPN to initialize, but<br>
&gt; &gt; as soon as the first TLS handshake comes it, OpenVPN will crash due<br>
&gt; &gt; to &quot;ouf of memory&quot;, and might even end up in a crash loop.<br>
&gt; &gt;<br>
&gt; &gt; Steady-state OpenVPN requires between 8 MB and 30-50 MB (servers with<br>
&gt; &gt; many concurrent clients) of memory.<br>
&gt; &gt;<br>
&gt; &gt; So: with this patch, we check if getrlimit() is available, and if yes,<br>
&gt; &gt; log the amount of mlock&#39;able memory.  If the amount is below 20 MB,<br>
&gt; &gt; which is an arbitrary value &quot;large enough for most smaller deployments&quot;,<br>
&gt; &gt; we abort.<br>
&gt; &gt;<br>
&gt; <br>
&gt; This is required only if privileges are dropped, isn&#39;t it? Could be made<br>
&gt; conditional<br>
&gt; on o-&gt;username is set.<br>
<br>
&quot;I&#39;m not sure&quot;, TBH.  rlimit handling in unix is a bit of an unknown<br>
territory for me.<br>
<br>
What I understand is that root can *increment* the rlimit at will, but<br>
I&#39;d assume that the rlimit value &quot;in existance right now&quot; (specifically,<br>
the soft limit) applies to root processes as well.  Sort of a voluntary<br>
protection against processes running away.<br></blockquote><div><br></div><div>On modern linux kernels (since some 2.6.x..) RLIMIT_MEMLOCK applies only to unprivileged processes -- privileged processes allowed to lock &quot;unlimited&quot; amount of memory as documented in man mlock. We updated the man page based on that sometime ago.</div><div><br></div><div>We could also consider using setrlimit to increase the limit before dropping privileges. </div><div><br></div><div>But I haven&#39;t checked how other &quot;unix&quot;-like OSes behave.</div><div><br></div><div>Selva</div></div></div>
Gert Doering March 7, 2021, 6:44 p.m. | #4
Hi,

On Sun, Mar 07, 2021 at 01:36:03PM -0500, Selva Nair wrote:
> > "I'm not sure", TBH.  rlimit handling in unix is a bit of an unknown
> > territory for me.
> >
> > What I understand is that root can *increment* the rlimit at will, but
> > I'd assume that the rlimit value "in existance right now" (specifically,
> > the soft limit) applies to root processes as well.  Sort of a voluntary
> > protection against processes running away.
> 
> On modern linux kernels (since some 2.6.x..) RLIMIT_MEMLOCK applies only to
> unprivileged processes -- privileged processes allowed to lock "unlimited"
> amount of memory as documented in man mlock. We updated the man page based
> on that sometime ago.

Indeed, "man mlock" says something about "privileged processes" on Linux
(it doesn't say that on FreeBSD).

> We could also consider using setrlimit to increase the limit before
> dropping privileges.

That's another possible angle... just up soft+hard to "something"
(how much would that be? :-) ) and log the fact.

David, Arne, any opinion on this?  Where do we want to go?

gert
Selva Nair March 7, 2021, 7:20 p.m. | #5
Hi,

On Sun, Mar 7, 2021 at 1:44 PM Gert Doering <gert@greenie.muc.de> wrote:

> Hi,
>
> On Sun, Mar 07, 2021 at 01:36:03PM -0500, Selva Nair wrote:
> > > "I'm not sure", TBH.  rlimit handling in unix is a bit of an unknown
> > > territory for me.
> > >
> > > What I understand is that root can *increment* the rlimit at will, but
> > > I'd assume that the rlimit value "in existance right now"
> (specifically,
> > > the soft limit) applies to root processes as well.  Sort of a voluntary
> > > protection against processes running away.
> >
> > On modern linux kernels (since some 2.6.x..) RLIMIT_MEMLOCK applies only
> to
> > unprivileged processes -- privileged processes allowed to lock
> "unlimited"
> > amount of memory as documented in man mlock. We updated the man page
> based
> > on that sometime ago.
>
> Indeed, "man mlock" says something about "privileged processes" on Linux
> (it doesn't say that on FreeBSD).
>
> > We could also consider using setrlimit to increase the limit before
> > dropping privileges.
>
> That's another possible angle... just up soft+hard to "something"
> (how much would that be? :-) ) and log the fact.
>

Rereading my comment on Trac #1059 I recall testing this and concluding
100MB enough for clients. On modern machines that's a low amount of memory
--- not allowing swapout of 100MB should be acceptable.  For servers, I
think there is no reliable limit that we could come up with.

Selva
<div dir="ltr"><div dir="ltr"><br></div>Hi,<div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Mar 7, 2021 at 1:44 PM Gert Doering &lt;<a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a>&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">Hi,<br>
<br>
On Sun, Mar 07, 2021 at 01:36:03PM -0500, Selva Nair wrote:<br>
&gt; &gt; &quot;I&#39;m not sure&quot;, TBH.  rlimit handling in unix is a bit of an unknown<br>
&gt; &gt; territory for me.<br>
&gt; &gt;<br>
&gt; &gt; What I understand is that root can *increment* the rlimit at will, but<br>
&gt; &gt; I&#39;d assume that the rlimit value &quot;in existance right now&quot; (specifically,<br>
&gt; &gt; the soft limit) applies to root processes as well.  Sort of a voluntary<br>
&gt; &gt; protection against processes running away.<br>
&gt; <br>
&gt; On modern linux kernels (since some 2.6.x..) RLIMIT_MEMLOCK applies only to<br>
&gt; unprivileged processes -- privileged processes allowed to lock &quot;unlimited&quot;<br>
&gt; amount of memory as documented in man mlock. We updated the man page based<br>
&gt; on that sometime ago.<br>
<br>
Indeed, &quot;man mlock&quot; says something about &quot;privileged processes&quot; on Linux<br>
(it doesn&#39;t say that on FreeBSD).<br>
<br>
&gt; We could also consider using setrlimit to increase the limit before<br>
&gt; dropping privileges.<br>
<br>
That&#39;s another possible angle... just up soft+hard to &quot;something&quot;<br>
(how much would that be? :-) ) and log the fact.<br></blockquote><div><br></div><div>Rereading my comment on Trac #1059 I recall testing this and concluding 100MB enough for clients. On modern machines that&#39;s a low amount of memory --- not allowing swapout of 100MB should be acceptable.  For servers, I think there is no reliable limit that we could come up with. </div><div><br></div><div>Selva</div></div></div></div>
tincanteksup March 7, 2021, 7:26 p.m. | #6
On 07/03/2021 19:20, Selva Nair wrote:

> Rereading my comment on Trac #1059 I recall testing this and concluding
> 100MB enough for clients. On modern machines that's a low amount of memory
> --- not allowing swapout of 100MB should be acceptable.  For servers, I
> think there is no reliable limit that we could come up with.
> 
> Selva
> 

I recall, when testing with EC the min. required was 89MB.

R
Gert Doering March 7, 2021, 9:28 p.m. | #7
Hi,

On Sun, Mar 07, 2021 at 02:20:32PM -0500, Selva Nair wrote:
> > That's another possible angle... just up soft+hard to "something"
> > (how much would that be? :-) ) and log the fact.
> 
> Rereading my comment on Trac #1059 I recall testing this and concluding
> 100MB enough for clients. On modern machines that's a low amount of memory
> --- not allowing swapout of 100MB should be acceptable.  For servers, I
> think there is no reliable limit that we could come up with.

This is interesting.  My machines, including servers, have a way lower
memory usage - but I'm not using EC.

Here's a linux server with 3 peers and a client (arm32):

USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
root     14569  0.0  0.5   5704  2980 ?        Ss    2020  12:31 openvpn
root     21020  0.1  0.5   5568  3040 ?        Ss    2020 454:45 openvpn

Here's a FreeBSD server that serves 5 clients right now, but has peaks
up to 150 concurrent clients (amd64)

USER      PID   %CPU %MEM    VSZ    RSS TT  STAT STARTED          TIME COMMAND
root     5023    0.3  0.1  30924  23172  -  Ss   20Dec20    1945:07.12 /usr/local/sbin/openvpn --cd /usr/local/etc/openvpn --daemon

... and another two Linux server instances...

USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
root      1138  2.1  1.3  26732 10628 ?        Ss    2019 21678:18 /usr/sbin/openvpn
root      2897  0.8  0.9  29548  7080 ?        Ss    2019 8596:15 /usr/sbin/openvpn


the last 3 see a high number of clients during work time, and quite
some churn (especially due to iOS/android connecting and disconnecting
frequently), but long-term memory usage is not high.

So there must be some massive transient usage due to EC at reconnection
time...


But anyway.  100M seems to be a good value, then.

I'll send a v2 of that patch, with setrlimit().

thanks,

gert
Arne Schwabe March 8, 2021, 8:06 a.m. | #8
Am 07.03.21 um 19:44 schrieb Gert Doering:
> Hi,
> 
> On Sun, Mar 07, 2021 at 01:36:03PM -0500, Selva Nair wrote:
>>> "I'm not sure", TBH.  rlimit handling in unix is a bit of an unknown
>>> territory for me.
>>>
>>> What I understand is that root can *increment* the rlimit at will, but
>>> I'd assume that the rlimit value "in existance right now" (specifically,
>>> the soft limit) applies to root processes as well.  Sort of a voluntary
>>> protection against processes running away.
>>
>> On modern linux kernels (since some 2.6.x..) RLIMIT_MEMLOCK applies only to
>> unprivileged processes -- privileged processes allowed to lock "unlimited"
>> amount of memory as documented in man mlock. We updated the man page based
>> on that sometime ago.
> 
> Indeed, "man mlock" says something about "privileged processes" on Linux
> (it doesn't say that on FreeBSD).
> 
>> We could also consider using setrlimit to increase the limit before
>> dropping privileges.
> 
> That's another possible angle... just up soft+hard to "something"
> (how much would that be? :-) ) and log the fact.
> 
> David, Arne, any opinion on this?  Where do we want to go?

Looking at this feature  from today's perspective, it feels like one of
OpenVPN's boutique features. Was probably useful at some point but
doesn't really make much sense today anymore. Esepcially with what is
written in the manpage. Today you rather would use full disk encryption
or disable swapping rather than to rely on OpenVPN's --mlock.

That being said I am against your patch, I am just wondering if that is
a feature we need to keep at all.

Arne
tincanteksup March 8, 2021, 1:45 p.m. | #9
On 08/03/2021 08:06, Arne Schwabe wrote:

> Looking at this feature  from today's perspective, it feels like one of
> OpenVPN's boutique features. Was probably useful at some point but
> doesn't really make much sense today anymore. Esepcially with what is
> written in the manpage. Today you rather would use full disk encryption
> or disable swapping rather than to rely on OpenVPN's --mlock.
> 
> That being said I am against your patch, I am just wondering if that is
> a feature we need to keep at all.
> 

Having all openvpn data remain permanently in memory also offers
a (small) performance boost.

Your alternative offers would impact performance and be system wide.

Therefore, I for one disagree.

--
David Sommerseth March 8, 2021, 7:47 p.m. | #10
On 07/03/2021 22:28, Gert Doering wrote:
> Hi,
> 
> On Sun, Mar 07, 2021 at 02:20:32PM -0500, Selva Nair wrote:
>>> That's another possible angle... just up soft+hard to "something"
>>> (how much would that be? :-) ) and log the fact.
>>
>> Rereading my comment on Trac #1059 I recall testing this and concluding
>> 100MB enough for clients. On modern machines that's a low amount of memory
>> --- not allowing swapout of 100MB should be acceptable.  For servers, I
>> think there is no reliable limit that we could come up with.
> 
> This is interesting.  My machines, including servers, have a way lower
> memory usage - but I'm not using EC.
> 
> Here's a linux server with 3 peers and a client (arm32):
> 
> USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
> root     14569  0.0  0.5   5704  2980 ?        Ss    2020  12:31 openvpn
> root     21020  0.1  0.5   5568  3040 ?        Ss    2020 454:45 openvpn
> 
> Here's a FreeBSD server that serves 5 clients right now, but has peaks
> up to 150 concurrent clients (amd64)
> 
> USER      PID   %CPU %MEM    VSZ    RSS TT  STAT STARTED          TIME COMMAND
> root     5023    0.3  0.1  30924  23172  -  Ss   20Dec20    1945:07.12 /usr/local/sbin/openvpn --cd /usr/local/etc/openvpn --daemon
> 
> ... and another two Linux server instances...
> 
> USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
> root      1138  2.1  1.3  26732 10628 ?        Ss    2019 21678:18 /usr/sbin/openvpn
> root      2897  0.8  0.9  29548  7080 ?        Ss    2019 8596:15 /usr/sbin/openvpn
> 
> 
> the last 3 see a high number of clients during work time, and quite
> some churn (especially due to iOS/android connecting and disconnecting
> frequently), but long-term memory usage is not high.
> 
> So there must be some massive transient usage due to EC at reconnection
> time...
> 

FWIW .... A couple of my CentOS 8.3 servers with 2 client connections
running 24/7.

* Server 1
USER         PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
openvpn  3437979  0.0  0.1  76360  2924 ?        Ss   Feb25   2:19 /usr/sbin/openvpn


* Server 2
USER         PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
openvpn  2805670  0.0  0.8  76624  6896 ?        Ss   Feb25   7:08 /usr/sbin/openvpn


The start date is from when openvpn-2.5.1 got released and updated.
OpenSSL 1.1.1g-12 is used on both hosts.
David Sommerseth March 9, 2021, 6:52 p.m. | #11
On 08/03/2021 14:45, tincanteksup wrote:
> 
> 
> On 08/03/2021 08:06, Arne Schwabe wrote:
> 
>> Looking at this feature  from today's perspective, it feels like one of
>> OpenVPN's boutique features. Was probably useful at some point but
>> doesn't really make much sense today anymore. Esepcially with what is
>> written in the manpage. Today you rather would use full disk encryption
>> or disable swapping rather than to rely on OpenVPN's --mlock.
>>
>> That being said I am against your patch, I am just wondering if that is
>> a feature we need to keep at all.
>>
> 
> Having all openvpn data remain permanently in memory also offers
> a (small) performance boost.
> 
> Your alternative offers would impact performance and be system wide.
> 
> Therefore, I for one disagree.

mlock() itself does not really have any impact your these arguments.

Yes, mlock() is about ensuring that OpenVPN can allocate a certain 
amount of memory which will stay entirely in RSS the memory pool as long 
as the memory pages has been locked.  But it is not a system wide knob; 
it's a per process knob and applications can even turn this on and off 
at will (given they have the needed privileges for it) during the 
lifetime of the process.

Due to this flexibility to when you can enable and disable memory 
locking, it is also clear it is not intended to be used as a performance 
knob.  It is designed to be more a security related feature, which in 
OpenVPN's context is there to avoid getting memory containing keying 
material being swapped out to disk.

On today's hardware even telling the kernel to not swap out a few 
hundred megabytes will also not result in much noticeable performance 
improvements.  To feel the difference, you would need to have some 
really ancient hardware.

If you care or have issues with performance related to swapping, you 
need to revisit your swap setup - or add more RAM.  A few hundred 
megabyte related to OpenVPN will not make things better or worse system 
wide on your system.  Fixing the real reason you have swapping issues will.
tincanteksup March 9, 2021, 8:04 p.m. | #12
On 09/03/2021 18:52, David Sommerseth wrote:
> On 08/03/2021 14:45, tincanteksup wrote:
>>
>>
>> On 08/03/2021 08:06, Arne Schwabe wrote:
>>
>>> Looking at this feature  from today's perspective, it feels like one of
>>> OpenVPN's boutique features. Was probably useful at some point but
>>> doesn't really make much sense today anymore. Esepcially with what is
>>> written in the manpage. Today you rather would use full disk encryption
>>> or disable swapping rather than to rely on OpenVPN's --mlock.
>>>
>>> That being said I am against your patch, I am just wondering if that is
>>> a feature we need to keep at all.
>>>
>>
>> Having all openvpn data remain permanently in memory also offers
>> a (small) performance boost.
>>
>> Your alternative offers would impact performance and be system wide.
>>
>> Therefore, I for one disagree.
> 
> mlock() itself does not really have any impact your these arguments.
> 
> Yes, mlock() is about ensuring that OpenVPN can allocate a certain 
> amount of memory which will stay entirely in RSS the memory pool as long 
> as the memory pages has been locked.  But it is not a system wide knob; 
> it's a per process knob and applications can even turn this on and off 
> at will (given they have the needed privileges for it) during the 
> lifetime of the process.
> 
> Due to this flexibility to when you can enable and disable memory 
> locking, it is also clear it is not intended to be used as a performance 
> knob.  It is designed to be more a security related feature, which in 
> OpenVPN's context is there to avoid getting memory containing keying 
> material being swapped out to disk.
> 
> On today's hardware even telling the kernel to not swap out a few 
> hundred megabytes will also not result in much noticeable performance 
> improvements.  To feel the difference, you would need to have some 
> really ancient hardware.
> 
> If you care or have issues with performance related to swapping, you 
> need to revisit your swap setup - or add more RAM.  A few hundred 
> megabyte related to OpenVPN will not make things better or worse system 
> wide on your system.  Fixing the real reason you have swapping issues will.
> 
> 

I have swapping issues all the time and I can't add more RAM.
I don't want system wide disk encryption.
And I don't want an SSD either.

I do not have the money to keep up with modern hardware.

Having openvpn --mlock is exactly the right choice for my home
system.

Please, do not remove --mlock from openvpn.

Please try to put yourselves in the place of the average user,
for once.

R
David Sommerseth March 9, 2021, 8:27 p.m. | #13
On 07/03/2021 19:44, Gert Doering wrote:
> Hi,
> 
> On Sun, Mar 07, 2021 at 01:36:03PM -0500, Selva Nair wrote:
>>> "I'm not sure", TBH.  rlimit handling in unix is a bit of an unknown
>>> territory for me.
>>>
>>> What I understand is that root can *increment* the rlimit at will, but
>>> I'd assume that the rlimit value "in existance right now" (specifically,
>>> the soft limit) applies to root processes as well.  Sort of a voluntary
>>> protection against processes running away.
>>
>> On modern linux kernels (since some 2.6.x..) RLIMIT_MEMLOCK applies only to
>> unprivileged processes -- privileged processes allowed to lock "unlimited"
>> amount of memory as documented in man mlock. We updated the man page based
>> on that sometime ago.
> 
> Indeed, "man mlock" says something about "privileged processes" on Linux
> (it doesn't say that on FreeBSD).
> 
>> We could also consider using setrlimit to increase the limit before
>> dropping privileges.
> 
> That's another possible angle... just up soft+hard to "something"
> (how much would that be? :-) ) and log the fact.
> 
> David, Arne, any opinion on this?  Where do we want to go?

I do think mlock serves a purpose in ensuring where memory pages are 
kept, from a general perspective.  But does it really make sense in 
today's virtualized world?  Where we're mostly having several GiBs of 
RAM available.  I'm not sure it matters that much either way.  But for 
those who care about memory security, it is a useful feature.

Since the code to enable this shouldn't be enormous (since we use 
mlockall()), I don't see much winning in code complexity by tearing it 
out when it may provide some security related features.


The unprivileged vs privileged process on Linux kernels change happened 
in 2.6.9, according to "Linux Programming Interface" (LPI) [Michael 
Kerrisk, chapter 50)].  I do not consider that an issue at all; I can't 
think of any still fully supported Linux distributions with such an 
ancient Linux kernel.  It should really not run security related tasks 
(from a general perspective).

If I've understood the LPI book correctly, the default (RLIMIT_MEMLOCK) 
is set to 32KiB for both hard and soft limits.  Clearly not enough for 
our use case.

setrlimit() by itself isn't really that magical.  Basically you populate 
struct rlimit with the limits you want and call 
setrlimit(RLIMIT_MEMLOCK, &new_limits);   Plus error checking and all that.

But we should look into if we should acquire CAP_IPC_LOCK capability as 
well, otherwise we need to completely ensure that mlockall() is really 
called before privileges have been dropped.  I know I had to kick out 
--mlock on a few configs a long while ago, related to lack of privileges 
- but that could have been SELinux restrictions as well.  Need to 
revisit this.
David Sommerseth March 9, 2021, 8:53 p.m. | #14
On 09/03/2021 21:04, tincanteksup wrote:
 >
> I have swapping issues all the time and I can't add more RAM.
> I don't want system wide disk encryption.
> And I don't want an SSD either.
> 
> I do not have the money to keep up with modern hardware.
> 
> Having openvpn --mlock is exactly the right choice for my home
> system.
> 
> Please, do not remove --mlock from openvpn.
> 

How much memory does your OpenVPN process consume?  And if the process 
(or task in kernel lingo) is active (not idling), it generally will not 
be swapped out.  If it does, you are already running way to much on your 
host.  So you need to prioritize what that host should really run.

OpenVPN's --mlock does not save you.

And in fact, the kernel may as well swap out the memory pages containing 
the program itself and just keep the data pages allocated by the program 
in memory.  Which will result in a sluggish OpenVPN performance regardless.

This gives a brief overview which might help you see what happens:
<https://scoutapm.com/blog/understanding-page-faults-and-memory-swap-in-outs-when-should-you-worry>

And even though you don't want SSD.  At least consider if you can get 
hold of a reasonably performing SSD with not too many GB and activate 
that device as a swap device on your host.  It will not be optimal, but 
at least the general swapping can go faster if you have a decent 
SATA/SAS controller.

 > Please try to put yourselves in the place of the average user,
 > for once.

The average users I know does not push their hardware beyond its limits 
so much it hurts the overall performance.  Those I know would start 
planning for an upgrade.  Or get an RPi4 and run OpenVPN on it, as it 
got a reasonable network performance: 
<https://notenoughtech.com/raspberry-pi/2019-raspberry-pi-network-speed-test/>

But openvpn --mlock is never the solution to performance and swapping. 
Never ever.
tincanteksup March 9, 2021, 9:20 p.m. | #15
On 09/03/2021 20:53, David Sommerseth wrote:
> On 09/03/2021 21:04, tincanteksup wrote:
>  >
>> I have swapping issues all the time and I can't add more RAM.
>> I don't want system wide disk encryption.
>> And I don't want an SSD either.
>>
>> I do not have the money to keep up with modern hardware.
>>
>> Having openvpn --mlock is exactly the right choice for my home
>> system.
>>
>> Please, do not remove --mlock from openvpn.
>>
> 
> How much memory does your OpenVPN process consume?  And if the process 
> (or task in kernel lingo) is active (not idling), it generally will not 
> be swapped out.  If it does, you are already running way to much on your 
> host.  So you need to prioritize what that host should really run.
> 
> OpenVPN's --mlock does not save you.
> 
> And in fact, the kernel may as well swap out the memory pages containing 
> the program itself and just keep the data pages allocated by the program 
> in memory.  Which will result in a sluggish OpenVPN performance regardless.
> 
> This gives a brief overview which might help you see what happens:
> <https://scoutapm.com/blog/understanding-page-faults-and-memory-swap-in-outs-when-should-you-worry> 
> 
> 
> And even though you don't want SSD.  At least consider if you can get 
> hold of a reasonably performing SSD with not too many GB and activate 
> that device as a swap device on your host.  It will not be optimal, but 
> at least the general swapping can go faster if you have a decent 
> SATA/SAS controller.
> 
>  > Please try to put yourselves in the place of the average user,
>  > for once.
> 
> The average users I know does not push their hardware beyond its limits 
> so much it hurts the overall performance.  Those I know would start 
> planning for an upgrade.  Or get an RPi4 and run OpenVPN on it, as it 
> got a reasonable network performance: 
> <https://notenoughtech.com/raspberry-pi/2019-raspberry-pi-network-speed-test/> 
> 
> 
> But openvpn --mlock is never the solution to performance and swapping. 
> Never ever.
> 
> 

As I Initially clarified, there *may* be a Small performance tweak by 
using --mlock.

The problem here is that by removing --mlock you shift the
burden of securing ephemeral key data to the under lying OS.

You may as well print the full private keys in the log (again)
and expect the user to delete them all and shred their disk.

As for how I must spend my money, that is no business of yours.

my2c
Gert Doering March 9, 2021, 9:43 p.m. | #16
Hi,

On Mon, Mar 08, 2021 at 09:06:10AM +0100, Arne Schwabe wrote:
> That being said I am against your patch, I am just wondering if that is
> a feature we need to keep at all.

Just for clarification: is there a "not" missing in that sentence?

(Code-wise, mlock / *rlimit is just ~40-50 lines of code, isolated to
init time, so it really doesn't cost/save us much either way)

gert

Patch

diff --git a/configure.ac b/configure.ac
index 1ab8fe59..c65df3e2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -645,7 +645,7 @@  AC_FUNC_FORK
 
 AC_CHECK_FUNCS([ \
 	daemon chroot getpwnam setuid nice system getpid dup dup2 \
-	getpass syslog openlog mlockall getgrnam setgid \
+	getpass syslog openlog mlockall getrlimit getgrnam setgid \
 	setgroups stat flock readv writev time gettimeofday \
 	ctime memset vsnprintf strdup \
 	setsid chdir putenv getpeername unlink \
diff --git a/doc/man-sections/generic-options.rst b/doc/man-sections/generic-options.rst
index d5f08839..0a7d3caf 100644
--- a/doc/man-sections/generic-options.rst
+++ b/doc/man-sections/generic-options.rst
@@ -237,6 +237,12 @@  which mode OpenVPN is configured as.
   likely fail. The limit can be increased using ulimit or systemd
   directives depending on how OpenVPN is started.
 
+  If the platform has the getrlimit(2) system call, OpenVPN will check
+  for the amount of mlock-able memory before calling mlockall(2), and 
+  abort if less than 20 Mb are available.  20 Mb is somewhat arbitrary -
+  it is enough for a moderately-sized OpenVPN deployment, but the memory
+  usage will go beyond that if the number of concurrent clients is high.
+
 --nice n
   Change process priority after initialization (``n`` greater than 0 is
   lower priority, ``n`` less than zero is higher priority).
diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c
index ef688c23..67a69748 100644
--- a/src/openvpn/platform.c
+++ b/src/openvpn/platform.c
@@ -193,6 +193,24 @@  void
 platform_mlockall(bool print_msg)
 {
 #ifdef HAVE_MLOCKALL
+
+#ifdef HAVE_GETRLIMIT
+    struct rlimit rl;
+    if (getrlimit(RLIMIT_MEMLOCK,&rl)<0)
+    {
+        msg(M_WARN | M_ERRNO, "WARNING: getrlimit(RLIMIT_MEMLOCK) failed");
+    }
+    else
+    {
+	msg(M_INFO, "mlock: MEMLOCK limit: soft=%ldkb, hard=%dkb",
+	    ((long int) rl.rlim_cur)/1024, ((long int) rl.rlim_max)/1024);
+	if (rl.rlim_cur < 20*1024*1024)
+	{
+	    msg(M_FATAL, "mlock: RLIMIT_MEMLOCK < 20 MByte, increase limit");
+	}
+    }
+#endif
+
     if (mlockall(MCL_CURRENT | MCL_FUTURE))
     {
         msg(M_WARN | M_ERRNO, "WARNING: mlockall call failed");
diff --git a/src/openvpn/platform.h b/src/openvpn/platform.h
index 01f3200c..02c23e38 100644
--- a/src/openvpn/platform.h
+++ b/src/openvpn/platform.h
@@ -48,6 +48,10 @@ 
 #include <stdio.h>
 #endif
 
+#ifdef HAVE_GETRLIMIT
+#include <sys/resource.h>
+#endif
+
 #include "basic.h"
 #include "buffer.h"