JDK-8213082 : (zipfs) Add support for POSIX file permissions
  • Type: CSR
  • Component: core-libs
  • Sub-Component: java.nio
  • Priority: P3
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 14
  • Submitted: 2018-10-29
  • Updated: 2019-06-28
  • Resolved: 2019-06-14
Related Reports
CSR :  
Description
Summary
-------

Support POSIX file permissions in OpenJDK's Zip File System (Module `jdk.zipfs`, Provider class `jdk.nio.zipfs.ZipFileSystemProvider`).

Problem
-------

POSIX file systems define a certain set of permission attributes for files (e.g. read, write, execute for user, group or all). Being able to store POSIX permission information in Zip files and access it with APIs/tools is an important capability that users of Zip files are in need of on POSIX systems.

Although there is no explicit definition in the Zip specification about the canonical way to store these attributes, there exists a common sense amongst implementors of Zip tooling serving as a de-facto standard, leveraging Zip's CEN header fields `version made by` and `external file attributes`.

The OpenJDK did not support POSIX file permissions in Zip files so far. The lack of this basic functionality leads to avoidable dependencies to 3rd party libraries and tools.

Solution
--------

The OpenJDK has a (java.nio.file.spi) FileSystemProvider that handles Zip files. It is implemented in class `jdk.nio.zipfs.ZipFileSystemProvider` of module jdk.zipfs. It handles file URIs with the URI scheme `jar`.

Furthermore, java.nio already provides abstraction for POSIX attributes. For instance there exists a class `java.nio.file.attribute.PosixFilePermission` encapsulating POSIX file permissions as file system attributes. There is also a file attribute view named `java.nio.file.attribute.PosixFileAttributeView` and a utility class `java.nio.file.attribute.PosixFilePermissions`.

It feels natural to enhance the ZipFileSystemProvider to support POSIX file attributes. That is, getting and setting the attributes and store/read them in Zip files.

In a Zip archive, the permission information is persisted by leverating CEN header fields `version made by` and `external file attributes`. The relevant sections of the Zip File Format Specification - https://pkware.cachefly.net/webdocs/casestudies/APPNOTE.TXT - are "4.4.2 version made by (2 bytes)" and "4.4.15 external file attributes: (4 bytes)". A coding example, though written in C, can be found in the infozip implementation which is available as Open Source: http://infozip.sourceforge.net/.

When POSIX permissions are to be stored on a Zip entry, the upper byte of CEN field "version made by" will be set to 3 - meaning external file attribute compatibility of type "UNIX". The permissions are stored as flag values in the upper 2 bytes of field "external file attributes". Upon reading a Zip entry, if "UNIX" is found as external file attribute compatibility value in "version made by", POSIX permissions will be restored from the upper 2 bytes of field "external file attributes".

Additional considerations

 - Performance: Supporting POSIX permissions comes with the small cost of having to read/write the CEN header field `external attributes`. But this should be rather negligible.
 - Security: There should not be a security issue with this new feature. There has been a concern regarding JAR signing where the signature would not include the header fields storing the permissons. However, JAR signing has never been supported by the zipfs provider. Furthermore, copying of files between different file systems (e.g. from Zip Filesystem to UnixFileSystem) won't tranfer permission bits by default since the default inter-provider implementation of Files.copy only copies basic attributes.
 - Other: To use the methods in `java.nio.file.Files` that deal with POSIX permissions, the Zip file system needs to support classes `PosixFileAttributes` as well as `PosixFileAttributeView`. These offer methods that deal with owner and group information whose support is out of scope of this change. Also the POSIX permission information is not optional in these classes which is problematic since it is optional in Zip files. So a certain set of defaults needs to be defined.

Specification
-------------

The following text is added to the `jdk.zipfs` module-info file:

