openzfs / zfs

OpenZFS on Linux and FreeBSD
https://openzfs.github.io/openzfs-docs
Other
10.5k stars 1.74k forks source link

NFS/POSIX ACL support #170

Closed user318 closed 10 years ago

user318 commented 13 years ago

It will be good to have POSIX ACL support. As I see zfs already have xattr support and some other filesystems made ACL support over xattr. I'm do not know the internals, but this task may be simple to implement.

behlendorf commented 13 years ago

Things are unfortunately a little trickier than it first appears.

Internally ZFS fully supports and enforces NFS style ACLs. Unfortunately, under Linux the existing tools only manipulate Posix style ACLs. There has been some work done to the bring NFS ACL model to Linux under the name Rich-ACLs. To integrate with the new Rich-ACL tool chain ZFS needs to provide a virtual system.richacl xattr interface. This xattr would not be stored like other xattrs but would instead integrate with the zfs_getacl() and zfs_setacl(). This xattr hook would be responsible for translating a vsecattr_t to and from a lineal stream of bytes for the xattr.

Posix ACLs can be easily supported by adding a few hooks and leveraging the existing Posix ACL support functions. However, it may be best that they not be implemented to avoid coherency issues between Posix and Rich ACLs (NFS/ZFS).

user318 commented 13 years ago

Thank You for description. I think POSIX ACL can be helpful too if there is some applications that have POSIX ACLs support. As I remember samba have something like that. rsync have ACL support, but I'm not sure that it only POSIX, because man says just "ACL". Do not know about other applications. I just want to say they are not useless even with presence of other ACLs. And may be considered to be implemented in the future.

behlendorf commented 13 years ago

Summary of Required Work

Internally ZFS fully supports and enforces NFS style ACLs. Unfortunately, under Linux the existing tools only manipulate Posix style ACLs. There has been some work done to the bring NFS ACL model to Linux under the name Rich-ACLs (pdf). To integrate with the new Rich-ACL tool chain ZFS needs to provide a virtual system.richacl xattr interface. This xattr would not be stored like other xattrs but would instead integrate with the zfs_getacl() and zfs_setacl(). This xattr hook would be responsible for translating a vsecattr_t to and from a lineal stream of bytes for the xattr.

Posix ACLs can be easily supported by adding a few hooks and leveraging the existing Posix ACL support functions. However, it may be best that they not be implemented to avoid coherency issues between Posix and Rich ACLs (NFS/ZFS).

maxximino commented 13 years ago

What about a mount option / file-system property about WHICH security model has to be enforced? (and maybe the other one completely hidden even from management tools, like setfacl/getfacl). On Linux world Rich-ACLs are almost unused. Posix ACLs are much more utilized. In my typical configurations, absence of ACLs for a filesystem is a showstopper. I think that in this way we can get a working ACL implementation (Posix) in much less time.

toadicus commented 13 years ago

I, too, would like to see POSIX ACL support integrated into ZFS on Linux. Linux is POSIX, and until RichACLs are more mainstream (or at least in the kernel), I think POSIX integration in ZFS on Linux makes sense.

tomharvey commented 13 years ago

I would love to see this included! Was very nearly a showstopper for us, but decided to live without for a few months.

behlendorf commented 13 years ago

When ACLs are cleanly handled we should ensure the following patch to resurrent the aclmode property is merged. This change has already been made to the Illumos and FreeBSD implementations.

Issue #742: Resurrect the ZFS "aclmode" property https://www.illumos.org/issues/742 https://github.com/illumos/illumos-gate/commit/a3c49ce110f325a563c245bedc4d533adddb7211

maxximino commented 12 years ago

It is possible to map Posix < - > NFSv4 ACLs. IETF has a draft about that mapping: http://tools.ietf.org/id/draft-ietf-nfsv4-acl-mapping-03.txt But it only clearly specifies Posix => NFSv4 (unidirectional).

I think that the mapping approach is theoretically the best one, but it is more error-prone than others as 1:1 mapping is not possible. There will always be corner cases in which an user is denied (or worse, given) a privilege that's not meant to be denied (or worse, given). At least it should come with a "big fat warning". But it's not wise to have security feature that must approximate what it should do. Proposal: on "equivocal" NFSv4 acl disallow ANY access and print error on kernel log. Problem about that proposal: Snapshots cannot be manually corrected. For setting them it shouldn't be a problem, as it can be simply seen as an “odd” on-disk-format for Posix ACL's. Much more configurable inheritance of NFSv4 ACLs creates another set of problems to solve.

