Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZipArchive permissions on linux #17912

Closed
chris-mackubin opened this issue Jul 21, 2016 · 24 comments · Fixed by #70735
Closed

ZipArchive permissions on linux #17912

chris-mackubin opened this issue Jul 21, 2016 · 24 comments · Fixed by #70735
Labels
area-System.IO.Compression os-linux Linux OS (any supported distro)
Milestone

Comments

@chris-mackubin
Copy link

ZipArchive doesn't currently allow manipulation permissions within. I ran into this when creating file in memory, dropping it into a zip and handing off to another system. With this code deployed on Linux, I'm not seeing a way to give the receiving system read access to contents of zip.

Example:

    using (var memoryStream = new MemoryStream())
    {
        using (var archive = new ZipArchive(memoryStream, ZipArchiveMode.Create, true))
        {
            var lambdaFile = archive.CreateEntry("index.js");

            using (var entryStream = lambdaFile.Open())
            using (var streamWriter = new StreamWriter(entryStream))
            {
                streamWriter.Write(functionContent);
            }
        }

        memoryStream.Seek(0, SeekOrigin.Begin);
        svcFile.UploadFile(bucketName, s3Key, memoryStream);
    }
@ianhays
Copy link
Contributor

ianhays commented Jul 21, 2016

I'm not sure I follow. In your example you don't actually have a zip file, you have a stream that contains zipped data. Are you sending that data somewhere and constructing a new ZipArchive object from it?

Or are you saying that when unzipping a zip you want the files within to have different Unix permissions? We don't have support in CoreFX to modify traditional unix RWX permissions, though I added an issue to track that here.

It's not clear where the failure point in your example is.

@chris-mackubin
Copy link
Author

chris-mackubin commented Jul 22, 2016

Sorry, should have added more detail. In my example above, the upload method is sending the stream into another system (in this case AWS S3) which is saving the stream to disk in their system. By default thou, there is no read access on the ArchiveEntry, leading to permission denied when the other system attempts to access the contents of zip.

I should note: The service that is running the code above to create the ZipArchive and stream is running on Linux in a docker container.

  • If I manually download the same zip file back out of S3 and access onto a windows machine I have access. But, if i download same said zip file down onto a linux box, the zip file has permissions, the entries inside have none.

    -rw-rw-r-- 1 ec2-user ec2-user 1507 Jul 21 07:16 test.zip
    ---------- 1 ec2-user ec2-user 3788 Jul 20 15:32 index.js

It's not that I want to change the permissions at unzip. I want to set the permissions at creation or better yet within the ZipArchive. I shouldn't have to create a physical file to set them. Same as if I manually created a file, gave it permission and then added it to a Zip. The permissions of the file within would retain those permissions when unzipped. The permissions are part of the data being compressed.

@ianhays
Copy link
Contributor

ianhays commented Jul 22, 2016

How are you unzipping on the target AWS machine? Are you using ZipArchive, ZipFile, or `the``unzip``` command?

The Zip format has a bit of a shaky relationship with unix file permissions. Technically Unix RWX isn't formally supported as a part of the spec, but some programs represent it through the extra flags in the file headers. Others will assume permissions based on the permissions of the zip itself when unzipping.

@ianhays ianhays self-assigned this Jul 22, 2016
@chris-mackubin
Copy link
Author

chris-mackubin commented Jul 22, 2016

I'm not unzipping it, it's handed off to the external system (aws). Zip contains a lambda function, they are unzipping and pulling out contents. So can't say with certainty. Given they are Linux, I would assume unzip.

Interesting point, if the above example were run on a windows machine, the zip file and contents handed off is readable. If I pull the zip file down and opened it on a Linux box, I can see the zip file and content both have -rw-rw-r-- rights.

@ianhays
Copy link
Contributor

ianhays commented Jul 22, 2016

Interesting point, if the above example were run on a windows machine, the zip file and contents handed off is readable

The only real difference in our ZipArchive/Entrys between Unix and Windows is that a ZipArchive created on Unix has the ZipVersionMadeByPlatform bit set to indicate it came from a Unix machine. I did some manual testing to repro your issue, and it seems the Unix unzip command defaults permissions for zips created on Windows but not for those created on Unix. I'm guessing that it expects extra flags to be set for zips created on Unix, and since our zips do not have those flags, permissions are set to 0 since that's what's in the extra flag field.

