Issue 96918

Summary: ACL information not being read.
Product: General Reporter: sirloxelroy <sirloxelroy>
Component: codeAssignee: thorsten.martens
Status: CLOSED FIXED QA Contact: issues@framework <issues>
Severity: Trivial    
Priority: P3 CC: chris, issues, kpalagin, mail.twerner, Mathias_Bauer, noel, tim
Version: OOO300m9   
Target Milestone: OOo 3.2   
Hardware: All   
OS: All   
Issue Type: DEFECT Latest Confirmation in: ---
Developer Difficulty: ---
Attachments:
Description Flags
Patch to restore the old behaviour when opening files with ACLs or remote files none

Description sirloxelroy 2008-12-04 22:25:11 UTC
Setup: Mac OS X 10.5.5 Leopard Server, Open Directory Authentication, AFP Shared
drives with ACL enabled and used, OpenOffice.org 3 m9 Build 9358 installed on
the workstations (OS X 10.5.5 there also).  Users home directories mounted from
server.  

Problem: Files are opening Read Only for OpenOffice.org 3 even though the ACL's
say that the file is RW.  Tried other programs such as Text Editor that comes
with Mac and can read odt files, it sees the files as RW.  THe files are
assigned the attributes when created of 0750, with ACL attributes of the
ahb_staff2 group have RW functions, and everyone is a member of the ahb_staff2
group. 

