[GitHub] [tomcat] minfrin opened a new pull request #382: Add support for unix domain sockets.

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
92 messages Options
12345
Reply | Threaded
Open this post in threaded view
|

[GitHub] [tomcat] rmaucher commented on pull request #382: Add support for unix domain sockets.

GitBox

rmaucher commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-747467253


   I am not convinced about adding that feature to the APR endpoint ...
   
   Anyway:
   - The changes to IntrospectionUtils are too much given the actual use, strings could be used and the endpoint is the only place that actually deals with the two types so it seems enough
   - I don't get the idea behind the "permissions", since I don't think Tomcat is the party that is supposed to be creating the socket
   
   The UDS feature should already work with NIO and Java 16 EA by using an inherited channel. The limitation is that there is only one endpoint that can use a UDS. I'm ok with adding full UDS support to NIO using the compat package (the amount of reflection needed does not seem too bad so I may try it to see how that would work).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] [tomcat] rmaucher commented on pull request #382: Add support for unix domain sockets.

GitBox
In reply to this post by GitBox

rmaucher commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-750141172


   I added the feature for NIO, since it wasn't too difficult using https://openjdk.java.net/jeps/380 . Testing with curl works fine, I'll add a test in the testsuite next. It does have specific unlock accept, some compatibility code for port/address annoyances ("TCP local addresses" for access logs, JMX name, connector name), and the port attribute becomes optional. This cannot be added to NIO2 since the feature is not available.
   
   Some possible changes:
   - The permission attribute, is it really useful ?
   - Reflection is used for this attribute for now since this is NIO only
   - The socket is not deleted on shutdown (although the channel is closed)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] [tomcat] michael-o commented on pull request #382: Add support for unix domain sockets.

GitBox
In reply to this post by GitBox

michael-o commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-750148235


   I see no reason why this cannot work which Java UDS and APR UDS.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] [tomcat] martin-g commented on pull request #382: Add support for unix domain sockets.

GitBox
In reply to this post by GitBox

martin-g commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-750260041


   @rmaucher https://github.com/apache/tomcat/commit/884b997f5a9a7da9f696d00574d3b727afbfae8c#diff-117ff4ae372c7a4f6643546174bcc2dbf5a25bd399fe1b89f55e72d2d4150285R212


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] [tomcat] rmaucher commented on pull request #382: Add support for unix domain sockets.

GitBox
In reply to this post by GitBox

rmaucher commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-750274850


   It can 100% work with APR, except I personally don't want to add features to that component at this point.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] [tomcat] michael-o commented on pull request #382: Add support for unix domain sockets.

GitBox
In reply to this post by GitBox

michael-o commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-750490202


   >
   >
   > It can 100% work with APR, except I personally don't want to add features to that component at this point.
   
   If you personally don't want to, @minfrin happily will.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] [tomcat] minfrin commented on pull request #382: Add support for unix domain sockets.

GitBox
In reply to this post by GitBox

minfrin commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-759497608


   > * The permission attribute, is it really useful ?
   
   In the absence of a permission attribute (and without the "everyone" default), the socket is equivalent to a TCP port that has been firewalled off, and thus pointless.
   
   Ignoring special cases like a personal development environment, or a system with no user separation, daemons (like tomcat) are secured with a user tomcat, group tomcat, and a typical umask of 0750 (or some variation). This means that the a) the tomcat user can write, b) the tomcat group can read (typically allowing read access to log files), and c) everyone else get nothing.
   
   In order for any unix domain socket to be of use to anyone, it must be possible to write to it. If you can't write to it, you cannot submit a request. A unix domain socket that only the tomcat user can write to pointless, as you've giving the client control over the tomcat process. A read only unix domain socket for a request/response protocol like HTTP has no practical effect - having written nothing you will read nothing.
   
   For this reason, every daemon out there that I have seen has a mechanism to make the socket writable to a group, and defaulting to being accessible to everyone:
   
   https://github.com/Cisco-Talos/clamav-devel/blob/31824a659dff37ae03e3419395bb68e659c2b165/etc/clamd.conf.sample#L104
   
   https://github.com/trusteddomainproject/OpenDMARC/blob/b0d6408d0859adb336428e3d0bd87749513a9e79/opendmarc/opendmarc.conf.sample#L357
   
   https://github.com/rspamd/rspamd/blob/9c2d72c6eba3fc05fd7459e388ea7c92eb87095f/conf/options.inc#L48
   
   In the absence of an explicit control over permissions, making the permissions world writable by default allows the admin to secure the socket by restricting permissions on the parent directory, such as the following example:
   
   ```
   [root@localhost clamav-milter]# ls -al
   total 0
   drwx--x---.  2 clamilt clamilt   60 Jan 11 13:03 .
   drwxr-xr-x. 39 root    root    1080 Jan 11 13:06 ..
   srw-rw-rw-.  1 clamilt clamilt    0 Jan 11 13:03 clamav-milter.socket
   ```
   
   In the above, the socket itself is world writable, but the parent directory is protected, and therefore the socket is protected.
   
   > * The socket is not deleted on shutdown (although the channel is closed)
   
   If the socket is not deleted on shutdown, the server cannot subsequently be started up. Deleting the socket on shutdown is the most common behaviour. Deleting the socket is startup is not done, as it means that multiple daemons can be started without error.
   
   I think I saw a commit go past fixing this, need to verify.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] [tomcat] minfrin closed pull request #382: Add support for unix domain sockets.

