Details

Description

It turns out that the file owner does not necessarily get an explicit AclEntry; this depends on whether the created file has sufficient permissions already through, say, a permission for everybody to write. The present logic removes all AclEntries except those granted to the file's owner, erroneously presuming there would be such an entry always. This led to all AclEntries being removed.

This error is seen in Oracle's nightly regressions for Windows, but did not reproduced when running manually on Windows. This was due to different default inherited permissions on the directories in which the regression tests were run.

Construct a new AclEntry for the owner with all rights, and removed existing ones (NTFS). This should handle the error seen in Oracle's regressions.

For Solaris/ZFS and similar file systems which support both Posix file attributes view and ACLs, don't touch the ACLs but stick to the Posix flags.

For the latter my rationale is as follows: Principle of least surprise: most users never touch the ACLs but use the more familiar Posix file masks. It turned out the existing Derby implementation, although protecting the file adequately, showed a "+" in the ls(1) listing indicating that the settings could not be directly mapped onto the Posix model. The reason was that we removed more permissions than the plain read,write, and execute. Since ZFS internally builds on ACLs, the ls(1) listing would should that Derby had been tinkering with the non-mappable ACL permissions. I think it is better to stick to the Posix permissions by default. If people are using ACL functionality they are likely more than average concerned with security and can run with default file permissions and take full responsibility of the permissions fo created filed. Rationale end.

Dag H. Wanvik
added a comment - 07/Nov/11 10:34 - edited Uploading a patch which makes two changes:
Construct a new AclEntry for the owner with all rights, and removed existing ones (NTFS). This should handle the error seen in Oracle's regressions.
For Solaris/ZFS and similar file systems which support both Posix file attributes view and ACLs, don't touch the ACLs but stick to the Posix flags.
For the latter my rationale is as follows: Principle of least surprise: most users never touch the ACLs but use the more familiar Posix file masks. It turned out the existing Derby implementation, although protecting the file adequately, showed a "+" in the ls(1) listing indicating that the settings could not be directly mapped onto the Posix model. The reason was that we removed more permissions than the plain read,write, and execute. Since ZFS internally builds on ACLs, the ls(1) listing would should that Derby had been tinkering with the non-mappable ACL permissions. I think it is better to stick to the Posix permissions by default. If people are using ACL functionality they are likely more than average concerned with security and can run with default file permissions and take full responsibility of the permissions fo created filed. Rationale end.
Rerunning regressions of NTFS, Solaris/ZFS and Linux.

I ran a quick test on the Windows machine where we saw the failures in the nightly tests. I ran the derbynet test suite and verified that it failed with permission problems without the patch and passed with the patch.

Tiny nit: The patch adds an import for the Monitor class, but it doesn't use it. The rest of the changes look good to me.

Knut Anders Hatlen
added a comment - 08/Nov/11 12:47 I ran a quick test on the Windows machine where we saw the failures in the nightly tests. I ran the derbynet test suite and verified that it failed with permission problems without the patch and passed with the patch.
Tiny nit: The patch adds an import for the Monitor class, but it doesn't use it. The rest of the changes look good to me.