Observations: From what I can gather so far it seems that OpenOffice.org is not
seeing the ACLs, or looking at them correctly???  I am not posotive how OO opens
a file and would like to attempt to reproduce the opening so I can see where the
problem is.
Comment 1 sirloxelroy 2008-12-07 05:37:54 UTC
UPDATE:  I tested the same files with Neo-Office (http://www.neooffice.org/) and
they open up in RW mode.
Comment 2 sirloxelroy 2008-12-12 17:46:22 UTC
I am surprised that nothing has been done on this.  If OpenOffice wants to
become a player in Mac Office Environments someone should at least comment that
they are going to look into this.  
Comment 3 florian 2009-01-02 14:24:08 UTC
I' ll have a look into this.
Comment 4 florian 2009-01-03 00:26:31 UTC
Issue is confirmed. I tested with a file with permissions set to standard 0644 with owner fheckl:wheel (just 
saved the file into /Users/Shared). I set ACL permissions with chmod +a "staff allow read,write" to allow all 
users to write to this file.
With a different user(tester:staff) account I got the following:
- TextEdit let's you edit the file, but you cannot save actually (write gives an error)
- OOo opens the file in read-only mode
- NeoOffice (tested with version 2.2.4) let's you edit the file and correctly save any updates (write gives no 
error).
Will add more tests over the weekend.
Comment 5 sirloxelroy 2009-02-09 16:25:20 UTC
This was an Issue with NeoOffice 3.0 Early Access 2.0.  It turned out to be a
core OO issue which Plube has written a patch for (Bug #3406).  I just thought I
would let you all know.  Also, I am sorry if I sounded a little scathing about
the issue, I should have waited for a little while after posting the comment.
Comment 6 bollin 2009-03-16 09:07:57 UTC
This is a general issue and the same thing happens with OOorg 3 and Linux and it
is a major regression from OOorg 2.x. Our users can no longer write shared files
on our filesystem that can be written by other applications including OOorg 2.x.
The bug #31890 describes a similar problem.

BTW there is no patch in Bug #3406 as the former poster indicated.
Comment 7 chris 2009-04-03 17:19:17 UTC
This is not a Mac only issue. I have reproduced it by using extended ACLs, and I
am told it can be reproduced via Samba too. 

The problem occurs in any place that the simple detection heuristic for a read
only file in osl_getFileStatus fails. This is a result of the change in
mediadescriptor.cxx (Revision 1.17.78.1) that was made as part of Issue 85794.

Before the change, OOo would try to open the file for writing to decide if the
file could be opened for editing or not. Now, it will not try to open the file
for writing if locking is off, and the heuristic will be used instead.

haggai->mav: I am assigning this to you for your comments about this change.

I will attach a patch which solves the immediate problem. I have not yet
verified if mav's locking changes are all working as intended.
Comment 8 chris 2009-04-03 17:21:24 UTC
haggai->mav: Assigning to you. Please see my last comment.
Comment 9 chris 2009-04-03 17:22:47 UTC
Created attachment 61384 [details]
Patch to restore the old behaviour when opening files with ACLs or remote files
Comment 10 mikhail.voytenko 2009-04-14 07:38:01 UTC
The OOo3.1 version should work well in this scenario in default configuration,
could somebody please try the OOo3.1 release candidate?

mav->haggai: Thank you for the investigation. Sorry, but the attached patch is
just a hack that would break the behavior in case system file locking is turned
off. 

The real problem, as you already have already written, is in
implementation/usage of osl_getFileStatus() call. This problem should be fixed
even if the default configuration works well in OOo3.1, since it is possible to
turn the system file locking off, and then we would have just the same problem.
Comment 11 noelkoethe 2009-04-15 13:52:40 UTC
I tested this problem with OOo 3.1 rc1 (Build 9396) (on Debian Linux) and the
problem is still there.

I'm user "nk" and the file permissions (extended ACLs) are:

# getfacl testACL.odt 
# file: testACL.odt
# owner: root
# group: root
user::rw-
user:nk:rw-
group::r--
mask::rw-
other::r--

OOo 3.1 rc1 opens this file - where I have write permission - read-only.
Comment 12 mikhail.voytenko 2009-04-15 14:19:25 UTC
What I mean is that if it does not work with the 3.1 rc1 it would not also work
with the attached patch. The 3.1 in default configuration does exactly the same.
Anyway, the detection of the file readonly state has to be fixed.

mav->noelkoethe: Is it possible to create a new file on the location where the
target file is? It not, it is probably the problem, OOo can not create the lock
file.

Comment 13 mikhail.voytenko 2009-04-15 14:27:41 UTC
Sorry, a typo. I mean of course "...If not, it is probably the problem, OOo can
not create the lock file."
Comment 14 noelkoethe 2009-04-15 15:55:32 UTC
The directory is /tmp/ and everyone can write into the directory.

If I'm the owner (or have writeaccess via group) the lockfile is created:

-rw-r--r--   1 nk   root     76 15. Apr 16:51 .~lock.testACL.odt#
-rw-rw-r--+  1 root nk     7461 15. Apr 14:46 testACL.odt

and if I have write access via the extended ACLs no lockfile will be created.
The lockfile is only created if OOo detects the document as writeable which
makes sense.:)
Comment 15 chris 2009-04-15 17:21:09 UTC
haggai->mav:

> mav->haggai: Thank you for the investigation. Sorry, but the attached patch is
> just a hack that would break the behavior in case system file locking is turned
> off. 

Okay, we hadn't run through the full set of tests yet to determine if anything
else had broken as a result of the patch. That was why I posted the patch for
your comments before going further :)

> The real problem, as you already have already written, is in
> implementation/usage of osl_getFileStatus() call. This problem should be fixed
> even if the default configuration works well in OOo3.1, since it is 
> possible to turn the system file locking off, and then we would have just 
> the same problem.

I don't think you can rely on osl_getFileStatus, instead of opening the file as
writable. You'll always have potential race conditions, where the state of the
file changes between the calls. Or are you thinking of using osl_getFileStatus
to see if you could open it as writable, then handle an error in the race
condition case if you then find it is no longer writeable?
Comment 16 mikhail.voytenko 2009-04-16 07:54:53 UTC
mav->noelkoethe:  You are right. Sorry, I have forgot about the last change I
did there. Indeed, the file readonly state is checked always now, so the patch
would not make behavior better in the latest version.

mav->haggai:

The fact that the patch can not be integrated as it is does not reduce the value
of the investigations. :)