GitBox
In reply to this post by GitBox

minfrin closed pull request #382:
URL: https://github.com/apache/tomcat/pull/382


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] [tomcat] minfrin commented on pull request #382: Add support for unix domain sockets.

GitBox
In reply to this post by GitBox

minfrin commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-761070369


   Life is too short to fight with git.
   
   Opened a fresh PR at https://github.com/apache/tomcat/pull/401.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] [tomcat] minfrin commented on pull request #382: Add support for unix domain sockets.

GitBox
In reply to this post by GitBox

minfrin commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-761071502


   > @minfrin Do you want to peform anymore changes or do want to me run verifcation on it? Do you think a test would be possible to start up and shut down a UDS?
   
   I've added a test based on the existing APR tests. Can you confirm this is all ok?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] [tomcat] minfrin commented on pull request #382: Add support for unix domain sockets.

GitBox
In reply to this post by GitBox

minfrin commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-761110354


   > I added the feature for NIO, since it wasn't too difficult using https://openjdk.java.net/jeps/380 .
   
   Thank you for doing this, it is a huge help.
   
   This patch is all about supporting people who are not yet in a position to use Java 16.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] [tomcat] rmaucher commented on pull request #382: Add support for unix domain sockets.

GitBox
In reply to this post by GitBox

rmaucher commented on pull request #382:
URL: https://github.com/apache/tomcat/pull/382#issuecomment-761238160


   > > * The permission attribute, is it really useful ?
   >
   > In the absence of a permission attribute (and without the "everyone" default), the socket is equivalent to a TCP port that has been firewalled off, and thus pointless.
   >
   > Ignoring special cases like a personal development environment, or a system with no user separation, daemons (like tomcat) are secured with a user tomcat, group tomcat, and a typical umask of 0750 (or some variation). This means that the a) the tomcat user can write, b) the tomcat group can read (typically allowing read access to log files), and c) everyone else get nothing.
   >
   > In order for any unix domain socket to be of use to anyone, it must be possible to write to it. If you can't write to it, you cannot submit a request. A unix domain socket that only the tomcat user can write to pointless, as you've giving the client control over the tomcat process. A read only unix domain socket for a request/response protocol like HTTP has no practical effect - having written nothing you will read nothing.
   >
   > For this reason, every daemon out there that I have seen has a mechanism to make the socket writable to a group, and defaulting to being accessible to everyone:
   >
   > https://github.com/Cisco-Talos/clamav-devel/blob/31824a659dff37ae03e3419395bb68e659c2b165/etc/clamd.conf.sample#L104
   >
   > https://github.com/trusteddomainproject/OpenDMARC/blob/b0d6408d0859adb336428e3d0bd87749513a9e79/opendmarc/opendmarc.conf.sample#L357
   >
   > https://github.com/rspamd/rspamd/blob/9c2d72c6eba3fc05fd7459e388ea7c92eb87095f/conf/options.inc#L48
   >
   > In the absence of an explicit control over permissions, making the permissions world writable by default allows the admin to secure the socket by restricting permissions on the parent directory, such as the following example:
   >
   > ```
   > [root@localhost clamav-milter]# ls -al
   > total 0
   > drwx--x---.  2 clamilt clamilt   60 Jan 11 13:03 .
   > drwxr-xr-x. 39 root    root    1080 Jan 11 13:06 ..
   > srw-rw-rw-.  1 clamilt clamilt    0 Jan 11 13:03 clamav-milter.socket
   > ```
   >
   > In the above, the socket itself is world writable, but the parent directory is protected, and therefore the socket is protected.
   >
   > > * The socket is not deleted on shutdown (although the channel is closed)
   >
   > If the socket is not deleted on shutdown, the server cannot subsequently be started up. Deleting the socket on shutdown is the most common behaviour. Deleting the socket is startup is not done, as it means that multiple daemons can be started without error.
   >
   > I think I saw a commit go past fixing this, need to verify.
   
   Ok, I still dislike the permission attribute quite a bit but I can understand things can be annoying without it in some cases.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[hidden email]



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

12345