joakime
commented
11 months ago Since you are Jetty 10, which UnixSocket implementation in Jetty are you using? (There are 2) The classname you are using should be enough for us.
Open minfrin opened 11 months ago
joakime
commented
11 months ago Since you are Jetty 10, which UnixSocket implementation in Jetty are you using? (There are 2) The classname you are using should be enough for us.
joakime
commented
11 months ago If you are using the Jetty org.eclipse.jetty.unixdomain.server.UnixDomainServerConnector the passed in Path on .setUnixDomainPath(Path) is used with the JVM call ...
java.net.UnixDomainSocketAddress.of(Path)
That means that you are in total control over the Path that you provide to Jetty (eg: its location, permissions, ACL, attributes, etc). Just setup the Path object the way you want and give that object to the above Jetty code, it'll just be passed into the JVM level code as-is, and after that it's totally in the hands of the JVM behaviors.
sbordet
commented
11 months ago @joakime the file is deleted when the connector is stopped, and recreated on start. Would be cumbersome to have a pre-script before starting Jetty to create the file with the proper permissions.
I think it's reasonable to have a setUnixDomainPathPermissions("rw-rw----"), but would it work on Windows/Mac?
joakime
commented
11 months ago The UnixDomain code in the JVM seems to create the Path file (and file descriptor) deep in its code. Our code only deletes it on stop (and badly, need to open a bug about that).
I think it's reasonable to have a
setUnixDomainPathPermissions("rw-rw----"), but would it work on Windows/Mac?
The code to do that would look like this ...
package fs;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.FileAttribute;
import java.nio.file.attribute.PosixFilePermission;
import java.nio.file.attribute.PosixFilePermissions;
import java.util.Set;
public class UnixPermsDemo
{
public static void main(String[] args) throws IOException
{
Path file = Path.of("/tmp/jetty-demo-foo");
Set<PosixFilePermission> perms = PosixFilePermissions.fromString("rw-rw----");
FileAttribute<Set<PosixFilePermission>> attribute = PosixFilePermissions.asFileAttribute(perms);
Files.createFile(file, attribute);
}
}results in
$ ls -la /tmp/jet*
-rw-rw---- 1 joakim joakim 0 Nov 6 16:22 /tmp/jetty-demo-fooIf you execute that in Windows you'll get ...
Exception in thread "main" java.lang.UnsupportedOperationException: 'posix:permissions' not supported as initial attribute
at java.base/sun.nio.fs.WindowsSecurityDescriptor.fromAttribute(WindowsSecurityDescriptor.java:358)
at java.base/sun.nio.fs.WindowsFileSystemProvider.newByteChannel(WindowsFileSystemProvider.java:228)
at java.base/java.nio.file.Files.newByteChannel(Files.java:380)
at java.base/java.nio.file.Files.createFile(Files.java:658)
at fs.UnixPermsDemo.main(UnixPermsDemo.java:18)The windows equivalent would be ...
package fs;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.AclEntry;
import java.nio.file.attribute.AclEntryPermission;
import java.nio.file.attribute.AclEntryType;
import java.nio.file.attribute.AclFileAttributeView;
import java.nio.file.attribute.FileAttribute;
import java.nio.file.attribute.UserPrincipal;
import java.nio.file.attribute.UserPrincipalLookupService;
import java.util.EnumSet;
import java.util.List;
public class WindowsPermsDemo
{
public static void main(String[] args) throws IOException
{
// The file we are going to create
Path file = Path.of("C:\\temp\\jetty-demo-foo");
// First get the UserPrincipal
UserPrincipalLookupService upls = file.getFileSystem().getUserPrincipalLookupService();
UserPrincipal user = upls.lookupPrincipalByName(System.getProperty("user.name"));
// Now lets build the ACL for this file
AclEntry.Builder builder = AclEntry.newBuilder();
builder.setPermissions( EnumSet.of(AclEntryPermission.READ_DATA, AclEntryPermission.EXECUTE,
AclEntryPermission.READ_ACL, AclEntryPermission.READ_ATTRIBUTES, AclEntryPermission.READ_NAMED_ATTRS,
AclEntryPermission.WRITE_ACL, AclEntryPermission.DELETE
));
builder.setPrincipal(user);
builder.setType(AclEntryType.ALLOW);
List<AclEntry> aclEntries = List.of(builder.build());
FileAttribute<List<AclEntry>> attribute = new FileAttribute<List<AclEntry>>() {
@Override
public String name()
{
return "acl:acl";
}
@Override
public List<AclEntry> value()
{
return aclEntries;
}
};
// Create File
Files.createFile(file, attribute);
}
}Our code only deletes it on stop (and badly, need to open a bug about that).
What's wrong?
minfrin
commented
11 months ago The equivalent code in Tomcat is here:
SocketAddress sa = UnixDomainSocketAddress.of(getUnixDomainSocketPath());
serverSock = ServerSocketChannel.open(StandardProtocolFamily.UNIX);
serverSock.bind(sa, getAcceptCount());
if (getUnixDomainSocketPathPermissions() != null) {
Path path = Paths.get(getUnixDomainSocketPath());
Set<PosixFilePermission> permissions =
PosixFilePermissions.fromString(getUnixDomainSocketPathPermissions());
if (path.getFileSystem().supportedFileAttributeViews().contains("posix")) {
FileAttribute<Set<PosixFilePermission>> attrs = PosixFilePermissions.asFileAttribute(permissions);
Files.setAttribute(path, attrs.name(), attrs.value());
} else {
File file = path.toFile();
if (permissions.contains(PosixFilePermission.OTHERS_READ) && !file.setReadable(true, false)) {
log.warn(sm.getString("endpoint.nio.perms.readFail", file.getPath()));
}
if (permissions.contains(PosixFilePermission.OTHERS_WRITE) && !file.setWritable(true, false)) {
log.warn(sm.getString("endpoint.nio.perms.writeFail", file.getPath()));
}
}
}The code takes posix permissions as parameters, and if the filesystem supports posix they are used directly, while if the filesystem does not support posix (windows etc) everyone-read and everyone-write posix permissions are mapped to everyone-read and everyone write for the filesystem.
joakime
commented
11 months ago @minfrin that looks like a class of security bugs where the filesystem is modified after the creation of the file (or directory).
We've had several CVEs filed over the years for this kind of code. (Tomcat too!)
That else branch does nothing on Windows, some local testing shows it just logs, 100% of the time.
At this point, you would probably be better off, security wise, to have your UnixDomainSocketPath in a directory with the correct permissions. Then ask the JVM developers to expose better permission controls on their UnixDomainSocketAddress class.
joakime
commented
11 months ago Our code only deletes it on stop (and badly, need to open a bug about that).
What's wrong?
Our code uses Files.deleteIfExists()
We don't check the exception on use of that, or verify that it was actually deleted. It can easily result in an Exception if the file is still in use (determined by a host of various factors, the JVM, the FileSystem, the OS, etc), or even a silent failure where the file wasn't actually deleted. We don't protect against that, and this is a particularly nasty problem on Windows. (it is quite common for a file that was recently in use, we are denied the ability to delete it until some arbitrary timeout occurs).
In our current code, If someone decided to use UnixDomainServerConnector with a start -> stop -> start flow that file is not guaranteed to be deleted for the next start.
And that file must be deleted before we use the java.net.UnixDomainSocketAddress.of(Path).
We either need to be sure that the file is deleted on close(), and/or we need protection on bindServerSocketChannel() to fail if that file exists (with a clear error message).
I think with the current code, if we fail close() due to the inability to delete, we wind up with a failed stop(), which is undesired, but I would classify this bug as low priority because of this behavior.
minfrin
commented
11 months ago @minfrin that looks like a class of security bugs where the filesystem is modified after the creation of the file (or directory).
We've had several CVEs filed over the years for this kind of code. (Tomcat too!)
Yep, however in this case the socket is created with security restrictions that are too narrow, and the security permissions have to be widened before the socket is in any way useful to anybody.
That else branch does nothing on Windows, some local testing shows it just logs, 100% of the time.
At this point, you would probably be better off, security wise, to have your UnixDomainSocketPath in a directory with the correct permissions. Then ask the JVM developers to expose better permission controls on their UnixDomainSocketAddress class.
The directory with correct permissions is a given:
[root@seawitch ~]# ls -al /run/jenkins/
total 0
drwxrwx---. 2 jenkins jenkins 60 Nov 7 19:25 .
drwxr-xr-x. 64 root root 1560 Nov 7 17:27 ..
srwxrwxr-x. 1 jenkins jenkins 0 Nov 7 19:25 jenkins.socketThe problem is we need an option to allow people other than the owner of the socket (jenkins) to write to the socket (apache).
Right now the workaround is to give the second process the same permissions as the first, making the security separation pointless.
joakime
commented
11 months ago You should be able to use a LifeCycle.Listener to set the permissions yourself.
Example:
UnixDomainServerConnector connector = ...
// other setup of connector here
// setup listener here
connector.addEventListener(new LifeCycle.Listener()
{
@Override
public void lifeCycleStarted(LifeCycle event)
{
if (event instanceof UnixDomainServerConnector unixdomain)
{
Path path = unixdomain.getUnixDomainPath(); // careful, this might be null if you didn't specify a path.
Set<PosixFilePermission> unixperms = PosixFilePermissions.fromString("rw-rw----");
try
{
Files.setPosixFilePermissions(path, unixperms);
}
catch (IOException e)
{
LOG.warn("Unable to set permissions", e);
}
}
}
});Yep, however in this case the socket is created with security restrictions that are too narrow, and the security permissions have to be widened before the socket is in any way useful to anybody.
@minfrin I don't think there is much that we can do (nor the JDK).
I was reading https://man7.org/linux/man-pages/man7/unix.7.html (section "Pathname socket ownership and permissions"), and the socket file permissions are controlled by umask, but in general by OS configuration.
According to that documentation, "The socket file has all permissions enabled, other than those that are turned off by the process umask(2)".
So it seems that it is not Jetty nor the JDK in control of those permissions.
Having Jetty change the permissions after the fact leads to CVEs, and we don't want to do that.
If you want Apache to connect you have at least to share something between the Apache user and the Jenkins user (e.g. same group), and then your umask should do the rest.
What's your umask for the jenkins user? If it's 0002, how is your socket file created with srwxr-xr-x?
Neither Jetty nor the JDK do that, so you must have something else in your system.
FTR, this is the JDK code that performs the bind:
JNIEXPORT void JNICALL
Java_sun_nio_ch_UnixDomainSockets_bind0(JNIEnv *env, jclass clazz, jobject fdo, jbyteArray path)
{
struct sockaddr_un sa;
int sa_len = 0;
int rv = 0;
if (unixSocketAddressToSockaddr(env, path, &sa, &sa_len) != 0)
return;
rv = bind(fdval(env, fdo), (struct sockaddr *)&sa, sa_len);
if (rv != 0) {
handleSocketError(env, errno);
}
}It's just the bind() system call. The callers of this code also do not change the socket file permissions.
minfrin
commented
11 months ago Yep, however in this case the socket is created with security restrictions that are too narrow, and the security permissions have to be widened before the socket is in any way useful to anybody.
@minfrin I don't think there is much that we can do (nor the JDK).
I was reading https://man7.org/linux/man-pages/man7/unix.7.html (section "Pathname socket ownership and permissions"), and the socket file permissions are controlled by
umask, but in general by OS configuration.According to that documentation, "The socket file has all permissions enabled, other than those that are turned off by the process umask(2)".
So it seems that it is not Jetty nor the JDK in control of those permissions.
Correct. The umask represents the defaults on all files created by the application, not just unix domain sockets. It affects directories, other files.
Typical umasks are 0022 (owner full control, everyone else read only), or 0027 (owner full control, group read only, everyone else nothing). You want 0022 or 0027 because you want separation between users.
The purpose of unix domain sockets is to funnel "allowed" data into the application, and funnelling allowed data is another way of saying "write". In other words, in order for any application using unix domain sockets to be useful in any way to anybody, it needs to allow write.
But our umask denied write to everyone, so our only choice is to expand the permissions on the unix domain socket.
Having Jetty change the permissions after the fact leads to CVEs, and we don't want to do that.
Having Jetty expand permissions can never lead to a CVE by definition, as you expanding permission. Why would anyone try and use a race condition to access a socket they're forbidden to access when they can just wait until the race is over and get the access they need?
Expanding permission of the socket is standard practice wherever unix domain sockets are supported.
Here is the mode being set on a unix domain socket in dovecot:
https://www.postfix.org/SASL_README.html
service auth {
3 ...
4 unix_listener /var/spool/postfix/private/auth {
5 mode = 0660
6 # Assuming the default Postfix user and group
7 user = postfix
8 group = postfix
9 }
10 ...
11 }In opendkim:
http://www.opendkim.org/opendkim.conf.5.html
UMask (integer)
Requests a specific permissions mask to be used for file creation. This only really applies to creation of the socket when Socket specifies a UNIX domain socket, and to the PidFile (if any); temporary files are created by the mkstemp(3) function that enforces a specific file mode on creation regardless of the process umask. See umask(2) for more information.If you want Apache to connect you have at least to share something between the Apache user and the Jenkins user (e.g. same group), and then your
umaskshould do the rest.
As explained above that does not work. If I changed the umask (and I have in my case, I have no choice), I am giving apache write access not only to the unix domain socket, but also write access to all the jenkins files. Having done this, the user separation is pointless, I might as well run both jenkins and apache as the same user. Obviously that's bad.
What's your
umaskfor thejenkinsuser? If it's0002, how is your socket file created withsrwxr-xr-x? Neither Jetty nor the JDK do that, so you must have something else in your system.
The default mask for the jenkins user is 0022.
Full explanation here:
https://github.com/jenkinsci/packaging/blob/master/systemd/jenkins.service#L141
Obviously this is pointless, as a read only unix domain socket cannot be written to, which means it cannot accept requests from any other user other than jenkins.
The only way to make this unix domain socket useful is make it writable by at least the group, and that has to be done as an extra step.
FTR, this is the JDK code that performs the
bind:JNIEXPORT void JNICALL Java_sun_nio_ch_UnixDomainSockets_bind0(JNIEnv *env, jclass clazz, jobject fdo, jbyteArray path) { struct sockaddr_un sa; int sa_len = 0; int rv = 0; if (unixSocketAddressToSockaddr(env, path, &sa, &sa_len) != 0) return; rv = bind(fdval(env, fdo), (struct sockaddr *)&sa, sa_len); if (rv != 0) { handleSocketError(env, errno); } }It's just the
bind()system call. The callers of this code also do not change the socket file permissions.
Most software out there allows full specification of umask, because it is understood you are only ever doing this to expand access, not restrict access.
If this is still a concern, have an option like this:
unixDomainSocketAllow=group - expand group read/write permission to the socket after creation (will fail on Windows, but Windows can do the option below and protect the directory). unixDomainSocketAllow=everyone - expand group and others read/write permission to the socket after creation.
By definition, the above is safe.
sbordet
commented
11 months ago @joakime @gregw @minfrin so the proposal can be the following:
ServerSocketChannel and get the socket file.PosixFilePermissions of the socket file; if we get a failure (e.g. Windows), bail out.PosixFilePermission.GROUP_WRITE is there; if so, nothing else to do.PosixFilePermission.GROUP_WRITE to the permissions; if we get a failure (e.g. Windows), bail out.In this way we would be only enlarging 1 permission (GROUP_WRITE), so no CVE risk. Perhaps we need to verify that we also have GROUP_READ.
About expanding to OTHERS, I'll be cautious for now and not do it unless there is a precise use case that cannot be done with GROUP.
Would that work?
joakime
commented
11 months ago @sbordet your proposal is almost the same as tomcats' impl, but they use OTHER, not GROUP. I would rather see the ability to act on the Path post-bind in code as a solution (similar to the LifeCycle.Listener solution above). Let the user decide what they want to do in code, be it unix or windows or something else.
eg:
UnixDomainServerConnector myUnixDomainServerConnector = new UnixDomainServerConnector() {
public void bindServerSocketChannel() {
super.bindServerSocketChannel();
Path path = unixdomain.getUnixDomainPath(); // careful, this might be null if you didn't specify a path.
Set<PosixFilePermission> unixperms = PosixFilePermissions.fromString("rw-rw----");
try {
Files.setPosixFilePermissions(path, unixperms);
} catch (IOException e) {
LOG.warn("Unable to set permissions", e);
}
}
}@joakime but seems a common enough need that overriding the connector would be too cumbersome.
Jetty version(s)
Jetty 10+
Jetty Environment
Java version/vendor
(use: java -version)openjdk version "17.0.9" 2023-10-17 LTS OpenJDK Runtime Environment (Red_Hat-17.0.9.0.9-1) (build 17.0.9+9-LTS) OpenJDK 64-Bit Server VM (Red_Hat-17.0.9.0.9-1) (build 17.0.9+9-LTS, mixed mode, sharing)
OS type/version
RHEL9
Description
When a unix domain socket is created, the socket is created given the umask of the server. This in almost all cases creates a socket that can only be connected to by the same user that runs jetty.
This breaks the privilege separation between components.
The workaround is to change the umask to 0002 or 0007, but this has the side effect that it gives full application write access to the entity connecting to the socket.
The permissions on the socket need to be configurable. No need to overthink the difference between unix and windows, if "allow anyone to connect" is allowed, the admin is given the option to put the sock in a directory protected as they need it.
How to reproduce?
Create a jenkins instance with unix domain socket support. See the permissions on the socket.