| 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. | expand |
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 <<a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a>> 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 "ouf of memory", 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'able memory. If the amount is below 20 MB,<br> which is an arbitrary value "large enough for most smaller deployments",<br> we abort.<br></blockquote><div><br></div><div>This is required only if privileges are dropped, isn't it? Could be made conditional </div><div>on o->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 <<a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a>><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 -> 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,&rl)<0)<br></blockquote><div><br></div><div>spaces after "," and around "<" :)</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, "WARNING: getrlimit(RLIMIT_MEMLOCK) failed");<br> + }<br> + else<br> + {<br> + msg(M_INFO, "mlock: MEMLOCK limit: soft=%ldkb, hard=%dkb",<br> </blockquote><div><br></div><div>%dkb --> %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 "/"</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 < 20*1024*1024)<br> + {<br> + msg(M_FATAL, "mlock: RLIMIT_MEMLOCK < 20 MByte, increase limit");<br></blockquote><div><br></div><div>Mbyte -> 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, "WARNING: mlockall call failed");<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 <stdio.h><br> #endif<br> <br> +#ifdef HAVE_GETRLIMIT<br> +#include <sys/resource.h><br> +#endif<br> +<br> #include "basic.h"<br> #include "buffer.h"<br></blockquote><div><br></div><div> Selva</div></div></div></div>
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
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 <<a href="mailto:gert@greenie.muc.de">gert@greenie.muc.de</a>> 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> > On Sun, Mar 7, 2021 at 11:31 AM Gert Doering <<a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a>> wrote:<br> > <br> > > 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 "ouf of memory", 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'able memory. If the amount is below 20 MB,<br> > > which is an arbitrary value "large enough for most smaller deployments",<br> > > we abort.<br> > ><br> > <br> > This is required only if privileges are dropped, isn't it? Could be made<br> > conditional<br> > on o->username is set.<br> <br> "I'm not sure", 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'd assume that the rlimit value "in existance right now" (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 "unlimited" 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't checked how other "unix"-like OSes behave.</div><div><br></div><div>Selva</div></div></div>
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
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 <<a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a>> 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> > > "I'm not sure", 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'd assume that the rlimit value "in existance right now" (specifically,<br> > > the soft limit) applies to root processes as well. Sort of a voluntary<br> > > protection against processes running away.<br> > <br> > On modern linux kernels (since some 2.6.x..) RLIMIT_MEMLOCK applies only to<br> > unprivileged processes -- privileged processes allowed to lock "unlimited"<br> > amount of memory as documented in man mlock. We updated the man page based<br> > on that sometime ago.<br> <br> Indeed, "man mlock" says something about "privileged processes" on Linux<br> (it doesn't say that on FreeBSD).<br> <br> > We could also consider using setrlimit to increase the limit before<br> > dropping privileges.<br> <br> That's another possible angle... just up soft+hard to "something"<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'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>
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
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
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
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. --
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.
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.
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
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.
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.
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
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
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"
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(-)