As for the race condition, actually the read only mode of OOo is a designed race
condition itself, since the document file state can be changed while having the
document open. 

The only possible problem while using the attributes, as you have correctly
pointed, is that the document gets readonly state while being opened/edited. The
OOo implementation should be ready for it and handle it accordingly - either the
file would be opened readonly ( if it happens between the calls as you have
mentioned ) or an error would be shown on saving ( if it happens during the
editing ).

Thus I still tend to think that the correct approach would be to fix the
attributes handling. 
Comment 17 diocles 2009-04-20 15:25:29 UTC
[I've been looking at this problem with haggai.]

diocles->mav: Fixing the attribute handling in osl_getFileStatus is /just about/
possible, but ugly... I really do not think we want to reimplement the OS's
POSIX ACL checks in userspace - it is likely to be complicated and prone to
error, and will still not take account of such things as NFSv4 ACLs or
SELinux/Apparmor permissions.

There is a POSIX access(2) function (does this in fact respect ACLs? probably
not), but there is a comment above set_file_access_rights() that there is a
potential performance problem involving symlinks when using it.

The code path which attempts to open the stream as writable and handles the
error condition is vastly more reliable.  How would this break exactly when file
locking is turned off?
Comment 18 mikhail.voytenko 2009-04-21 07:19:18 UTC
mav->diocles: When the system file locking is off the document stream is opened
readonly. We could of course try to open it read-write, but on some systems it
is not possible to open a document in read-write mode without system file
locking being used.

I understand your concerns, but sorry, the file status handling should work as
expected. If it can not work so in general, this API just has no sense.

Second, I would not like to introduce a special solution for the case when
file-system lock is active. There are already enough differences between these
cases in office behavior, and any change that lets file-system lock and OOo lock
scenarios behave differently should have a good reason. 
Comment 19 mikhail.voytenko 2009-04-21 08:41:05 UTC
Ok, it is probably indeed a good enough reason to introduce the difference
between system file locking implementation and OOo file locking implementation.
Actually, the mode when system file locking is off is used mostly in cases when
it is not reliable. In this case a non-perfect implementation of ACL handling
would be acceptable.

I shall change the implementation in the way that the file is opened directly
for editing ( without file status evaluation ) in case system file locking is
active.
Comment 20 kpalagin 2009-07-22 20:54:03 UTC
Mikhail,
is this issue still planned for 3.2?

Regards,
Kirill Palagin.
Comment 21 mikhail.voytenko 2009-09-02 20:56:20 UTC
Fixed in fwk116. Now the office tries to open the file for editing always.

There is still a problem with this solution. If the problematic file is locked
by another thirdparty application using the system locking, the office is not
able to detect it.

The reason for this is that if the file can not be opened for editing, the
office still has to use osl_getFileStatus() to detect whether the file is
readonly, and only if it is not readonly the office treats it as a locked one.
So for this scenario we still have to improve osl_getFileStatus(), although this
scenario looks to be much less problematic than the original one described in
this issue.
Comment 22 mikhail.voytenko 2009-09-03 17:07:01 UTC
mav->tm: Please test the issue. To test it you could use the following steps on
solaris ( the cws has unxsoli4.pro build, so it could be done on Intel Solaris
machine )
- create a document on disk that supports ACL ( for example "tausch" )
- make "chmod 444 <path to the document>"
- "setfacl -r -m group:<group name>:rw- <path to the document>"
- let another user from <group name> group open the document using the CWS
installation => the document should be editable.
Comment 23 mikhail.voytenko 2009-09-03 18:29:48 UTC
Just to clear the problem described in my comment from Sep 2. The mentioned
document will not be recognized as locked one in the described in the comment
scenario, instead it will be handled as a read-only document.
Comment 24 mikhail.voytenko 2009-09-03 18:30:42 UTC
*** Issue 101922 has been marked as a duplicate of this issue. ***
Comment 25 thorsten.martens 2009-09-10 13:01:30 UTC
checked and verified in cws fwk116 -> OK !