My suggested workaround would be to chmod when you unzip, but it sounds like you don't have control over that. The long term solution here would be to default the extra flag permission values when creating a zip on Unix and possibly provide some sort of control over them.

@ianhays
Copy link
Contributor

ianhays commented Aug 25, 2016

The long term solution here would be to default the extra flag permission values

On second thought, this doesn't make a ton of sense. The values being excluded should already be treated as "default". Using the extra bits to retain unix permissions is a fairly new part of Zip, so unzip should accommodate the case where they're left out in the same way that they accommodate zips created on a non-Unix platform.

and possibly provide some sort of control over them.

Personally I'd like to see everything in the header exposed eventually, but that's a big API pill to swallow particularly when you account for the huge number of undocumented 3rd party extra fields.

For now though, I don't think we should mess with adding permissions values for new entries unless they are specifically set by the programmer. There are too many instances where assuming permissions for new entries could very well be the wrong thing to do. It would be nice to retain existing file permissions for ZipFile.CreateFrom*, but as long as ZipFile is its own assembly that's going to require adding the API to ZipArchiveEntry first.

For some background on the formatting, this StackOverflow post does a good job describing the format of the external attr field that Unix uses to store its permissions.

@ianhays ianhays closed this as completed Aug 25, 2016
@chris-mackubin
Copy link
Author

I get you, just sucks. Basically in it's current form the functionality is limited when run on a non-windows server. For a work around, I wound up adding a condition for OSPlatform and on non-windows having to not utilize ZipArchive at all, but rather run zip command via system.diagnostics.process 😞

I was however able to avoid also having to run chmod is similar fashion. Only because i found by looking through code base, that if File.Attributes is set to ReadOnly (which luckily was enough in my case 😃 ) it was handled in corefx. Only reference i saw doing such...

@auspex
Copy link

auspex commented Aug 23, 2019

I have a program pulling two archive files from a server (GeoServer, running in Linux), combining the two by writing the contents of each to a new stream, and downloading the new file to the user's browser:

        public IActionResult GetShapeFile(
            string filename,
            string cql_filter,
            string viewparams
        )
        {
            return new FileCallbackResult(new MediaTypeHeaderValue("application/zip"), async (outputStream, _) =>
            {
                using (var zipArchive = new ZipArchive(new WriteOnlyStreamWrapper(outputStream), ZipArchiveMode.Create))
                {
                    await copyZip(zipArchive, $"{_settings.EffortLayer}_Detail", viewparams);

                    if (cql_filter != null){
                        await copyZip(zipArchive, $"{_settings.SpeciesLayer}_Detail", viewparams, $"{cql_filter}");
                    }
                }
            })
            {
                FileDownloadName = $"{filename}"
            };
        }
        public async Task<int> copyZip (ZipArchive outZip, string layer, string viewparams, string taxon=""){
            // get the layer as a zip stream
            using (var stream = await _wfsService.GetLayer(layer, viewparams, taxon))
            {
                // open the incoming stream read-only
                using(var inZip = new ZipArchive(stream, ZipArchiveMode.Read))
                {
                    // copy each entry in the stream to 'outZip'
                    foreach (ZipArchiveEntry entry in inZip.Entries)
                    {
                        var name = entry.FullName;
                        var zipEntry = outZip.CreateEntry(name);
                        using (var outStream = zipEntry.Open())
                            using (var inStream = entry.Open())
                                await inStream.CopyToAsync(outStream);
                    }
                }
            }
            return 1;
        }

If the dotnet app is running on Linux, I get no read permissions. If it's running on Windows, I do.

Since it's a web app, I can hardly expect the user to know that the only way to use his downloaded Zip is to unzip it and chmod the resulting files.

Now, it's not a huge problem for me, as the dotnet server is going to be on Windows for the foreseeable future, but I shouldn't be tied there just because ZipArchive doesn't provide decent defaults. Especially since the input archives are being created with the Unix permissions set correctly, so it should be possible to create an archive entry that exactly matches one that's being read.

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 1.1.0 milestone Jan 31, 2020
@daves0
Copy link

daves0 commented Feb 18, 2020

This is still an issue. I'm creating a .zip archive in a MemoryStream using ZipArchive that is emailed to someone, and when the recipient opens the .zip and saves the contained file to disk, that file has NO read/write access permissions and cannot be used unless they change the file permissions using their OS. Unlike Auspex above, I am not using a Windows server (I'm running on AWS Lambda) and this is a show-stopping bug for me.

