naviserver-devel — Developer discussion

Re: [naviserver-devel] ns_mktemp

[Wolfgang W. <wol...@di...>]

I like the solution suggested by Andrew as well. This should allow an 
easy transition of existing code without sacrificing security.

Am 07.10.24 um 17:59 schrieb Andrew Piskorski:
> On Mon, Oct 07, 2024 at 12:54:32PM +0200, Gustaf Neumann (sslmail) wrote:
>
>> However, there are many cases, where existing programs use "ns_mkstemp", which cannot be replaced easily. When looking at OpenACS, I see 33 cases like
>>
>> - the temporary name is passed to an external program (e.g. "tar", "zip", image creation), or
>> - the temporary name is passed to a Tcl function expecting a filename (e.g. "file copy").
> Interesting.  It sounds like are no 100% good solutions for this, and
> in fact there CAN'T be.
>
>> - Call the safe function (e.g. mkstemp()) and delete the file, while
>>    producing a depreciation message? This could also be done on the Tcl-level.
> I guess so.  Probably with a switch to control whether the file is
> created and remains (new-style behavior), or gets quickly deleted
> (more fully backward compatible).
>
> I'd lean towards turning ns_mktemp into a wrapper around Tcl's "file
> tempfile".  That calls TclUnixOpenTemporaryFile() and thus mkstemp()
> or mkstemps(), but it looks like Tcl has already done the necessary
> work to easily make the wrapper backwards compatible with the
> old-style ns_mktemp.  And having the compatibility wrapper in Tcl
> instead of C makes it a lot easier for NaviServer users to adjust it
> to their own needs if necessary.
>

Re: [naviserver-devel] ns_mktemp

[Brian F. <bri...@ai...>]

I agree with Andrew's suggested approach here.

Brian
________________________________
From: Andrew Piskorski <at...@pi...>
Sent: Monday 7 October 2024 4:59 pm
To: nav...@li... <nav...@li...>
Subject: Re: [naviserver-devel] ns_mktemp

On Mon, Oct 07, 2024 at 12:54:32PM +0200, Gustaf Neumann (sslmail) wrote:

> However, there are many cases, where existing programs use "ns_mkstemp", which cannot be replaced easily. When looking at OpenACS, I see 33 cases like
>
> - the temporary name is passed to an external program (e.g. "tar", "zip", image creation), or
> - the temporary name is passed to a Tcl function expecting a filename (e.g. "file copy").

Interesting.  It sounds like are no 100% good solutions for this, and
in fact there CAN'T be.

> - Call the safe function (e.g. mkstemp()) and delete the file, while
>   producing a depreciation message? This could also be done on the Tcl-level.

I guess so.  Probably with a switch to control whether the file is
created and remains (new-style behavior), or gets quickly deleted
(more fully backward compatible).

I'd lean towards turning ns_mktemp into a wrapper around Tcl's "file
tempfile".  That calls TclUnixOpenTemporaryFile() and thus mkstemp()
or mkstemps(), but it looks like Tcl has already done the necessary
work to easily make the wrapper backwards compatible with the
old-style ns_mktemp.  And having the compatibility wrapper in Tcl
instead of C makes it a lot easier for NaviServer users to adjust it
to their own needs if necessary.

--
Andrew Piskorski <at...@pi...>


_______________________________________________
naviserver-devel mailing list
nav...@li...
https://lists.sourceforge.net/lists/listinfo/naviserver-devel

Re: [naviserver-devel] ns_mktemp

[Georg L. <jor...@ma...>]

On 10/7/24 12:54, Gustaf Neumann (sslmail) wrote:
> Dear all.
> [..] However, there are many cases, where existing programs use 
> "ns_mkstemp", which cannot be replaced easily. When looking at 
> OpenACS, I see 33 cases like - the temporary name is passed to an 
> external program (e.g. "tar", "zip", image creation), or - the 
> temporaryname is passed to a Tcl function expecting a filename (e.g. 
> "file copy"). So, dropping the support for "ns_mkstemp" fully is not a 
> good option. Also, providing a "home-cooked" version of "ns_mktemp" is 
> not good either (both in Tcl or in C), since technically speaking, 
> this will not be better than the original function having the same 
> problems. Ignoring the compilation warning is not good either, since 
> sooner or later, the deprecated function will be removed. What should 
> we do? - place "ns_mktemp" into an external module? NaviServer will 
> compile nicely, but applications like OpenACS will have to load the 
> module, making administration and migration to NaviServer 5 less 
> smooth. - Call the safe function (e.g. mkstemp()) and delete the file, 
> while producing a depreciation message? This could also be done on the 
> Tcl-level.

I like this option best. It maintains backward compatibility for the 
application, encourages update to more secure approaches, discourages 
future use - especially when accompanied by respective hints in the 
documentation - and removes the warnings for up-to-date applications.

At some time in the future, the wrapped ns_mktemp could then be 
deprecated and moved out into an external module, which still allows 
legacy operations to continue using it, while raising the bar.

Best Regards,

   Georg

> Other options? Opinions? All the best
> -g [1] 
> https://pubs.opengroup.org/onlinepubs/009695399/functions/mktemp.html 
> [1] https://man.openbsd.org/OpenBSD-7.5/mkstemp.3
>
>
> _______________________________________________
> naviserver-devel mailing list
> nav...@li...
> https://lists.sourceforge.net/lists/listinfo/naviserver-devel

Re: [naviserver-devel] ns_mktemp

[Zoran V. <zv...@ar...>]

On 07.10.24 12:54, Gustaf Neumann (sslmail) wrote:
> So, dropping the support for "ns_mkstemp" fully is not a good option.

You mean ns_mktemp?

Re: [naviserver-devel] ns_mktemp

[Zoran V. <zv...@ar...>]

On 07.10.24 17:59, Andrew Piskorski wrote:
> I'd lean towards turning ns_mktemp into a wrapper around Tcl's "file
> tempfile".  That calls TclUnixOpenTemporaryFile() and thus mkstemp()
> or mkstemps(), but it looks like Tcl has already done the necessary
> work to easily make the wrapper backwards compatible with the
> old-style ns_mktemp.

Well, if this is so as stated above, then it is a no-brainer!

Re: [naviserver-devel] ns_mktemp

[Andrew P. <at...@pi...>]

On Mon, Oct 07, 2024 at 12:54:32PM +0200, Gustaf Neumann (sslmail) wrote:

> However, there are many cases, where existing programs use "ns_mkstemp", which cannot be replaced easily. When looking at OpenACS, I see 33 cases like
> 
> - the temporary name is passed to an external program (e.g. "tar", "zip", image creation), or
> - the temporary name is passed to a Tcl function expecting a filename (e.g. "file copy").

Interesting.  It sounds like are no 100% good solutions for this, and
in fact there CAN'T be.

> - Call the safe function (e.g. mkstemp()) and delete the file, while
>   producing a depreciation message? This could also be done on the Tcl-level.

I guess so.  Probably with a switch to control whether the file is
created and remains (new-style behavior), or gets quickly deleted
(more fully backward compatible).

I'd lean towards turning ns_mktemp into a wrapper around Tcl's "file
tempfile".  That calls TclUnixOpenTemporaryFile() and thus mkstemp()
or mkstemps(), but it looks like Tcl has already done the necessary
work to easily make the wrapper backwards compatible with the
old-style ns_mktemp.  And having the compatibility wrapper in Tcl
instead of C makes it a lot easier for NaviServer users to adjust it
to their own needs if necessary.