> A Zip file system supports a file attribute view named "zip" that
> defines an attribute named "permissions" of type
> `Set<PosixFilePermission>`. The "permissions" attribute is the set of
> access permissions that are optionally stored for entries in a Zip
> file. The value of the attribute is null for entries that do not have
> access permissions. Zip file systems do not enforce access
> permissions.
> 
> The "permissions" attribute may be read and set using the
> Files.getAttribute and Files.setAttribute methods. The following
> example uses these methods to read and set the attribute:
> 
>  
>      Set<PosixFilePermission> perms = Files.getAttribute(entry, "zip:permissions");
>      if (perms == null) {
>          perms = PosixFilePermissions.fromString("rw-rw-rw-");
>          Files.setAttribute(entry, "zip:permissions", perms);
>      }
>
> In addition to the "zip" view, a Zip file system optionally supports the
> `PosixFileAttributeView` ("posix"). This view extends the
> "basic" view with type safe access to the owner, group-owner, and
> permissions attributes. The "posix" view is only supported when the
> Zip file system is created with the provider property
> "enablePosixFileAttributes" set to "true". The following creates a
> file system with this property and reads the access permissions of a
> file:
> 
>  
>      var env = Map.of("enablePosixFileAttributes", "true");
>      try (FileSystem fs = FileSystems.newFileSystem(file, env) {
>          Path entry = fs.getPath("entry");
>          Set<PosixFilePermission> perms = Files.getPosixFilePermissions(entry);
>      }
> 
> The file owner and group owner attributes are not persisted, meaning
> they are not stored in the zip file. The provider properties
> "defaultOwner" and "defaultGroup" can be used to configure the default
> values for these attributes. If these properties are not set then the
> file owner defaults to the owner of the zip file, and the group owner
> defaults to the zip file's group owner (or the file owner on platforms
> that don't support a group owner).
> 
> The "permissions" attribute is not optional in the "posix" view so a
> default set of permissions are used for entries that do not have
> access permissions stored in the Zip file. The default set of
> permissions are `OWNER_READ`, `OWNER_WRITE` and `GROUP_READ`. The
> default permissions can be configured with the provider property
> "defaultPermissions".
> 
> The following provider properties for Zip file systems will be added:
> 
>  - enablePosixFileAttributes: If the value is true, the Zip file system will support the PosixFileAttributeView.  
>     Data Type: java.lang.String, Default Value: false
>  - defaultOwner: Override the default owner for entries in the Zip file system. The value can be a UserPrincipal or a String value that
> is used as the UserPrincipal's name.  
>     Data Type: UserPrincipal or java.lang.String, Default Value: null/unset
>  - defaultGroup: Override the the default group for entries in the Zip file system. The value can be a GroupPrincipal or a String value that
> is used as the GroupPrincipal's name.	 
>     Data Type: GroupPrincipal or java.lang.String, Default Value: null/unset
>  - defaultPermissions: Override the default Set of permissions for entries in the Zip file system. The value can be a
> Set<PosixFilePermission> or a String that is parsed by
> PosixFilePermissions::fromString  
>     Data Type: Set<PosixFilePermission> or java.lang.String, Default Value: null/unset


Comments
Thanks for approving. As the feature came to late for JDK13, I have set Fix version to 14. Please correct me if that was a wrong step.
28-06-2019

Catching up on CSRs after an influx of reviews, moving to Approved.
14-06-2019

After further progress on both, the specification and the implementation, I'm moving this one to finalized for CSR review.
05-06-2019

Hi Alan, I replaced the specification section with a wording proposal which shall also be amended to the jdk.zipfs module documentation. I also removed the attached patch, which was a bit outdated anyway. Please see if the CSR can get your review now or let me know what I shall adapt further. Thanks Christoph
07-01-2019

The original proposal was part of a larger proposal that had several security concerns. Removing java.util.zip and the `jar` tool from the proposal makes this much easier to discuss. jdk.zipfs/module-info.java is the place to document support for file attributes beyond the basic file attributes that all file system providers are required to support. So for the CSR then I think we can prune it of most of the implementation details, patch etc and replace it with the proposal changes to the module description.
28-12-2018

In addition to Volker, please have libs and security engineers who have commented on previous versions of this request, Alan, Sean, etc. review the proposal. As a procedural point, I'm marking this request as pended until those reviews are done.
22-12-2018

I've thoroughly looked at this proposal and honestly speaking, I don't think that we even need a CSR for the proposed enhancement. Only the implementation of some classes in the `jdk.nio.zipfs` package are changed but no publicly exported API's. As Christoph emphasized, the proposed changes **only** affect the Zip File System as implemented in the `jdk.nio.zipfs` package. It doesn't touch neither the implementation nor the behavior of any of the classes in the `java.util.zip` or `java.util.jar` packages. Previous reviewers of this CSR and of the corresponding change JDK-8213031 raised some concerns regarding the security implication of this change - especially in the context of signed jars. I've looked at these concerns but I couldn't find any evidence for problems because: - no exposed JAR functionality is affected by these changes (because that is handled by the implementation in `java.util.zip` and `java.util.jar`) - Jar signing doesn't take file attributes into account (even not the basic attributes like the modification time). It only hashes the file contents. You can already now use external zip tools to update the attributes of files in a signed archive without affecting the validity of the signature. For all these reasons I've reviewed this CSR and I kindly ask you to approve it.
21-12-2018

The overall "feature" require more discussion and agreement on whether it is the right thing to do or not. The implications for signed JARs is a significant discussion point, there are also several points to discuss around tooling that may capture and propagate file permissions. As regards the zipfs piece then the changes do change the supported interface. The jdk.zipfs module description may be the place to document the attributes that are supported.
30-10-2018

Christoph, given the discussions in this area, please have one or more engineers who have worked on Zip and have contributed to the discussions, like Sherman or Alan, review this request before proceeding further. Having a security engineer as a reviewer would be beneficial as well. It would be helpful for the purposes of CSR for the spec changes to be called out explicitly to aid the discussion. From looking over the patch, there is no explicit API change; is that correct? Also, please confirm that this patch only seeks to cover basic unix-style permissions (rwx bits) and not any particular ACL scheme. Moving the request to Pended. Please re-Propose once the security review is complete and the requested parties have reviewed the current proposal.
29-10-2018