I think that a first step this implementation should able to:

In successive steps, the mapping logic NFSv4 => Posix can be tuned and made much more customizable.

Any better ideas?

behlendorf commented 12 years ago

This seems like a reasonable starting place to me, just a few comments.

While a perfect 1:1 mapping isn't possible things are not actually so bad. As you point out there is a well specified IETF Posix -> NFSv4 mapping which can be used to set the correct ACLs on-disk. Once set on-disk as a SA the existing zfs implementation should start enforcing them independent of generic Linux ACL hooks in the VFS.

You'll then of course need to implement some reasonable NFSv4 -> Posix mapping to read the ACLs. However, reasonable implementations of this already do exist. Case in point the Linux nfs kernel server which is forced to store all of its NFSv4 ACLs as Posix ACLs which is a lossy operation. As an aside, in the longer term exposing the raw nfsv4/zfs ACL would be a good thing for the nfs kernel server. You could potentially avoid a pointless NFSv4 -> Posix -> NFSv4 conversion.

Finally, we'd want to have a test suite to verify we got this right. This is a security feature after all. Happily it's my understanding that several good test suites already exist.

rohan-puri commented 12 years ago

There is a work going on the richacls by Aneesh kumar, Andreas Gruenbacher on the previous work done by Greg Banks. Patches were submitted for 3.1 mainline merged., but due to some changes it will take time & will be merge in the next release. Link to patches : - https://lkml.org/lkml/2011/10/18/279

Once this gets to mainline, we can make use of them to support nfsv4acls for ZFS.

user318 commented 12 years ago

Once this gets to mainline, we can make use of them to support nfsv4acls for ZFS.

And what about POSIX ACLs? Brian said that it is not so hard to implement them usinx xattr. Can this be done too by someone, please. :)

rohan-puri commented 12 years ago

ZFS supports nfsv4acls, so IMHO we should give support for nfsv4acls first & see whether posix acls are really needed. If yes then we can figure out the way to define a mapping between them.

behlendorf commented 12 years ago

I don't see any reason why both can't be done. Maxximino is currently working on supporting the system.posixacl xattr interface via a well documented translation. Adding the system.richacl xattr interface can be supported when its integrated and a real tool chain is available.

maxximino commented 12 years ago

"currently working" as academic- and work-related tasks allow me, but in high priority for "free" time. I've already looked into IllumOs CDDL licensed code and found some code that does NFSv4 <-> posix acl translation. It should be a solid base, from correctness point of view. Details upon request.

aarcane commented 12 years ago

I know solaris just provides two copies of chmod to manage ACLs. Unfortunately, this is very awkward and clunky. I propose that we use a helper program, setzacl, getzacl or similar to provide ACL viewing and modification facilities on zfs on linux. Preferably, the interface should be created to be similar to (or compatible with) solaris chmod, possibly improved, but standardized across both zfs linux implementations, and preferably able to handle future filesystems with "nfs4" acls. (If anyone can explain why they're called nfs4 ACLs, that'd be a big help, too!)

randomvariable commented 12 years ago

I would add that Samba users might like to stick with nfs4 ACLs, not least as they come closest to matching Windows NTFS ACLs feature by feature. This matters when you want to use Samba as a server for users home and profile folders in an Active Directory environment where newer versions of Windows check for specific ACLs. Also, Windows interop was one of the design goals for ZFS back in the days of Sun, so it'd be sad to see it go...

That said, I fully understand the drive for POSIX compliance.

aarcane - See http://wiki.linux-nfs.org/wiki/index.php/ACLs for the history of NFSv4 ACLs.

fireappleblack commented 12 years ago

I'm experimenting with a Samba 4 DC and ZFS CIFS share at the moment; the base system is ubuntu 12.04.

XP Pro SP3 clients can see and administer Active Directory (users and computers) and set NTFS permissions properly on folders shared from EXT4.