-- 
Andrew Piskorski <at...@pi...>

[naviserver-devel] ns_mktemp

[Gustaf N. (sslmail) <ne...@wu...>]

Dear all.

Since Tcl9 is out there, it becomes time for working towards the NaviServer 5 release. The most recent version of NaviServer from the main branch works nicely with the release of Tcl9 (for trying, you might consider the docker image "gustafn/naviserver/latest-tcl9-bookworm” from https://hub.docker.com/repository/docker/gustafn/naviserver/).

One of the open topics for the NaviServer 5 release is the following: The function “ns_mktemp" exists in NaviServer since ages. The function is based on the POSIX call mktemp() [1], which is unfortunately deprecated since a few years. When compiling NaviServer, one sees messages like the following:

tclfile.c:255:51: warning: 'mktemp' is deprecated: This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of mktemp(3), it is highly recommended that you use mkstemp(3) instead. [-Wdeprecated-declarations]
  255 |         Tcl_SetObjResult(interp, Tcl_NewStringObj(mktemp(buffer), TCL_INDEX_NONE));
      |                                                   ^
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/_stdlib.h:210:1: note: 'mktemp' has been explicitly marked deprecated here
  210 | __deprecated_msg("This function is provided for compatibility reasons only.  Due to security concerns inherent in the design of mktemp(3), it is highly recommended that you use mkstemp(3) instead.")
      | ^

Even worse, newer versions of OpenBSD produce warning messages whenever code is linked containing calls to mktemp() [2]. So, there is no future for mktemp() in NaviServer.

The problem with the C-level function mktemp() is that its usage leads to a race condition, opening space to attacks.  The problematic case happens when an application generates a file name with the mktemp() function and then create a file using this name. Unfortunately, this is not secure, because a different process may create a file with this name in the time between the call to mktemp() and the subsequent attempt to create the file by the first process. A malicious user can predict the name of the temporary file, resulting in other files being accessed, modified, corrupted, or deleted.

The recommended way is to combine the two steps and create the file in an atomic operation (POSIX mkstemp() for files and mkdtemp() for directories). These functions are used by Tcl "file tempfile" and by Tcl9 "file tempdir" and in NaviServer 5 via "ns_mkdtemp".

However, there are many cases, where existing programs use "ns_mkstemp", which cannot be replaced easily. When looking at OpenACS, I see 33 cases like

- the temporary name is passed to an external program (e.g. "tar", "zip", image creation), or
- the temporary name is passed to a Tcl function expecting a filename (e.g. "file copy").

So, dropping the support for "ns_mkstemp" fully is not a good option. Also, providing a "home-cooked" version of "ns_mktemp" is not good either (both in Tcl or in C), since technically speaking, this will not be better than the original function having the same problems. Ignoring the compilation warning is not good either, since sooner or later, the deprecated function will be removed.

What should we do?
- place "ns_mktemp" into an external module?
  NaviServer will compile nicely, but applications like OpenACS 
  will have to load the module, making administration and migration
  to NaviServer 5 less smooth.
- Call the safe function (e.g. mkstemp()) and delete the file, while
  producing a depreciation message? This could also be done on the Tcl-level.

Other options? Opinions?

All the best
-g

[1] https://pubs.opengroup.org/onlinepubs/009695399/functions/mktemp.html
[1] https://man.openbsd.org/OpenBSD-7.5/mkstemp.3

[naviserver-devel] NaviServer 4.99.31 available

[Gustaf N. <ne...@wu...>]

Dear all,

I am glad to announce that the release of NaviServer 4.99.30 is 
available at SourceForge [1] and on GitHub [2]. In essence, this release 
contains a few small bugfixes and some backported features from the 
NaviServer 5 developement (auto SNI and Improved handling of large file 
uploads, portability improvements). Since we want to release NaviServer 
5 after Tcl9, and Tcl9 is still not out, some of the backports become 
more urgent.


All the best!

-gustaf neumann

[1] https://sourceforge.net/projects/naviserver/files/naviserver/4.99.31/
[2] https://github.com/orgs/naviserver-project/repositories

=======================================
NaviServer 4.99.31, released 2024-08-11
=======================================

  71 files changed, 897 insertions(+), 303 deletions(-)

New Features:
-------------

   - Added auto-SNI support to "ns_http" and "ns_connchan open": When
     opening an TLS-connection to named target, the used hostname is
     automatically used as a default value for the SNI-host.  The
     automatically derived values has a lower priority than the
     parameter "-hostname" of "ns_http run" or "ns_connchan open".

     This change eases of use with cloud services and affects all
     packages using, e.g., "ns_http", including reverse proxies.

   - Introduced experimental command "ns_fseekchars": This changes
     Improves file-based parsing of multipart/form-data, supporting
     files larger than 4GB, reducing memory usage, and speeding up
     processing..


Bug Fixes:
----------

     - Fixed invalid memory reallocation of pool descriptors array
       [commit 7efaafe4].

     - Fixed potential issue for move operation with overlapping memory
       areas. This change is which was critical for ARM architecture
       when musl is used (Alpine Linux) [commit 20438ed4].

     - Fixed potential crash in "ns_conn copy" to handle cases where an
       incoming request contains no content [commit 79b8cb8a].


Changes in Sample Configuration Files:
--------------------------------------

     - Sample configuration file updates for handling container
       mappings and whitelisting domain names [commits ece849ac,
       9a102a97, 86a176ab].

     - Backported from NaviServer 5: Includes multiple changes to
       configuration files and scripts [commit 0d32e5ea].

Misc Improvements:
------------------

     - Improved forward compatibility to ease backports [commit ef8b4386].

     - Improved spelling and documentation across multiple files to
       enhance readability and understanding [multiple commits e.g.,
       a2cdaa10, 15253120].

     - Improved portability by not assuming that type "char" is the
       same as "unsigned char", important for compatibility with
       different architectures [commit 7635ef00].

     - Made testing more robust for docker images with partial IPv6
       setups to improve reliability in varied deployment environments
       [commit 516ca38c].

     - Removed misleading and unneeded "-encoding binary": This command
       was a no-op since many Tcl releases and was removed in Tcl9 at
       all. This change clarifies code usage [commit a36d15b1].

Re: [naviserver-devel] Problem with a Tcl extension and NaviServer

[Matthew B. <mm...@gm...>]

After doing a little digging, I tracked it down to a line in the USB
library; sure enough, IOHIDDeviceOpen returns a "privilege violation"
error. And just to confirm, if I run NaviServer as root, it all works fine.
So thanks again Gustaf for pointing me in the right direction.

Now I just need to puzzle out why there's no privilege violation when I run
the code via tclsh from the command line. Hopefully that will give me some
ideas for a fix since running NaviServer as root isn't really a practical
option.

Regards,

Matt



On Sat, Jun 29, 2024 at 8:48 AM <
nav...@li...> wrote:

> Date: Fri, 28 Jun 2024 12:41:30 -0400
> From: Matthew Burke <mm...@gm...>
> To: nav...@li...
> Subject: Re: [naviserver-devel] Problem with a Tcl extension and NaviServer
> Message-ID:
>         <
> CAC...@ma...>
> Content-Type: text/plain; charset="utf-8"
>
> Hi Gustaf,
>
> Thanks for the reply. I'll start looking into permissions issues and see if
> anything there looks promising. I should also instrument the USB library
> and try to get a better sense of what, exactly, is failing.
>
> Regards,
>
> Matt
>
>
> On Thu, Jun 27, 2024 at 9:15?AM <
> nav...@li...> wrote:
>
> >
> > Dear Mathew,
> >
> > The answer for your this problem is probably in the call of
> > blink1_openById(devid);
> >
> > Without looking into this call, I would suspect first from my experience
> > with NaviServer IoT projects a permission problem when accessing the
> > hardware interface.
> > For security reasons, NaviServer switches to a nonprivileged user ...
> >
> > all the best -g
> >
> > On 25.06.24 16:18, Matthew Burke wrote:
> > > Hello!
> > >
> > > I am writing a Tcl extension in C to wrap a driver for a USB LED
> > > status light. The extension runs fine from tclsh but there are issues
> > > running it from a Tcl ?page under NaviServer.
> > >
> > > The extension implements one command via Tcl_CreateObjCommand. The
> > > command proc implements a number of sub-commands broken into two
> > > categories: device-independent functionality, e.g. count the number of
> > > status lights that are plugged in, and device-dependent functionality,
> > > e.g. attach to a status light, display a particular color, etc
> > >
> > > The device-independent sub-commands work when run in a Tcl page. For
> > > example, this page runs fine when served from NaviServer:
> > >
> > >
> > > package require blink
> > >
> > > set n [blink enumerate]
> > >
> > > ns_return 200 text/html "
> > > There are <strong>$n</strong> devices plugged in.
> > > "
> > >
> > >
> > > In order to have a device display a color, etc., you first have to run
> > > the "open" command. And that's not working. So here's an example Tcl
> > page:
> > >
> > >
> > > package require blink
> > >
> > > set d [blink open 0]
> > > set q [ns_conn query]
> > > set s [ns_parsequery $q]
> > >
> > > set red [ns_set get $s red 255]
> > > set green [ns_set get $s green 0]
> > > set blue [ns_set get $s blue 0]
> > >
> > > blink set $d $red $green $blue
> > >
> > > ns_return 200 text/html "
> > > ? ? <p>Query: [ns_conn query]</p>
> > >
> > > ? ? <ul>
> > > ? ? <li>Red: $red</li>
> > > ? ? <li>Green: $green</li>
> > > ? ? <li>Blue: $blue</li>
> > > ? ? </ul>
> > > "
> > >
> > >
> > > When this page is served, I get an error in the line: set d [blink
> > > open 0].
> > >
> > > There is a C struct, Blinker, ?that contains data about the attached
> > > device. My C code first allocates
> > > memory for the struct
> > >
> > >
> > > blinkPtr = (Blinker *)Tcl_Alloc(sizeof(Blinker));
> > >
> > >
> > > and this seems to work -- blinkPtr is non-NULL after the assignment.
> > > Then there is a function from the driver library that actually
> > > attaches to the device:
> > >
> > > blinkPtr->device = blink1_openById(devid);
> > >
> > > It should return the memory address of an allocated structure that's
> > > part of the USB library. But the?function?returns NULL. (Just to
> > > reiterate, this works fine when run from tclsh and returns a non-NULL
> > > value.)
> > >
> > > I'm assuming there is some sort of memory protection or similar that I
> > > need to take into account. Any suggestions on what to look at would be
> > > greatly appreciated.
> > >
> > > Thanks,
> > >
> > > Matt
> > >
> > > P.S. This is on macOS 14.3. I'm compiling the extension as follows:
> > >
> > > clang -dynamiclib -DUSE_TCL_STUBS -I/usr/local/ns/include
> > > -L/usr/local/ns/lib -ltclstub8.6 -o blink.dylib blink.c
> > > -L/usr/local/lib -lBlink1
> > >
> > >
> > >
> > >
> > > _______________________________________________
> > > naviserver-devel mailing list
> > > nav...@li...
> > > https://lists.sourceforge.net/lists/listinfo/naviserver-devel
> >
> >
> >
> > ------------------------------
> >
> >
> >
> > ------------------------------
> >
> > Subject: Digest Footer
> >
> > _______________________________________________
> > naviserver-devel mailing list
> > nav...@li...
> > https://lists.sourceforge.net/lists/listinfo/naviserver-devel
> >
> >
> > ------------------------------
> >
> > End of naviserver-devel Digest, Vol 162, Issue 3
> > ************************************************
> >
> -------------- next part --------------
> An HTML attachment was scrubbed...
>
> ------------------------------
>
>
>
> ------------------------------
>
> Subject: Digest Footer
>
> _______________________________________________
> naviserver-devel mailing list
> nav...@li...
> https://lists.sourceforge.net/lists/listinfo/naviserver-devel
>
>
> ------------------------------
>
> End of naviserver-devel Digest, Vol 162, Issue 4
> ************************************************
>

Re: [naviserver-devel] Problem with a Tcl extension and NaviServer

[Matthew B. <mm...@gm...>]

Hi Gustaf,

Thanks for the reply. I'll start looking into permissions issues and see if
anything there looks promising. I should also instrument the USB library
and try to get a better sense of what, exactly, is failing.

Regards,

Matt


On Thu, Jun 27, 2024 at 9:15 AM <
nav...@li...> wrote:

>
> Dear Mathew,
>
> The answer for your this problem is probably in the call of
> blink1_openById(devid);
>
> Without looking into this call, I would suspect first from my experience
> with NaviServer IoT projects a permission problem when accessing the
> hardware interface.
> For security reasons, NaviServer switches to a nonprivileged user ...
>
> all the best -g
>
> On 25.06.24 16:18, Matthew Burke wrote:
> > Hello!
> >
> > I am writing a Tcl extension in C to wrap a driver for a USB LED
> > status light. The extension runs fine from tclsh but there are issues
> > running it from a Tcl ?page under NaviServer.
> >
> > The extension implements one command via Tcl_CreateObjCommand. The
> > command proc implements a number of sub-commands broken into two
> > categories: device-independent functionality, e.g. count the number of
> > status lights that are plugged in, and device-dependent functionality,
> > e.g. attach to a status light, display a particular color, etc
> >
> > The device-independent sub-commands work when run in a Tcl page. For
> > example, this page runs fine when served from NaviServer:
> >
> >
> > package require blink
> >
> > set n [blink enumerate]
> >
> > ns_return 200 text/html "
> > There are <strong>$n</strong> devices plugged in.
> > "
> >
> >
> > In order to have a device display a color, etc., you first have to run
> > the "open" command. And that's not working. So here's an example Tcl
> page:
> >
> >
> > package require blink
> >
> > set d [blink open 0]
> > set q [ns_conn query]
> > set s [ns_parsequery $q]
> >
> > set red [ns_set get $s red 255]
> > set green [ns_set get $s green 0]
> > set blue [ns_set get $s blue 0]
> >
> > blink set $d $red $green $blue
> >
> > ns_return 200 text/html "
> > ? ? <p>Query: [ns_conn query]</p>
> >
> > ? ? <ul>
> > ? ? <li>Red: $red</li>
> > ? ? <li>Green: $green</li>
> > ? ? <li>Blue: $blue</li>
> > ? ? </ul>
> > "
> >
> >
> > When this page is served, I get an error in the line: set d [blink
> > open 0].
> >
> > There is a C struct, Blinker, ?that contains data about the attached
> > device. My C code first allocates
> > memory for the struct
> >
> >
> > blinkPtr = (Blinker *)Tcl_Alloc(sizeof(Blinker));
> >
> >
> > and this seems to work -- blinkPtr is non-NULL after the assignment.
> > Then there is a function from the driver library that actually
> > attaches to the device:
> >
> > blinkPtr->device = blink1_openById(devid);
> >
> > It should return the memory address of an allocated structure that's
> > part of the USB library. But the?function?returns NULL. (Just to
> > reiterate, this works fine when run from tclsh and returns a non-NULL
> > value.)
> >
> > I'm assuming there is some sort of memory protection or similar that I
> > need to take into account. Any suggestions on what to look at would be
> > greatly appreciated.
> >
> > Thanks,
> >
> > Matt
> >
> > P.S. This is on macOS 14.3. I'm compiling the extension as follows:
> >
> > clang -dynamiclib -DUSE_TCL_STUBS -I/usr/local/ns/include
> > -L/usr/local/ns/lib -ltclstub8.6 -o blink.dylib blink.c
> > -L/usr/local/lib -lBlink1
> >
> >
> >
> >
> > _______________________________________________
> > naviserver-devel mailing list
> > nav...@li...
> > https://lists.sourceforge.net/lists/listinfo/naviserver-devel
>
>
>
> ------------------------------
>
>
>
> ------------------------------
>
> Subject: Digest Footer
>
> _______________________________________________
> naviserver-devel mailing list
> nav...@li...
> https://lists.sourceforge.net/lists/listinfo/naviserver-devel
>
>
> ------------------------------
>
> End of naviserver-devel Digest, Vol 162, Issue 3
> ************************************************
>

Re: [naviserver-devel] Problem with a Tcl extension and NaviServer

[Gustaf N. <ne...@wu...>]

Dear Mathew,

The answer for your this problem is probably in the call of 
blink1_openById(devid);

Without looking into this call, I would suspect first from my experience 
with NaviServer IoT projects a permission problem when accessing the 
hardware interface.
For security reasons, NaviServer switches to a nonprivileged user ...

all the best -g

On 25.06.24 16:18, Matthew Burke wrote:
> Hello!
>
> I am writing a Tcl extension in C to wrap a driver for a USB LED 
> status light. The extension runs fine from tclsh but there are issues 
> running it from a Tcl  page under NaviServer.
>
> The extension implements one command via Tcl_CreateObjCommand. The 
> command proc implements a number of sub-commands broken into two 
> categories: device-independent functionality, e.g. count the number of 
> status lights that are plugged in, and device-dependent functionality, 
> e.g. attach to a status light, display a particular color, etc
>
> The device-independent sub-commands work when run in a Tcl page. For 
> example, this page runs fine when served from NaviServer:
>
>
> package require blink
>
> set n [blink enumerate]
>
> ns_return 200 text/html "
> There are <strong>$n</strong> devices plugged in.
> "
>
>
> In order to have a device display a color, etc., you first have to run 
> the "open" command. And that's not working. So here's an example Tcl page:
>
>
> package require blink
>
> set d [blink open 0]
> set q [ns_conn query]
> set s [ns_parsequery $q]
>
> set red [ns_set get $s red 255]
> set green [ns_set get $s green 0]
> set blue [ns_set get $s blue 0]
>
> blink set $d $red $green $blue
>
> ns_return 200 text/html "
>     <p>Query: [ns_conn query]</p>
>
>     <ul>
>     <li>Red: $red</li>
>     <li>Green: $green</li>
>     <li>Blue: $blue</li>
>     </ul>
> "
>
>
> When this page is served, I get an error in the line: set d [blink 
> open 0].
>
> There is a C struct, Blinker,  that contains data about the attached 
> device. My C code first allocates
> memory for the struct
>
>
> blinkPtr = (Blinker *)Tcl_Alloc(sizeof(Blinker));
>
>
> and this seems to work -- blinkPtr is non-NULL after the assignment. 
> Then there is a function from the driver library that actually 
> attaches to the device:
>
> blinkPtr->device = blink1_openById(devid);
>
> It should return the memory address of an allocated structure that's 
> part of the USB library. But the function returns NULL. (Just to 
> reiterate, this works fine when run from tclsh and returns a non-NULL 
> value.)
>
> I'm assuming there is some sort of memory protection or similar that I 
> need to take into account. Any suggestions on what to look at would be 
> greatly appreciated.
>
> Thanks,
>
> Matt
>
> P.S. This is on macOS 14.3. I'm compiling the extension as follows:
>
> clang -dynamiclib -DUSE_TCL_STUBS -I/usr/local/ns/include 
> -L/usr/local/ns/lib -ltclstub8.6 -o blink.dylib blink.c 
> -L/usr/local/lib -lBlink1
>
>
>
>
> _______________________________________________
> naviserver-devel mailing list
> nav...@li...
> https://lists.sourceforge.net/lists/listinfo/naviserver-devel

[naviserver-devel] Problem with a Tcl extension and NaviServer

[Matthew B. <mm...@gm...>]

Hello!

I am writing a Tcl extension in C to wrap a driver for a USB LED status
light. The extension runs fine from tclsh but there are issues running it
from a Tcl  page under NaviServer.

The extension implements one command via Tcl_CreateObjCommand. The command
proc implements a number of sub-commands broken into two categories:
device-independent functionality, e.g. count the number of status lights
that are plugged in, and device-dependent functionality, e.g. attach to a
status light, display a particular color, etc

The device-independent sub-commands work when run in a Tcl page. For
example, this page runs fine when served from NaviServer:


package require blink

set n [blink enumerate]

ns_return 200 text/html "
There are <strong>$n</strong> devices plugged in.
"


In order to have a device display a color, etc., you first have to run the
"open" command. And that's not working. So here's an example Tcl page:


package require blink

set d [blink open 0]
set q [ns_conn query]
set s [ns_parsequery $q]

set red [ns_set get $s red 255]
set green [ns_set get $s green 0]
set blue [ns_set get $s blue 0]

blink set $d $red $green $blue

ns_return 200 text/html "
    <p>Query: [ns_conn query]</p>

    <ul>
    <li>Red: $red</li>
    <li>Green: $green</li>
    <li>Blue: $blue</li>
    </ul>
"


When this page is served, I get an error in the line: set d [blink open 0].

There is a C struct, Blinker,  that contains data about the attached
device. My C code first allocates
memory for the struct


blinkPtr = (Blinker *)Tcl_Alloc(sizeof(Blinker));


and this seems to work -- blinkPtr is non-NULL after the assignment. Then
there is a function from the driver library that actually attaches to the
device:

blinkPtr->device = blink1_openById(devid);

It should return the memory address of an allocated structure that's part
of the USB library. But the function returns NULL. (Just to reiterate, this
works fine when run from tclsh and returns a non-NULL value.)

I'm assuming there is some sort of memory protection or similar that I need
to take into account. Any suggestions on what to look at would be greatly
appreciated.

Thanks,

Matt

P.S. This is on macOS 14.3. I'm compiling the extension as follows:

clang -dynamiclib -DUSE_TCL_STUBS -I/usr/local/ns/include
-L/usr/local/ns/lib -ltclstub8.6 -o blink.dylib blink.c -L/usr/local/lib
-lBlink1

[naviserver-devel] Improvement for file uploads

[Gustaf N. <ne...@wu...>]

Dear all, I've committed the following change to the GitHub repository 
of NaviServer, that adds significant improvements for FORM uploads of 
large files. It makes it now possible to handle files uploads via 
multipart/form-data (usual format) larger than 4GB without crashing. The 
support is just for NaviServer, applications (e.g. OpenACS) might still 
try to read such large files into Tcl_Objs leading to crashes with Tcl 
8. Although, loading huge data into memory is not a good practice and 
leads to memory bloats. However, this should work with Tcl9.
The code replaces an implementation that has not changed for the last 17 
years.
I am planing to backport this change also to the 4.99 branch.
All the best
-g


Added experimental command "ns_fseekchars" and use it in form.tcl for parsing multipart/form-data

As a consequence,
(a) the file-based parser of multipart/form-data is able to read files >4GB,
(b) leads to less memory bloat and
(c) is more than a factor of 10 faster

        file size        old  ns_fseekchars  factor
           65,517       4,471           151   29.61
          124,523       1,139            94   12.12
       74,006,378     682,375        54,752   12.46
    2,104,408,064  18,916,496     1,564,472   12.09
    3,992,977,408  35,942,768     3,061,061   11.74
    5,368,709,120                 3,817,896	

The problem with the old file-based parser was that it was searching for boundary
strings using the Tcl "gets" command:

		if { [string match $boundary* [string trim [gets $fp]]] } {
                 ...
                 }

Since "gets" reads a line (i.e., all character until the next new
line) Tcl can crash, when the next new line is more than 4GB
away. Even with e.g. 2GB, it will temporarily create a Tcl_Obj with
2GB content, which is stripped etc. leading therefore to a potential
memory bloat keeping multiple huge Tcl_Objs in memory.

The new code avoids all this by performing the search for the boundary
in C.

TODO: add documentation page

[naviserver-devel] supporting relative redirects in NaviServer

[Gustaf N. (sslmail) <ne...@wu...>]

Dear all,

RFC 2616 requires an absolute URI in the "Location" header field. So if someone calls "ns_returnredirect /", NaviServer transforms it on the fly into an absolute URL by prefixing it with the location (e.g. https://openacs.org/). NaviServer (and OpenACS) has some complex code to compute the location value, especially when virtual servers are involved (or for "host-node mapped" subsites in OpenACS). The situation is further complicated when running behind a reverse proxy and/or in a containerized environment. In such cases, the location is computed from the "host" request header field, which must be validated, otherwise an attacker could hijack a session and redirect it to a spoofed site. 

The situation changed 10 years ago (June 2014) with the introduction of RFC 7231, which allows relative redirects (see https://www.rfc-editor.org/rfc/rfc7231#section-7.1.2). Using relative redirects greatly simplifies configuration and closes the attack vector using the host header field. RFC 7231 has been superseded by RFC 9110 (June 2022), which also supports relative redirects via the "location" response header field (see https://www.rfc-editor.org/rfc/rfc9110#field.location).

The latest version of NaviServer (already running on OpenACS.org) now implements relative redirects as supported by the newer RFCs. Only if a location proc has been registered "ns_locationproc", the URL will be prefixed with it for better backwards compatibility.

The change to support relative redirects should simplify many configurations. There have been many questions about redirects where people have struggled to set them up. I've only added this change to the main branch of NaviServer. I have no intention of backporting it to the 4.99 series at this time.

If you have any objections or suggestions, please let me know.

Re: [naviserver-devel] Order of URL processing

[Gustaf N. (sslmail) <ne...@wu...>]

There are multiple processing steps involved in your example. One thing is how to use the URLspace mappings (responsible for all kinds of requests, but not only), where the URL ordering plays a rule, and the another thing are request filters, which are fully independent of the URLspace.

Given the lifecycle of requests, the resolving of the requests happens in the step “process request” below:


Once a connection thread is selected, the server runs filters (pre-auth and post-auth) and then it runs the request procs.  




There might be multiple of these filters, where the registration order is relevant. A filter might decide to continue to the next filter or send a reply to the client. This is singled via the result of the filter procs. A filter should return one of the three values “filter_ok”, “filter_break” or “filter_return” (for details, check the man page of ns_register)

https://naviserver.sourceforge.io/n/naviserver/files/ns_register.html#3

In your case, the reverse proxy is registered via a match-all post-auth filter (/*) ending with a “filter-return” which has the consequence no request-proc (such as fastpath) will be involved. So registering additional request procs won’t make a difference.

In your example, one could either register an additional filter at the beginning of the filter change that lets the static requests through (ending with filter_break),, or one could consider moving the handling of the reverse proxy to the URLspace, by registering the handlers via ns_register_proc instead of using ns_register_filter.

Hope this explains,

all the best
-g

> On 02.05.2024, at 16:33, Georg Lehner <jor...@ma...> wrote:
> I am trying to configure Naviserver as reverse proxy for a Django project.
> 
> Django features deployment of static assests like css, js etc. into a separate URL, typically /static,  which can then be served directly by the webserver.  Everything else has to be reverse-proxied to the wsgi server which serves the Django app via http.
> 
> My revproxy setup is:
> 
> ns_section ns/server/default {
>     ns_param enabletclpages     false
>     ns_param minthreads         10
> }
> ns_section ns/server/default/fastpath {
>     ns_param pagedir            www
>     ns_param serverdir          $serverroot/default
>     ns_param directoryproc      ns_returnnotfound
>     ns_param directoryfile      index.html
> }
> ns_section ns/server/default/modules {
>     ns_param revproxy           tcl
>     ns_param nssock             nssock
> }
> ns_section ns/server/default/module/revproxy {
>     ns_param filters {
>         ns_register_filter postauth GET  /* ::revproxy::upstream -target http://localhost:65193 <http://localhost:65193/>
>         ns_register_filter postauth POST /* ::revproxy::upstream -target http://localhost:65193 <http://localhost:65193/>
>     }
> }
> ns_section ns/server/default/module/nssock {
>     ns_param port               0
> }
> I have made several attempts to register the /static/* URL to be handled by fastpath and map it to a directory have failed. The server always sends the requests to the revproxy backend.
> Is it possible to "override" this, and if yes then how?
> 
> Best Regards,
> 
>   Georg
> 

[naviserver-devel] Order of URL processing

[Georg L. <jor...@ma...>]

Hello,

I am trying to configure Naviserver as reverse proxy for a Django project.

Django features deployment of static assests like css, js etc. into a 
separate URL, typically /static,  which can then be served directly by 
the webserver.  Everything else has to be reverse-proxied to the wsgi 
server which serves the Django app via http.

My revproxy setup is:

ns_section ns/server/default {
     ns_param enabletclpages     false
     ns_param minthreads         10
}
ns_section ns/server/default/fastpath {
     ns_param pagedir            www
     ns_param serverdir          $serverroot/default
     ns_param directoryproc      ns_returnnotfound
     ns_param directoryfile      index.html
}
ns_section ns/server/default/modules {
     ns_param revproxy           tcl
     ns_param nssock             nssock
}
ns_section ns/server/default/module/revproxy {
     ns_param filters {
         ns_register_filter postauth GET  /* ::revproxy::upstream -targethttp://localhost:65193
         ns_register_filter postauth POST /* ::revproxy::upstream -targethttp://localhost:65193
     }
}
ns_section ns/server/default/module/nssock {
     ns_param port               0
}

I have made several attempts to register the /static/* URL to be handled 
by fastpath and map it to a directory have failed. The server always 
sends the requests to the revproxy backend.

Is it possible to "override" this, and if yes then how?

Best Regards,

   Georg

Re: [naviserver-devel] reverseproxymode with multiple X-Forwarded-For values

[David O. <da...@qc...>]

Thanks again Gustaf.
I've been running some tests and it's looking good.

There's one case I'm not sure this can cater for, and this is probably more
of an issue in development environments...
When the trusted cidr is a subnet which also contains the client.

Say
ns_section ns/parameters/reverseproxymode {
        ns_param enabled        on
        ns_param trustedservers {192.168.0.0/16}
        ns_param skipnonpublic  false
}
And Client IP = 192.168.0.10 with a proxy on 192.168.0.200

I think NaviServer will skip all addresses which appear in the trusted
subnet:

Do you think it would be appropriate to have this behaviour as configurable?

Option 1. Strict.
If the header is from a peer on the trusted list - return only the entry
our trusted peer added (regardless of whether that entry is, in turn, also
in the trusted subnet).
ff {192.168.0.11,192.168.0.10} key 0-1 peer 192.168.0.200 forwarded
{192.168.0.10}

Option 2. Greedy.
If the header is from a peer on the trusted list - return the first
untrusted entry searching from the right
ff {192.168.0.11,192.168.0.10} key 0-1 peer 192.168.0.200 forwarded {}

(I think this is why Nginx introduced the "real_ip_recursive" option on
their real_ip module for instance.)
https://nginx.org/en/docs/http/ngx_http_realip_module.html

Re: [naviserver-devel] refproxy module/ns_connchan: no driver for protocol 'https' found

[Georg L. <jor...@ma...>]

On 2024-05-01 13:56, Gustaf Neumann (sslmail) wrote:
>
>
>> On 30.04.2024, at 20:53, Georg Lehner <jor...@ma...> wrote:
>>
>> Hello,
>>
>> I am trying to set up the revproxy module on a current naviserver git 
>> checkout.
>>
>> Upon connection to the frontend I get:
>>
>>     Request Error
>>     Error during opening connection to backend
>>     http://localhost:65193/ failed.
>>     Error message: no driver for protocol 'http' found
>>
>>     NaviServer/5.0.0a
>>
>
> Probably, you have no nssock module installed. ns_sock uses the 
> network drivers installed, and in case, your configuration network 
> does not include it, it is not available. If you want to use a driver 
> module, but you do not want to listen on a port, configure the port 
> with the value 0
> ...

Thank you, this worked out. In fact I had only configured nsssl on this 
server.

Best Regards,

   Georg

-- 

MagmaSoft e.U. (FN 601643w)
Inhaber: Ing. Georg Lehner
Kolpingstraße 15, A-4020 Linz
https://magma-soft.at/
mailto:jo...@ma...

Re: [naviserver-devel] refproxy module/ns_connchan: no driver for protocol 'https' found

[Gustaf N. (sslmail) <ne...@wu...>]

> On 30.04.2024, at 20:53, Georg Lehner <jor...@ma...> wrote:
> 
> Hello,
> 
> I am trying to set up the revproxy module on a current naviserver git checkout.
> 
> Upon connection to the frontend I get:
> 
> Request Error
> Error during opening connection to backend http://localhost:65193/ failed.
> Error message: no driver for protocol 'http' found
> 
> NaviServer/5.0.0a
> 

Probably, you have no nssock module installed. ns_sock uses the network drivers installed, and in case, your configuration network does not include it, it is not available. If you want to use a driver module, but you do not want to listen on a port, configure the port with the value 0

> The same message I get when starting 'nsd -c' and running either of
> 
>     ns_connchan open http://example.com <http://example.com/> -> no driver for protocol 'http' found
> 
>     ns_connchan open https://example.com <https://example.com/> -> no driver for protocol 'https' found
> 
Same as above. When “nsd -c” is called, no network drivers are loaded

-g

[naviserver-devel] refproxy module/ns_connchan: no driver for protocol 'https' found

[Georg L. <jor...@ma...>]

Hello,

I am trying to set up the revproxy module on a current naviserver git 
checkout.

Upon connection to the frontend I get:

    Request Error
    Error during opening connection to backend http://localhost:65193/
    failed.
    Error message: no driver for protocol 'http' found

    NaviServer/5.0.0a

The same message I get when starting 'nsd -c' and running either of

     ns_connchan open http://example.com -> no driver for protocol 
'http' found

     ns_connchan open https://example.com -> no driver for protocol 
'https' found

I'm puzzled, because some months ago I tested revproxy successfully.

Can you give me hints on how to get going?

Best Regards,

   Georg

Re: [naviserver-devel] reverseproxymode with multiple X-Forwarded-For values

[Gustaf N. (sslmail) <ne...@wu...>]

Dear David & all,

i’ve committed now a version to the main branch of NaviServer on GitHub.
In addition of my last writeup, i’ve further increased the configurability to make ignoring of non-public servers configurable, no matter whether trusted severs are configured or not. To ease configuration, there is now an own section (instead of use ever-growing variable names).


OLD:
    ns_section ns/parameters {
       ...
       ns_param reverseproxymode true
       ...
    }

NEW:
    ns_section ns/parameters/reverseproxymode {
        ns_param enabled        on
        ns_param trustedservers {192.168.0.0/16 137.208.89.213}
        ns_param skipnonpublic  true
    }

The detailed commit message is 
https://github.com/naviserver-project/naviserver/commit/ab23158ece6fcbec4f740a41140592c910de64f3
"x-forwarded-for" reform (part 1) · naviserver-project/naviserver@ab23158
github.com


below are the test-cases, where the key $X-$Y 
- X: stands for skipnonpublic, and
- Y: stands for trustedservers configured (and trusted servers are "192.168.0.0/16 127.0.0.1”)

documentation updates will follow.

all the best
-g



    # just one XFF entry non-trusted (must be client)
    lappend cases {ff {1.1.1.1} key 0-0 peer 1.1.1.1 forwarded 1.1.1.1}
    lappend cases {ff {1.1.1.1} key 0-1 peer 1.1.1.1 forwarded 1.1.1.1}
    lappend cases {ff {1.1.1.1} key 1-0 peer 1.1.1.1 forwarded 1.1.1.1}
    lappend cases {ff {1.1.1.1} key 1-1 peer 1.1.1.1 forwarded 1.1.1.1}

    # just one entry trusted (i.e. must be proxy server)
    lappend cases {ff {192.168.1.10} key 0-0 peer 192.168.1.10 forwarded 192.168.1.10}
    lappend cases {ff {192.168.1.10} key 0-1 peer 127.0.0.1 forwarded {}}
    lappend cases {ff {192.168.1.10} key 1-0 peer 127.0.0.1 forwarded {}}
    lappend cases {ff {192.168.1.10} key 1-1 peer 127.0.0.1 forwarded {}}

    # just one entry local
    lappend cases {ff {127.0.0.3} key 0-0 peer 127.0.0.3 forwarded 127.0.0.3}
    lappend cases {ff {127.0.0.3} key 0-1 peer 127.0.0.3 forwarded 127.0.0.3}
    lappend cases {ff {127.0.0.3} key 1-0 peer 127.0.0.1 forwarded {}}    
    lappend cases {ff {127.0.0.3} key 1-1 peer 127.0.0.1 forwarded {}}

    # two entries, both untrusted
    lappend cases {ff {1.1.1.1, 2.2.2.2} key 0-0 peer 1.1.1.1 forwarded 1.1.1.1}
    lappend cases {ff {1.1.1.1, 2.2.2.2} key 0-1 peer 2.2.2.2 forwarded 2.2.2.2}
    lappend cases {ff {1.1.1.1, 2.2.2.2} key 1-0 peer 1.1.1.1 forwarded 1.1.1.1}
    lappend cases {ff {1.1.1.1, 2.2.2.2} key 1-1 peer 2.2.2.2 forwarded 2.2.2.2}

    # two entries, second trusted
    lappend cases {ff {1.1.1.1, 192.168.1.10} key 0-0 peer 1.1.1.1 forwarded 1.1.1.1}
    lappend cases {ff {1.1.1.1, 192.168.1.10} key 0-1 peer 1.1.1.1 forwarded 1.1.1.1}
    lappend cases {ff {1.1.1.1, 192.168.1.10} key 1-0 peer 1.1.1.1 forwarded 1.1.1.1}
    lappend cases {ff {1.1.1.1, 192.168.1.10} key 1-1 peer 1.1.1.1 forwarded 1.1.1.1}

    # two entries, both trusted
    lappend cases {ff {192.168.1.11, 192.168.1.10} key 0-0 peer 192.168.1.11 forwarded 192.168.1.11}
    lappend cases {ff {192.168.1.11, 192.168.1.10} key 0-1 peer 127.0.0.1 forwarded {}}
    lappend cases {ff {192.168.1.11, 192.168.1.10} key 1-0 peer 127.0.0.1 forwarded {}}
    lappend cases {ff {192.168.1.11, 192.168.1.10} key 1-1 peer 127.0.0.1 forwarded {}}

    # two entries, both local
    lappend cases {ff {127.0.0.2, 127.0.0.3} key 0-0 peer 127.0.0.2 forwarded 127.0.0.2}
    lappend cases {ff {127.0.0.2, 127.0.0.3} key 0-1 peer 127.0.0.3 forwarded 127.0.0.3}
    lappend cases {ff {127.0.0.2, 127.0.0.3} key 1-0 peer 127.0.0.1 forwarded {}}
    lappend cases {ff {127.0.0.2, 127.0.0.3} key 1-1 peer 127.0.0.1 forwarded {}}

    # empty entry
    lappend cases {ff {} key 0-0 peer 127.0.0.1 forwarded {}}
    lappend cases {ff {} key 0-1 peer 127.0.0.1 forwarded {}}
    lappend cases {ff {} key 1-0 peer 127.0.0.1 forwarded {}}
    lappend cases {ff {} key 1-1 peer 127.0.0.1 forwarded {}}

    # wrong entry
    lappend cases {ff {x} key 0-0 peer 127.0.0.1 forwarded {}}
    lappend cases {ff {x} key 0-1 peer 127.0.0.1 forwarded {}}
    lappend cases {ff {x} key 1-0 peer 127.0.0.1 forwarded {}}
    lappend cases {ff {x} key 1-1 peer 127.0.0.1 forwarded {}}

    # wrong entry on the right
    lappend cases {ff {137.208.116.31, x} key 0-0 peer 137.208.116.31 forwarded 137.208.116.31}
    lappend cases {ff {137.208.116.31, x} key 0-1 peer 127.0.0.1 forwarded {}}
    lappend cases {ff {137.208.116.31, x} key 1-0 peer 137.208.116.31 forwarded 137.208.116.31}
    lappend cases {ff {137.208.116.31, x} key 1-1 peer 127.0.0.1 forwarded {}}

    # wrong entry on the left
    lappend cases {ff {y, 137.208.116.31} key 0-0 peer 127.0.0.1 forwarded {}}
    lappend cases {ff {y, 137.208.116.31} key 0-1 peer 137.208.116.31 forwarded 137.208.116.31}
    lappend cases {ff {y, 137.208.116.31} key 1-0 peer 127.0.0.1 forwarded {}}
    lappend cases {ff {y, 137.208.116.31} key 1-1 peer 137.208.116.31 forwarded 137.208.116.31}



> On 29.04.2024, at 10:47, David Osborne <da...@qc...> wrote:
> 
> Hi Gustaf,
> 
> From your description it sounds like we could certainly work round our issue using the ReverseProxyTrustedServers config.
> Thank you very much for your time on this.
> 
> On Fri, 26 Apr 2024 at 14:09, Gustaf Neumann (sslmail) <ne...@wu... <mailto:ne...@wu...>> wrote:
>> Hi David,
>> 
>> I have now implemented the following (but not yet committed,
>> since i was side-tracked by some tcl9 issues and i am running out of
>> time.
>> 
>> From my understanding, this should address your problems now, 
>> and when “proxy 2” is removed.
>> 
>> An easy extension of this would be to let the site-admin configure
>> an alternative header field (like x-real-ip), which could bypass
>> the search through the list of candidate addresses.
>> 
>> all the best
>> -gn
>> 
> 
> _______________________________________________
> naviserver-devel mailing list
> nav...@li...
> https://lists.sourceforge.net/lists/listinfo/naviserver-devel

Re: [naviserver-devel] reverseproxymode with multiple X-Forwarded-For values

[David O. <da...@qc...>]

Hi Gustaf,

>From your description it sounds like we could certainly work round our
issue using the ReverseProxyTrustedServers config.
Thank you very much for your time on this.

On Fri, 26 Apr 2024 at 14:09, Gustaf Neumann (sslmail) <ne...@wu...>
wrote:

> Hi David,
>
> I have now implemented the following (but not yet committed,
> since i was side-tracked by some tcl9 issues and i am running out of
> time.
>
> From my understanding, this should address your problems now,
> and when “proxy 2” is removed.
>
> An easy extension of this would be to let the site-admin configure
> an alternative header field (like x-real-ip), which could bypass
> the search through the list of candidate addresses.
>
> all the best
> -gn
>
>

Re: [naviserver-devel] reverseproxymode with multiple X-Forwarded-For values

[Gustaf N. (sslmail) <ne...@wu...>]

Hi David,

I have now implemented the following (but not yet committed,
since i was side-tracked by some tcl9 issues and i am running out of
time.

From my understanding, this should address your problems now, 
and when “proxy 2” is removed.

An easy extension of this would be to let the site-admin configure
an alternative header field (like x-real-ip), which could bypass
the search through the list of candidate addresses.

all the best
-gn

=========================================================

Traditionally, NaviServer has implemented x-forwarded-for handling in
the simplest way, by treating the first value in the x-forwarded-for
header as the client (see also [1] for a brief description). This
works well for the most part in many production environments.

There are two potential problems with this:

a) If the client is connecting from a private network through
   potentially multiple proxy servers, the private address
   (e.g. 192.168.1.100) may end up as the peer (and also e.g. in the
   access.log), which is not useful for any kind of analysis.

b) If the client itself sends a (faked) "X-Forwarded-For" header, it
   can privide whatever it wants as header value.

One measure is for a reverse proxy to only accept X-Forwarded-For
headers from trusted sources, or to always treat its peer as a client
(providing the peer address in the header field).

This can lead to some loss of information if there are multiple proxy
servers in the chain, and sometimes, it is not possible for an
application to influence the proxy server configuration.

The new version now has two modes, the first being an extension of the
classic mode, and the second being better for untrusted header
content.

The default mode processes the address list from left to right,
skipping private addresses.  If the received "x-forwarded-for header
contains, for example, "192.168.1.100 1.1.1.1 2.2.2.2", then "1.1.1.1"
will be used as the client IP address.

Alternatively, when the NaviServer site admin configures trusted
reverse proxy servers in the configuration file (either by IP address
or in CDIR notation)

    ReverseProxyTrustedServers {192.168.0.0/16 137.208.89.213}

then the "x-forwarded-for" header will only be accepted from trusted
servers (otherwise the header will be ignored). In addition, the
address list is processed from right to left, skipping known trusted
reverse proxies. This reduces the problem of spoofed addresses. In the
address list "142.251.208.110 1.1.1.1 192.168.2.10", the trusted
reverse proxy "192.168.2.10" is skipped and "1.1.1.1" is used (instead
of the spoofed Google IP).

Note: Only when NaviServer receives multiple entries in the
"x-forwarded-for" header is there a difference between these two
modes.

[1] https://en.wikipedia.org/wiki/X-Forwarded-For

Re: [naviserver-devel] FreeBSD 14 - TCL 9.0 - NaviServer GIT Latest - error: incompatible function pointer - log.c

[Not S. <un...@cr...>]

Many thanks Gustaf for creating the patch, I can confirm this works.
I also saw the fix has been applied to TCL core too.

Regards,
David F


-----Original Message-----
From: Gustaf <ne...@wu...>
To: naviserver-devel <nav...@li...>
Date: Monday, 22 April 2024 6:08 PM GMT
Subject: Re: [naviserver-devel] FreeBSD 14 - TCL 9.0 - NaviServer GIT Latest - error: incompatible function pointer - log.c


Dear David,

I installed freebsd14 and could reproduce the issue. The issue is that Tcl_PanicProc is defined in Tcl9 with the attribute declaration TCL_NORETURN1, but it seems, this attribute definition has to be provided as well for the  prototype. It would be certainly better, if this attribute definition would be part of the type declaration of Tcl_PanicProc in Tcl9. Not very long ago, the same code worked fine, so something has changed in Tcl9.

The patch below addresses this issue, but this is not nice. Maybe, the tcl-core people have a suggestion, since this will effect as well other applications requiring the use of Tcl_SetPanicProc().

Please update your checkout of NaviServer from git, since i have fixed one more small issues unrelated to this.

all the best
-g


--- a/nsd/log.c
+++ b/nsd/log.c
@@ -96,7 +96,11 @@ static void LogEntriesFree(void *arg);
  */
 
 static Ns_TlsCleanup FreeCache;
-static Tcl_PanicProc Panic;
+static
+#ifndef NS_TCL_PRE9
+ TCL_NORETURN1
+#endif
+ Tcl_PanicProc Panic;
 
 static Ns_LogFilter LogToFile;
 static Ns_LogFilter LogToTcl;




> On 21.04.2024, at 16:18, Not Spam <un...@cr...> wrote:
> 
> Good afternoon,
> I'm currently rebuilding my machine and I am having some issues with NaviServer (4.99-main #defd765) compiling on FreeBSD 14, with TCL 9.0b2 or TCL 9.0b1
>  
> TCL 9.0b1
> ===
> cc  -O2 -Wall -fPIC  -pipe -finput-charset=UTF-8  -DNDEBUG -DSYSTEM_MALLOC -std=c99 -I../include -I"/usr/local/include"   -DHAVE_CONFIG_H   -c -o log.o log.c
> log.c:262:22: error: incompatible function pointer types passing 'Tcl_PanicProc' (aka 'void (const char *, ...)') to parameter of type 'void (*)(const char *, ...) __attribute__((noreturn))' [-Wincompatible-function-pointer-types]
>     Tcl_SetPanicProc(Panic);
>                      ^~~~~
> /usr/local/include/tcl.h:2366:37: note: passing argument to parameter 'panicProc' here
>                             TCL_NORETURN1 Tcl_PanicProc *panicProc);
>                                                                                      ^
>  
> TCL 9.0b2
> ===
> cc  -O2 -Wall -fPIC  -pipe -finput-charset=UTF-8  -DNDEBUG -DSYSTEM_MALLOC -std=c99 -I../include -I"/usr/local/include"   -DHAVE_CONFIG_H   -c -o log.o log.c
> log.c:262:22: error: incompatible function pointer types passing 'Tcl_PanicProc' (aka 'void (const char *, ...)') to parameter of type 'void (*)(const char *, ...) __attribute__((noreturn))' [-Wincompatible-function-pointer-types]
>     Tcl_SetPanicProc(Panic);
>                      ^~~~~
> /usr/local/include/tcl.h:2366:37: note: passing argument to parameter 'panicProc' here
>                             TCL_NORETURN1 Tcl_PanicProc *panicProc);
>                                                                                      ^
> 1 error generated.
>  
> I'm using the latest GIT repo version.
> Any advice would be nice, I wish to use the ZIPFS that TCL9 brings.
>  
> Regards,
> David F
> _______________________________________________
> naviserver-devel mailing list
> nav...@li...
> https://lists.sourceforge.net/lists/listinfo/naviserver-devel

_______________________________________________
naviserver-devel mailing list
nav...@li...
https://lists.sourceforge.net/lists/listinfo/naviserver-devel

Re: [naviserver-devel] reverseproxymode with multiple X-Forwarded-For values

[David O. <da...@qc...>]

On Wed, 24 Apr 2024 at 18:53, Gustaf Neumann (sslmail) <ne...@wu...>
wrote:

> This is the case where proxy 1 should not accept the x-forwarded-for
> header from an untrusted upstream server. In case, proxy1 is a nginx
> server, it should use
> proxy_set_header X-Forwarded-For $remote_addr;
> for untrusted upstream requests. Therefore, proxy 2 will receive
> x-forwarded-for with 1.1.1.1, 2.2.2.2 etc. and everything is fine.


Agreed - however without a formal standard for X-Forwarded-For, proxies
don't always provide this level of control. In our case, to preserve the
client IP, appending to any existing X-Forwarded-For is the only option
offered. (I have raised this with the suppliers of "proxy1"). I'm not sure
that this is actually wrong behaviour - but definitely unhelpful - yes!


> However, without the massaging in proxy2, you would end up with
>
>     X-Forwarded-For: 1.2.3.4,1.1.1.1,2.2.2.2
>
> where the rightmost address is as well incorrect. So, even by the search
> from the right, one should skip the known proxy servers from the list (here
> 2.2.2.2).  Also, one should aim for a configuration that works also, when
> one more different proxy is added.
>

Yes exactly. When the X-Forwarded-For header is just passed downstream
while appending peer addresses, the backend server must have knowledge of
its networking "situation" to make a judgement.
Possibly a list of the IPs of trusted proxies ("from right-to-left, use the
first IP which isn't a trusted proxy")

set reverseproxytrust [list 2.2.2.2/32 3.3.3.3/32]

Or, the backend would need knowledge of HOW MANY trusted proxies are
upstream. ("from right-to-left, use $count-1 on the list")

set reverseproxytrustedcount 2

This feels brittle though and does not take into account multiple routes
into the backend.

As I mentioned, we *can* write a wrapper to [ns_conn peeraddr -source
forwarded] which would return the value we want by inspecting the headers.
However this would still leave the access log logging the wrong IP - can
this be controlled via config?


> Does your proxy1 set something like the x-real-ip header?
>
>
It does not - I've raised the question with the developers.