Re: [PATCH] Avoiding mixed socket families in PROXY protocol v1 (ticket #2594)
Hi,
On Thu, Feb 22, 2024 at 01:59:25AM +, J Carter wrote:
> Hello Roman,
>
> On Wed, 21 Feb 2024 17:29:52 +0400
> Roman Arutyunyan wrote:
>
> > Hi,
> >
>
> [...]
>
> > Checking whether the address used in PROXY writer is in fact the address
> > that was passed in the PROXY header, is complicated. This will either
> > require
> > setting a flag when PROXY address is set by realip, which is ugly.
> > Another approach is checking if the client address written to a PROXY header
> > matches the client address in the received PROXY header. However since
> > currently PROXY protocol addresses are stored as text, and not all addresses
> > have unique text repersentations, this approach would require refactoring
> > all
> > PROXY protocol code + realip modules to switch from text to sockaddr.
> >
> > I suggest that we follow the first plan (INADDR_ANY etc).
> >
> > > [...]
> >
> > Updated patch attached.
> >
> > --
> > Roman Arutyunyan
>
> > diff --git a/src/core/ngx_proxy_protocol.c b/src/core/ngx_proxy_protocol.c
> > --- a/src/core/ngx_proxy_protocol.c
> > +++ b/src/core/ngx_proxy_protocol.c
> > @@ -279,7 +279,10 @@ ngx_proxy_protocol_read_port(u_char *p,
> > u_char *
> > ngx_proxy_protocol_write(ngx_connection_t *c, u_char *buf, u_char *last)
> > {
> > -ngx_uint_t port, lport;
> > +socklen_t local_socklen;
> > +ngx_uint_t port, lport;
> > +struct sockaddr*local_sockaddr;
> > +static ngx_sockaddr_t default_sockaddr;
>
> I understand you are using the fact static variables are zero
> initalized - to be both INADDR_ANY and "IN6ADDR_ANY", however is
> this defined behavior for a union (specifically for ipv6 case) ?
>
> I was under the impression only the first declared member, along with
> padding bits were garunteed to be zero'ed.
>
> https://stackoverflow.com/questions/54160137/what-constitutes-as-padding-in-a-union
It's not clear what exactly is meant by padding in that particular statement.
It may as well be everything beyong the first member. However C99 does not
require zeroing out the padding anyway.
I can hardly believe there's a platform where the second union member may
in fact be non-zero in this case. The entire union is allocated in the BSS
segment and is not even present in the binary file.
The reasons why C standard has this grey area are clear. Zeroing out a chunk
of memory may not be equivalent to initializing particular members to NULL, 0.0
etc on certain platforms. However nginx already heavily relies on ngx_memzero()
for initializing most structures. For platforms where this works, there are
no reasons why a static union would not be allocated in a BSS segment.
However it's not hard to avoid the issue completely, just to be on the safe
side. Thanks for noticing this.
> > if (last - buf < NGX_PROXY_PROTOCOL_V1_MAX_HEADER) {
> > ngx_log_error(NGX_LOG_ALERT, c->log, 0,
> > @@ -312,11 +315,21 @@ ngx_proxy_protocol_write(ngx_connection_
> >
> > *buf++ = ' ';
> >
> > -buf += ngx_sock_ntop(c->local_sockaddr, c->local_socklen, buf, last -
> > buf,
> > - 0);
> > +if (c->sockaddr->sa_family == c->local_sockaddr->sa_family) {
> > +local_sockaddr = c->local_sockaddr;
> > +local_socklen = c->local_socklen;
> > +
> > +} else {
> > +default_sockaddr.sockaddr.sa_family = c->sockaddr->sa_family;
> > +
> > +local_sockaddr = _sockaddr.sockaddr;
> > +local_socklen = sizeof(ngx_sockaddr_t);
> > +}
> > +
> > +buf += ngx_sock_ntop(local_sockaddr, local_socklen, buf, last - buf,
> > 0);
> >
> > port = ngx_inet_get_port(c->sockaddr);
> > -lport = ngx_inet_get_port(c->local_sockaddr);
> > +lport = ngx_inet_get_port(local_sockaddr);
> >
> > return ngx_slprintf(buf, last, " %ui %ui" CRLF, port, lport);
> > }
> >
> ___
> nginx-devel mailing list
> nginx-devel@nginx.org
> https://mailman.nginx.org/mailman/listinfo/nginx-devel
--
Roman Arutyunyan
# HG changeset patch
# User Roman Arutyunyan
# Date 1708522464 -14400
# Wed Feb 21 17:34:24 2024 +0400
# Node ID 2d9bb7b49d64576fa29a673133129f16de3cfbbe
# Parent 44da04c2d4db94ad4eefa84b299e07c5fa4a00b9
Avoiding mixed socket families in PROXY protocol v1 (ticket #2010).
When using realip module, remote and local addresses of a connection can belong
to different address families. This previously resulted in generating PROXY
protocol headers like this:
PROXY TCP4 127.0.0.1 unix:/tmp/nginx1.sock 55544 0
The PROXY protocol v1 specification does not allow mixed families. The change
substitutes server address with zero address in this case:
PROXY TCP4 127.0.0.1 0.0.0.0 55544 0
As an alternative, "PROXY UNKNOWN" header could be used, which unlike this
header does not contain any useful information about the client.
Also, the above mentioned format for unix socket address is not specified in
PROXY
Re: [PATCH 1 of 2] Linux packages: removed Ubuntu 23.04 'lunar' due to EOL
On Wed, Feb 21, 2024 at 01:53:47PM -0800, Konstantin Pavlov wrote: > # HG changeset patch > # User Konstantin Pavlov > # Date 1708551797 28800 > # Wed Feb 21 13:43:17 2024 -0800 > # Node ID 98a4f772621c4f0751042ab0f7e1f2d4ba53556f > # Parent e10905e43fa1d5abfdbc0bb6e9bd6e188aad6421 > Linux packages: removed Ubuntu 23.04 'lunar' due to EOL. > On Wed, Feb 21, 2024 at 01:53:48PM -0800, Konstantin Pavlov wrote: > # HG changeset patch > # User Konstantin Pavlov > # Date 1708551944 28800 > # Wed Feb 21 13:45:44 2024 -0800 > # Node ID 646ce0bcdac6817560f1c39bbcdf7439cc0be73d > # Parent 98a4f772621c4f0751042ab0f7e1f2d4ba53556f > Removed Maxim Dounin's PGP key. > Ok for me. ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[njs] Shell: added QuickJS engine support.
details: https://hg.nginx.org/njs/rev/cb3e068a511c
branches:
changeset: 2290:cb3e068a511c
user: Dmitry Volyntsev
date: Thu Feb 22 20:25:43 2024 -0800
description:
Shell: added QuickJS engine support.
diffstat:
auto/expect |22 +-
auto/make|23 +-
auto/options | 2 +
auto/quickjs |55 +
auto/summary | 4 +
configure| 1 +
external/njs_shell.c | 2225 -
src/njs_builtin.c| 3 +
src/test/njs_unit_test.c | 4 +-
test/setup | 5 +
test/shell_test.exp | 361 +---
test/shell_test_njs.exp | 418
test/test262 | 5 +
13 files changed, 2531 insertions(+), 597 deletions(-)
diffs (truncated from 3866 to 1000 lines):
diff -r 272af619b821 -r cb3e068a511c auto/expect
--- a/auto/expect Thu Feb 22 17:38:58 2024 -0800
+++ b/auto/expect Thu Feb 22 20:25:43 2024 -0800
@@ -20,11 +20,31 @@ fi
if [ $njs_found = yes -a $NJS_HAVE_READLINE = YES ]; then
cat << END >> $NJS_MAKEFILE
-shell_test:njs test/shell_test.exp
+shell_test_njs:njs test/shell_test.exp
PATH=$NJS_BUILD_DIR:\$(PATH) LANG=C.UTF-8 TERM=screen \
expect -f test/shell_test.exp
+ PATH=$NJS_BUILD_DIR:\$(PATH) LANG=C.UTF-8 TERM=screen \
+expect -f test/shell_test_njs.exp
END
+if [ $NJS_HAVE_QUICKJS = YES ]; then
+cat << END >> $NJS_MAKEFILE
+
+shell_test:shell_test_njs shell_test_quickjs
+
+shell_test_quickjs:njs test/shell_test.exp
+ PATH=$NJS_BUILD_DIR:\$(PATH) LANG=C.UTF-8 TERM=screen
NJS_ENGINE=QuickJS \
+expect -f test/shell_test.exp
+END
+
+else
+cat << END >> $NJS_MAKEFILE
+
+shell_test:shell_test_njs
+END
+
+fi
+
else
echo " - expect tests are disabled"
diff -r 272af619b821 -r cb3e068a511c auto/make
--- a/auto/make Thu Feb 22 17:38:58 2024 -0800
+++ b/auto/make Thu Feb 22 20:25:43 2024 -0800
@@ -241,8 +241,7 @@ lib_test: $NJS_BUILD_DIR/njs_auto_config
$NJS_BUILD_DIR/lvlhsh_unit_test
$NJS_BUILD_DIR/unicode_unit_test
-test262: njs
-
+test262_njs: njs
test/test262 --binary=$NJS_BUILD_DIR/njs
unit_test: $NJS_BUILD_DIR/njs_auto_config.h \\
@@ -265,6 +264,26 @@ dist:
&& echo njs-\$(NJS_VER).tar.gz done
END
+if [ $NJS_HAVE_QUICKJS = YES ]; then
+cat << END >> $NJS_MAKEFILE
+
+test262: njs test262_njs test262_quickjs
+
+test262_quickjs: njs
+ NJS_SKIP_LIST="test/js/promise_rejection_tracker_recursive.t.js \\
+test/js/async_exception_in_await.t.js" \\
+ test/test262 --binary='$NJS_BUILD_DIR/njs -n QuickJS -m'
+END
+
+else
+cat << END >> $NJS_MAKEFILE
+
+test262: njs test262_njs
+END
+
+fi
+
+
njs_ts_deps=`echo $NJS_TS_SRCS \
| sed -e "s# *\([^ ][^ ]*\)#\1$njs_regex_cont#g"`
diff -r 272af619b821 -r cb3e068a511c auto/options
--- a/auto/options Thu Feb 22 17:38:58 2024 -0800
+++ b/auto/options Thu Feb 22 20:25:43 2024 -0800
@@ -14,6 +14,7 @@ NJS_DEBUG_GENERATOR=NO
NJS_ADDRESS_SANITIZER=NO
NJS_ADDR2LINE=NO
+NJS_QUICKJS=YES
NJS_OPENSSL=YES
NJS_LIBXML2=YES
NJS_ZLIB=YES
@@ -47,6 +48,7 @@ do
--debug-opcode=*)NJS_DEBUG_OPCODE="$value" ;;
--debug-generator=*) NJS_DEBUG_GENERATOR="$value";;
+--no-quickjs)NJS_QUICKJS=NO ;;
--no-openssl)NJS_OPENSSL=NO ;;
--no-libxml2)NJS_LIBXML2=NO ;;
--no-zlib) NJS_ZLIB=NO ;;
diff -r 272af619b821 -r cb3e068a511c auto/quickjs
--- /dev/null Thu Jan 01 00:00:00 1970 +
+++ b/auto/quickjs Thu Feb 22 20:25:43 2024 -0800
@@ -0,0 +1,55 @@
+
+# Copyright (C) Dmitry Volyntsev
+# Copyright (C) NGINX, Inc.
+
+
+NJS_QUICKJS_LIB=
+NJS_HAVE_QUICKJS=NO
+
+if [ $NJS_QUICKJS = YES ]; then
+njs_found=no
+
+njs_feature="QuickJS library"
+njs_feature_name=NJS_HAVE_QUICKJS
+njs_feature_run=yes
+njs_feature_incs=
+njs_feature_libs=""
+njs_feature_test="#if defined(__GNUC__) && (__GNUC__ >= 8)
+ #pragma GCC diagnostic push
+ #pragma GCC diagnostic ignored \"-Wcast-function-type\"
+ #endif
+
+ #include
+
+ int main() {
+ JSRuntime *rt;
+
+ rt = JS_NewRuntime();
+ JS_FreeRuntime(rt);
+ return 0;
+ }"
+. auto/feature
+
+if [ $njs_found = no ]; then
+njs_feature="QuickJS library -lquickjs.lto"
+njs_feature_incs="/usr/include/quickjs/"
+njs_feature_libs="-L/usr/lib/quickjs/ -lquickjs.lto -lm -ldl -lpthread"
+
+. auto/feature
+fi
+
+if [ $njs_found = no ]; then
+
[njs] Fixed atob() with non-padded base64 strings.
details: https://hg.nginx.org/njs/rev/272af619b821
branches:
changeset: 2289:272af619b821
user: Dmitry Volyntsev
date: Thu Feb 22 17:38:58 2024 -0800
description:
Fixed atob() with non-padded base64 strings.
This fixes #695 issue on Github.
diffstat:
src/njs_string.c | 9 -
src/test/njs_unit_test.c | 12
2 files changed, 20 insertions(+), 1 deletions(-)
diffs (41 lines):
diff -r 0479e5821ab2 -r 272af619b821 src/njs_string.c
--- a/src/njs_string.c Wed Feb 14 21:34:02 2024 -0800
+++ b/src/njs_string.c Thu Feb 22 17:38:58 2024 -0800
@@ -4298,7 +4298,14 @@ njs_string_atob(njs_vm_t *vm, njs_value_
}
}
-len = njs_base64_decoded_length(str.length, pad);
+len = str.length;
+
+if (len % 4 != 0) {
+pad = 4 - (len % 4);
+len += pad;
+}
+
+len = njs_base64_decoded_length(len, pad);
njs_chb_init(, vm->mem_pool);
diff -r 0479e5821ab2 -r 272af619b821 src/test/njs_unit_test.c
--- a/src/test/njs_unit_test.c Wed Feb 14 21:34:02 2024 -0800
+++ b/src/test/njs_unit_test.c Thu Feb 22 17:38:58 2024 -0800
@@ -10093,6 +10093,18 @@ static njs_unit_test_t njs_test[] =
"].every(v => c(atob(v)).toString() == '8,52,86')"),
njs_str("true")},
+{ njs_str("atob('aGVsbG8=')"),
+ njs_str("hello") },
+
+{ njs_str("atob('aGVsbG8')"),
+ njs_str("hello") },
+
+{ njs_str("atob('TQ==')"),
+ njs_str("M") },
+
+{ njs_str("atob('TQ')"),
+ njs_str("M") },
+
/* Functions. */
{ njs_str("return"),
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