From @ianhays comment above on Jul 22, 2016, I have to wonder if the ZipVersionMadeByPlatform bit should NOT be set to Unix, since in fact these .zip files do not conform to ones created on Unix .. these archives appear like Windows ones in that they should be handled like them by Unix unzip, per the comment "and it seems the Unix unzip command defaults permissions for zips created on Windows but not for those created on Unix."

Would that be an appropriate way to go forward with these, and get this issue resolved? Without some resolution the ZipArchive functionality just isn't an acceptable option at this point unfortunately.

@skaman
Copy link

skaman commented Mar 3, 2020

I confirm that ZipArchive is not usable for create zip files on a linux server (for this problem with permissions). I had to use a third party library. I think it should generate readable zip files also on linux servers.

@marrrcin
Copy link

marrrcin commented Mar 9, 2020

This is still a valid issue, I don't understand why it's closed.

@danmoseley danmoseley reopened this Mar 9, 2020
@danmoseley
Copy link
Member

danmoseley commented Mar 9, 2020

Reopening in case IO area triage would like to see this feedback. (I have no views)

@danmoseley danmoseley added the untriaged New issue has not been triaged by the area owner label Mar 9, 2020
@jgimness
Copy link

I just ran into this problem today.

There's also issue #1548 which looks related

@marcwittke
Copy link

entry.ExternalAttributes = entry.ExternalAttributes | (Convert.ToInt32("664", 8) << 16);

...assuming 664 permissions

@carlossanlop carlossanlop modified the milestones: 1.1.0, Future Jul 8, 2020
@carlossanlop carlossanlop removed the untriaged New issue has not been triaged by the area owner label Jul 8, 2020
@ptsneves
Copy link

ptsneves commented Jul 11, 2020

I confirm @marcwittke solution works. Thanks a lot man. This was quite a deal breaker.
Before the ExternalAtributes change:

# unzip -Z /tmp/videos.zip 
Archive:  /tmp/videos.zip
Zip file size: 8860217 bytes, number of entries: 3
?---------  2.0 unx  1655297 b- stor 20-Jun-26 15:07 0-video.webm
?---------  2.0 unx  2738625 b- stor 20-Jun-16 18:42 4-video.webm
?---------  2.0 unx  4465751 b- stor 20-Jun-18 01:16 7-video.webm

After


# unzip -Z /tmp/videos.zip 
Archive:  /tmp/videos.zip
Zip file size: 8860217 bytes, number of entries: 3
?rw-rw-r--  2.0 unx  1655297 b- stor 20-Jun-26 15:07 0-video.webm
?rw-rw-r--  2.0 unx  2738625 b- stor 20-Jun-16 18:42 4-video.webm
?rw-rw-r--  2.0 unx  4465751 b- stor 20-Jun-18 01:16 7-video.webm

@v-yaremenko
Copy link

@marcwittke , @ptsneves , thanks for the provided solution.
However, I've faced that executable bit for user can't be set in a such way.
rwXrwxrwx <-- I mean the marked bit

My code:
fileEntry.ExternalAttributes |= (Convert.ToInt32("100777", 8) << 16);

Result (it is set to some default value):
-rw------- 2.0 fat 24 b defN 20-Sep-18 17:35 fileA.txt

But if I try to set 677, or some value without X bit for user, then it is okay:
fileEntry.ExternalAttributes |= (Convert.ToInt32("100677", 8) << 16);
-rw-rwxrwx 2.0 fat 19 b defN 20-Sep-18 17:35 fileB.txt

Would you please confirm that you are able to set the X bit for user?
If you have same issue, then it is a bug, as I understand.

@KirillOsenkov
Copy link
Member

I think the bug is that the ExternalAttributes field on ZipArchiveEntry defaults to 0 and common entrypoint APIs don't set it:

So if I'm using any standard high-level Zip creation API such as MSBuild's ZipDirectory task, which calls ZipFile.CreateFromDirectory (https://source.dot.net/#System.IO.Compression.ZipFile/System/IO/Compression/ZipFile.Create.cs,e30e0208967e9503), all the files will have permissions 0.

Combined with the global flag (I'm guessing) that controls whether the archive was created on Windows or not, the archive (when unzipped on Mac or Linux) will create files with 0 permissions.

We ran into this when we switched from building using Mono (which created archives that unpacked properly) to .NET.

@KirillOsenkov
Copy link
Member

I think on Mac/Linux it should copy the permissions of the file being archived (just like it copies the timestamp). When creating on Windows, it should default to 644 for files (and maybe 755 for .exe files and scripts??)

@KirillOsenkov
Copy link
Member

I think this might be a relevant link: https://stackoverflow.com/a/6297838/37899

@KirillOsenkov
Copy link
Member

KirillOsenkov commented Mar 5, 2021

I'm using https://ide.kaitai.io to spelunk inside the binary format and I do notice that on a .zip file presumably created using Mono we have the upper byte of VersionMadeBy set to 0:
image

Whereas in the file that has 0 permissions that byte is set to 3:
image

which tells me that the unpacker doesn't default the permissions to 644 if the byte is set to 3:
https://source.dot.net/#System.IO.Compression/System/IO/Compression/ZipArchiveEntry.Unix.cs,0c3d2244301f9384,references

Haha and looking at the Mono implementation it has a bug:
https://github.com/mono/mono/blob/d035e181d1b640e3b40edd3c00d94874fd7245f5/mcs/class/System.IO.Compression/corefx/ZipArchiveEntry.Mono.cs#L7-L8

Basically Mono will always return "Windows" even when on Mac, because the Path.PathSeparator is ; on Windows and : on Mac. They probably wanted Path.DirectorySeparatorChar instead ;)

@derwasp
Copy link
Contributor

derwasp commented Jun 14, 2022

Background

Recently we hit an issue where our customers saw that the artifact files in the zip archives that we produce have the permissions set to 000 if our system runs on Unix machines. When the unzip command is used, these permissions translate to the actual files also having the same permissions.
This was an inconvenience to them as if the same system is ran on Windows, the files would have the 644 permissions. The customers wanted this to have the same behavior.

Causes

Based on how I see it, the zip archives have two fields which can be responsible for the file permissions.

Version made by

One of them is Version made by. (See the Central directory file header section in the wiki page ) This is what the comment says about this field in the code:

The upper byte of the "version made by" flag in the central directory header of a zip file represents the
OS of the system on which the zip was created. Any zip created with an OS byte not equal to Windows (0)
or Unix (3) will be treated as equal to the current OS.
The value of 0 more specifically corresponds to the FAT file system while NTFS is assigned a higher value. However
for historical and compatibility reasons, Windows is always assigned a 0 value regardless of file system.

So to simplify the story, we can say that we have two possible options here: 0 for Windows and 3 for Unix.

This field is written here: ZipArchiveEntry.cs#L517

The unzipping software is using this field to determine that the permissions were present in the source system. In other words: we probably need to set the permissions to 644 on the Unix system if the archive was created on Windows, where these permissions don't exist. And this is exactly the behavior with unzip that we see.

External attributes

Another field is External attributes. This field is used to store in the file permissions when the archives are created on the Unix systems. Recently, the API that works with files received support for this: PR.
As you can see from the PR, the permissions are based on the actual permissions from the files:

    static partial void SetExternalAttributes(FileStream fs, ZipArchiveEntry entry)
    {
        Interop.Sys.FileStatus status;
        Interop.CheckIo(Interop.Sys.FStat(fs.SafeFileHandle, out status), fs.Name);

        entry.ExternalAttributes |= status.Mode << 16;
    }

So now from the perspective of the unzipping software if we have a Version made by set to 3 (meaning Unix), we need to read the External attributes and apply the corresponding permissions to the files. And if Version made by is set to 0 (meaning Windows) we need determine the permissions ourself and probably set it to 644.

The issue

If the file API is used, then everything already works: the permissions are preserved and when the unpacking software is going to work with the archive it would create the files with same permissions as before.
The caviat is that when we are creating a new ZipArchiveEntry from memory on the Unix machines. In this case, there's no file to begin with, but this new ZipArchiveEntry can still be unzipped using other software, such as unzip, which would follow the logic based on the Version made by flag.

When this happens, currently, we would create an archive which, when unzipped, would result in files having 000 permissions if our zipping logic runs on Windows and 644 (probably depends on the implementation of the unzipping software) if our zipping logic runs on Unix.

Even if this is not techincally a bug, at least, this behavior is not consistent across the two platforms from the users point of view.

A minimal repro looks like this:

using System.IO.Compression;

using var fileStream = new FileStream(@"test.zip", FileMode.Create);
using var archive = new ZipArchive(fileStream, ZipArchiveMode.Create, true);

ZipArchiveEntry demoFile = archive.CreateEntry("inmemory_file.txt");

using var entryStream = demoFile.Open();
using var streamWriter = new StreamWriter(entryStream);
streamWriter.Write("Hello zip!");

Console.WriteLine("Done.");

After running this on a Unix machine and unizipping the result with unzip test.zip you can do an ls -la and you would see something like this:

 xxxx@xxxx  ~/Projects/ConsoleApp1/ConsoleApp1/bin/Debug/net6.0  ls -la
drwxr-xr-x  10 derwasp  staff     320 Jun 14 14:48 .
drwxr-xr-x   3 derwasp  staff      96 Jun 13 16:42 ..
-rwxr-xr-x   1 derwasp  staff  132208 Jun 14 14:48 ConsoleApp1
-rw-r--r--   1 derwasp  staff     403 Jun 14 14:48 ConsoleApp1.deps.json
-rw-r--r--   1 derwasp  staff    5120 Jun 14 14:48 ConsoleApp1.dll
-rw-r--r--   1 derwasp  staff   10480 Jun 14 14:48 ConsoleApp1.pdb
-rw-r--r--   1 derwasp  staff     139 Jun 14 14:48 ConsoleApp1.runtimeconfig.json
----------   1 derwasp  staff      10 Jun 14 14:48 inmemory_file.txt
drwxr-xr-x   3 derwasp  staff      96 Jun 14 14:48 ref
-rw-r--r--   1 derwasp  staff     150 Jun 14 14:48 test.zip

And you can see that the file permissions are set to 000.

Workaround

To work this around one can do something like this:

    public static class ZipArchiveEntryExtensions
    {
        public static ZipArchiveEntry EnsureReadPermissions(this ZipArchiveEntry entry)
        {
            if (!OperatingSystem.IsWindows())
            {
                // BUGFIX
                // this is an equivalent of (Convert.ToInt32("644", 8) << 16)
                // where 644 are RW-R-R permissions
                const int readPermissions = 420 << 16;
                entry.ExternalAttributes |= readPermissions;
            }

            return entry;
        }
    }

And in the code, whenever you create a new entry, you would call the extension method like so (if we are modifying the repro from above):

using System.IO.Compression;

using var fileStream = new FileStream(@"test.zip", FileMode.Create);
using var archive = new ZipArchive(fileStream, ZipArchiveMode.Create, true);

ZipArchiveEntry demoFile = archive.CreateEntry("inmemory_file.txt").EnsureReadPermissions();

using var entryStream = demoFile.Open();
using var streamWriter = new StreamWriter(entryStream);
streamWriter.Write("Hello zip!");

Console.WriteLine("Done.");

Possible solution

It seems like the solution would be for us to set the permissions for such ZipArchiveEntry's to be 644 at the time of creation. And it seems like we can do it in ZipArchiveEntry.cs's constructors for the non-windows platforms. This, however, will change the current behavior and I am open to suggestions about which approach is best.

I would love to hear your thoughts on it @KirillOsenkov @danmoseley @ianhays

@danmoseley
Copy link
Member

Cc @eerhardt who added the original change to preserve Unix permissions IIRC.

@eerhardt
Copy link
Member

Has anyone looked at other implementations (Java, Python, Go, etc) to see what they do when creating .zip or .tar files from in-memory data? Do they have a hard-coded default permission (other than 000)?

Looking at our own, new Tar implementation we seem to default to 644 as well:

_header._mode = (int)TarHelpers.DefaultMode;

internal const TarFileMode DefaultMode = // 644 in octal
TarFileMode.UserRead | TarFileMode.UserWrite | TarFileMode.GroupRead | TarFileMode.OtherRead;

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 14, 2022
@derwasp
Copy link
Contributor

derwasp commented Jun 14, 2022

I didn't look into the other implementations yet. Tar implementation looks promising though, thanks for bringing this up @eerhardt.

I made a draft PR for the proposed fix so that we have it out there and we can collaborate further.

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 1, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jul 31, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.IO.Compression os-linux Linux OS (any supported distro)
Projects
None yet
Development

Successfully merging a pull request may close this issue.