Folders shared from ZFS become unbrowsable thru CIFS as soon as you alter their permissions via the XP gui (although you can still access them - as expected - from the server's command line).

I assume that's a result of the problems outlined in the thread above? Any work-around ideas would be gratefully received.

Enabling full NTFS ACL admin through Samba 4 would be a massive bonus for ZFS.

maxximino commented 12 years ago

Can you please post more details about your issue? Exact Samba4 version, which fileserver are you using (smbd or ntvfs) and any relevant settings? (e.g. are you using vfs_acl_xattr or something similar?) Maybe open another issue, IF the same Samba4 config works over extY mounted without "acl" option.

About manipulation of real NFSv4 (NTFS-like) ACLs through Samba, this can be done in a clean way only after "Rich ACL" patches are merged in the kernel.

fireappleblack commented 12 years ago

Thanks Maxximino. I'm using Samba 4.0.0 alpha19. I'm only attempting to server files with ntvfs (sudo /usr/local/samba/sbin/samba -i -M single) - which serves netlogon and sysvol shares from ext4 and my zfs shares from a mirrored pair of drives with aclinherit=passthrough set. No vfs_acl_xattr module is in use.

"About manipulation of real NFSv4 (NTFS-like) ACLs through Samba, this can be done in a clean way only after "Rich ACL" patches are merged in the kernel."

As far as I understand matters, the official richacls patches only support EXT4 - are you saying that if I e.g. use opensuse (with richacls included by default) or patch richacls into ubuntu / debian, zfs+samba4+ntfs acls should "just start working"?

maxximino commented 12 years ago

I've reproduced your issue. You are not using vfs_acl_xattr explicitly, but Samba4 is doing the same thing "silently". It saves his ACLs into the xattr named "security.NTACL" (which you can see with getfattr -n security.NTACL $FILENAME; and remove with setfattr -x security.NTACL $FILENAME). That attribute is considered only by Samba, not by anything in zfs/linux kernel. Since a simple perl program that saves values from /dev/urandom into xattrs in zfs, can read them back uncorrupted, I really think that it is a Samba4 problem. The problem shows only on zfs PROBABLY because on other fs it can map his ACLs on Posix ACLs instead of using xattrs.

No, simply patching a kernel with rich acl patches is not enough. When those patches land on mainline, it's possible to start to integrate rich acl support in zfs.

fireappleblack commented 12 years ago

I'm just building Samba 4 on Suse; I think I was about to reproduce your reproduction from a different angle. Saved me the pain.

Perhaps a reasonable (short term) workaround would be to serve files from zfs through a stable Samba 3 environment, authenticating against a Samba 4 DC in a different Linux-VServer for instance?

In the medium term, the utility of having ZFS-based CIFS shares combined with Samba 4 AD all based on a Linux OS with excellent generic hardware support in the same machine can't be overstressed. The small business / not-for-profit applications are huge.

Thanks for all the work so far.

kisg commented 11 years ago

I stumbled upon the following thread: https://lists.samba.org/archive/samba/2012-August/168660.html

The summary is that Samba 4 has an acl module called vfs_zfsacl, which is used on Solaris, so that Samba can use the native ZFS acls. Would it be possible to use this module with zfsonlinux? Are the necessary APIs made available to user space on Linux?

behlendorf commented 11 years ago

@kisg That's a good question, this is the first I've heard of the vfs_zfsacl module for Samba. Somebodies going to need to do some legwork to determine what interface they are expecting. Depending on what it is we might able able to provide it. Although if no translation is done you'll still have the issue of not being able to manage the ACLs on the Linux box.

kisg commented 11 years ago

I took a quick look into the source. You can find the Samba v4-0-stable branch version of vfs_zfsacl.c here: git.samba.org.

It simply converts from the internal Samba representation to the native SunOS NFSv4 acl/facl API. This API is implemented also on FreeBSD using a thin user-space wrapper around its own NFSv4 acl implementation.

Based on this analysis, we can't reuse this implementation on Linux. Instead, if the richacl integration of zfsonlinux is done, we could use the librichacl library to create a similarly simple (hopefully less than 1000 lines) vfs_richacl module for Samba.

From my cursory look at the richacl kernel patches, it seems that the zfsonlinux integration could even be done without actually patching the kernel, just pulling the required portions (data structures and xattr conversion) of the richacl code into the zfs tree for kernel versions which don't support it natively. This is important for us, because we (my company) would like to run a standard Ubuntu LTS kernel and only pull in zfsonlinux as an extra module.

However, I am not sure if richacl is still maintained (its repository is not really kept up to date), and on track for inclusion into the vanilla kernel.

kisg commented 11 years ago

I reached out to the author of the richacl patchset. He pointed me to the following experimental richacl vfs module: v4acls-experimental/samba.git

So it looks like that almost all pieces are in place (or at least exist in experimental form) except for the richacl support in zfs itself.

franx commented 11 years ago

Maybe porting libsunacl.c to linux could be sufficient from samba side ?

http://sourceforge.net/projects/libsunacl/

but as far as i understand "aclmode" will be still missing on zfsonlinux.

aarcane commented 11 years ago

I don't even know why we don't have some form of acl support yet. Why have zfs acls not been switched on? I know the code for a chmod and an ls exist.a getfacl style getzacl should have been provided by now.I'm sure there's some good reason for the lack of acls though. On Feb 15, 2013 2:07 PM, "franx" notifications@github.com wrote:

Maybe porting libsunacl.c to linux could be sufficient from samba side ?

http://sourceforge.net/projects/libsunacl/

but as far as i understand "aclmode" will be still missing on zfsonlinux.

— Reply to this email directly or view it on GitHubhttps://github.com/zfsonlinux/zfs/issues/170#issuecomment-13630736.

franx commented 11 years ago

zfs acl is supported test: create a file on a smb share on other filesystem with acl mount option enabled, set ACL on windows.

move that file on a zfsonlinux volume with aclinherit=passthrough acls are preserverd..

Wath atm doesn't have a solution is have it done in samba..

none of the acl_xattr or acl_tdb works correctly under such configuration, on bsd they use the vfs_zfsacl

trough libsunacl wich look's like a translator from zfs2bsd

vadik007 commented 11 years ago

My question maybe dumb, but i want to have clear understanding whether we have acl support or not. I`ve created VBox ubuntu minimal server 12.04 LTS, than installed ubuntu-zfs, created pool w commands

History for 'mypool': 2013-05-05.15:12:49 zpool create -f mypool /dev/disk/by-id/ata-VBOX_HARDDISK_VB88a04e0d-d8d1e7a4

History for 'tank': 2013-04-30.13:44:54 zpool create tank /root/vol1 2013-05-01.00:13:33 zfs set aclinherit=passthrough tank 2013-05-05.13:50:14 zfs set dedup=on tank

tank does support setfacl without problem mypool does NOT, and says operation not supported.

i want to use zfs with samba and i need to control multiple user access to folders. like that root@server:~# setfacl -m g:USERS:--- test4v007/

antst commented 11 years ago

I want to implement richacls patch for ZFS, but as I am totally inexperienced with ZFS, it will be nice if one of ZFSonLinux developers can provide some help (mostly some in form of hints and discussions).

iamacarpet commented 11 years ago

I'd like to give this a bump...

@behlendorf - Now you've got a stable filesystem release, do we have a solid timescale for when this will be done by? I know you've got it down for milestone 0.8, but at the current release pace, that seems like it could be a few years away - can we get it brought forward in any way?

We're wanting to move forward at work with deploying for our profile storage NAS servers with Samba, but not having ACLs may mean we have to stick with FreeBSD which we really don't want to do since all our experience base is with Linux.

tomposmiko commented 11 years ago

I hope, it's not full offtopic. Have you tried Debian kFreebsd?

It's almost like a kind of Linux..

iamacarpet commented 11 years ago

@sopmot - You know, I looked at it before and dismissed it as not being production ready, but I had a quick read up on it and it looks like it may be a little more battle-ready than ZoL for what we need if we're doing it straight away.

So good shout, thanks for setting me along the right track - Apologies for the rudeness.

aarcane commented 11 years ago

I keep looking at kfreebsd, and it's missing iscsi, nfsv4, and an equivalent to kvm virtualization. I was serious about using it for root until those shortcomings became apparent On May 19, 2013 9:50 AM, "Tamas Papp" notifications@github.com wrote:

I hope, it's not full offtopic. Have you tried Debian kFreebsd?

It's almost like a kind of Linux..

— Reply to this email directly or view it on GitHubhttps://github.com/zfsonlinux/zfs/issues/170#issuecomment-18120673 .

rightaditya commented 11 years ago

I'd also love to see this addressed. I'm unfortunately low on time these days (plus I don't know crap about low-level/kernel stuff), so I can't do much to help myself :( Really though I'm just looking for a good way to enforce certain permissions for new files in certain directories; I often put files in a publicly available folder and it'd be nice if they could be automatically set up with relaxed permissions, as I don't really want to set my umask to something so liberal for most files that I create, which end up in my home folder. In OSOL/FreeBSD I'd do this using NFSv4 ACLs, though I'm open to better solutions if anyone has any ideas. The only other thing I can think of is running a cronjob that recursively sets the perms on the relevant directories every half-hour or something, but this is so nastily inelegant! Such a kludge :(

iamacarpet commented 11 years ago

FYI so people don't make the same mistake as I have - Debian kFreeBSD is great for ZFS support, but the ACLs still don't work from userland, you just get "Function Not Implemented" - see Debian bug: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=607573

behlendorf commented 11 years ago

@iamacarpet It can happen as soon as someone who needs this functionality has time to work on it. Currently the major drivers of the project don't have much use for ACLs so they are at the bottom of the priority list. But there is nothing preventing someone who needs this support from jumping in and doing it sooner. Sorry, but it's just the reality of the situation.

n1mh commented 11 years ago

Hello,

we use zfsonlinux in a backup appliance and we think that ACLs are important for integration purposes. Some common operations, like changing permissions in a ZFS shared volume from a windows computer are needed.

Is ACL still in the roadmap?

Thanks.

ryao commented 11 years ago

@n1mh It is scheduled for version 0.8.0.

behlendorf commented 10 years ago

Thanks to @maxximino support for Posix ACLs has been implemented. I've opened pull request #1809 with a close-to-final-version of the patch which is ready for broader testing. It cleanly passes the ACL section of the Posix Test Suite and we're unaware of any outstanding issues.

For those who would like this functionality it would be very helpful if you could test the proposed patch with a realistic workload. Please verify it behaves as you would expect it to in your environment. Posix ACLs are disabled by default but may be enabled by setting the acltype property on the dataset.

zfs set acltype=posixacl pool/dataset
behlendorf commented 10 years ago

Linux style Posix ACLs have been implemented as an xattr and merged in to master. They are stored independently of the native NFS ACLs and will not conflict. The new dataset property acltype has been added to enable this functionality. For the best performance you are strongly encouraged to set both acltype=posixacl and xattr=sa. For further details see the updated man page:

       acltype=noacl | posixacl

           Controls  whether  ACLs  are  enabled and if so what type of ACL to
           use.  When a file system has the acltype property set to noacl (the
           default)  then  ACLs are disabled.  Setting the acltype property to
           posixacl indicates Posix ACLs should be used.  Posix ACLs are  spe-
           cific  to  Linux  and are not functional on other platforms.  Posix
           ACLs are stored as an xattr and therefore will  not  overwrite  any
           existing ZFS/NFSv4 ACLs which may be set.  Currently only posixacls
           are supported on Linux.

           To obtain the best performance  when  setting  posixacl  users  are
           strongly encouraged to set the xattr=sa property.  This will result
           in the Posix ACL being stored more efficiently on disk.  But  as  a
           consequence of this all new xattrs will only be accessable from ZFS
           implementations which support the xattr=sa property.  See the xattr
           property for more details.
aarcane commented 10 years ago

Should an item acltype=nfs4 also be recognized and treated the same as noacl, but accepted for compatibility sake? On Oct 29, 2013 3:05 PM, "Brian Behlendorf" notifications@github.com wrote:

Linux style Posix ACLs have been implemented as an xattr and merged in to master. They are stored independently of the native NFS ACLs and will not conflict. The new dataset property acltype has been added to enable this functionality. For the best performance you are strongly encouraged to set both acltype=posixacl and xattr=sa. For further details see the updated man page:

   acltype=noacl | posixacl

       Controls  whether  ACLs  are  enabled and if so what type of ACL to
       use.  When a file system has the acltype property set to noacl (the
       default)  then  ACLs are disabled.  Setting the acltype property to
       posixacl indicates Posix ACLs should be used.  Posix ACLs are  spe-
       cific  to  Linux  and are not functional on other platforms.  Posix
       ACLs are stored as an xattr and therefore will  not  overwrite  any
       existing ZFS/NFSv4 ACLs which may be set.  Currently only posixacls
       are supported on Linux.

       To obtain the best performance  when  setting  posixacl  users  are
       strongly encouraged to set the xattr=sa property.  This will result
       in the Posix ACL being stored more efficiently on disk.  But  as  a
       consequence of this all new xattrs will only be accessable from ZFS
       implementations which support the xattr=sa property.  See the xattr
       property for more details.

— Reply to this email directly or view it on GitHubhttps://github.com/zfsonlinux/zfs/issues/170#issuecomment-27348094 .

synnack commented 9 years ago

Why is this closed? acltype nfs4 is far more important than the nonstandard completely obsolete limited POSIX draft ACLs. NFS ACLs are the default for ZFS on other platforms and are far more flexible. They also allow seamless exporting on both NFSv4 and SMB as the ACL actually maps well to both NFS and NT ACLs. POSIX draft ACLs do neither well.

POSIX draft ACLs also don't deal well with inheritence, only giving a default for new files.. NFSv4 ACLs are the only way forward.

behlendorf commented 9 years ago

@synnack the major problem with supporting NFS ACLs isn't really on the ZFS side. We've preserved all that functionality and it is used internally. The issue lies with the Linux utilities (getfattr, setfattr, etc) which are built around POSIX rather than NFS ACLs. There have been attempts in the past to bring NFS ACLs to Linux but to my knowledge none of have been widely successful. Unless things have changed recently that's the biggest obstacle.

synnack commented 9 years ago

Sure, but look at Andreas Gruenbacher and Aneesh Kumar's work at OpenSuSE, they already ship richacl patches.. On the LKML for inclusion now..

unya commented 9 years ago

Except richacls are not NFSv4 ACLs, they are (bit insane) result of merging NFSv4 schema with POSIX ACLs, designed for ext4 and retaining all the worst parts of POSIX ACLs, IIRC.

What we need is a proper interface for NFSv4 ACLs, so that filesystems that support them can have them set. Mind you, there's at least one more type of ACLs supported (partially at least) by linux - AFS ACLs. So the possibility of having multiple schemes supported is not insane, though I guess we might need a Solaris-like API to support it best...

Of course, if richacls can be wrangled to stop all the parts that go outside NFSv4, and assuming userland doesn't screw you over (hello there, POSIX ACL mask bits!), and assuming that they actually implement all of NFSv4 spec... That's a lot of assumptions, to be honest.

I would actually propose adding an IOCTL applicable to files on ZFS at this rate

synnack commented 9 years ago

Yeah, I'm not sure what the guys are sniffing with the POSIX draft ACL stuff merged inside rich ACLs.. Better to be compatible with solaris and the BSDs than with the crappy POSIX draft ACLs imho..

ghfields commented 8 years ago

@behlendorf:

The issue lies with the Linux utilities (getfattr, setfattr, etc) which are built around POSIX rather than NFS ACLs. There have been attempts in the past to bring NFS ACLs to Linux but to my knowledge none of have been widely successful. Unless things have changed recently that's the biggest obstacle.

I see distros are using the "nfs4-acl-tools" package for NFS4 acl management. It uses nfs4_getfacl, nfs4_setfacl, and nfs4_editfacl. When I run these against zfs, I currently get "Operation to request attribute not supported." This seems to me the way linux is going to do NFS4. Now we just need a way for the tools and zfs to be aware of each other.

behlendorf commented 8 years ago

@ghfields thanks for commenting, I haven't looked in to nfs4 acls for a few years but it looks like they're making really good progress with the user space components. Based on a cursory reading of the nfs4-acl-tools source it looks like the expected user/kernel interface is via an xattr named system.nfs4_acl which contains the raw xdr encoded acl.

Getting this working might only require adding xattr handlers for a system.nfs4_acl xattr which translates between the nfs4 acl stored internally by ZFS and the representation of it expected by the utilities. Since NFSv4 is the only consumer of this the kernel doesn't provide any generic functionality we can use we'd need to write the functions to do this encoding/decoding.

On the surface getting this working looks very possible. I think it would be great if a developer wanted to tackle this feature.

ghfields commented 8 years ago

Since this issue was closed when the POSIX acls was implemented, should a new issue be created specifically for NFS4 acls? Or should this one be reopened?