Detect password protected file with ZipArchive

[srikrishnag]

Currently if a file is not able to be decompressed, there is no information given in the exception regarding if it was password protected file.

Expected outcome: Provide the reason for failure in extracting text for zip files, eg: A file was password protected etc

[ghost]

Tagging subscribers to this area: @dotnet/area-system-io-compression
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Currently if a file is not able to be decompressed, there is no information given in the exception regarding if it was password protected file.

Expected outcome: Provide the reason for failure in extracting text for zip files, eg: A file was password protected etc

Author:	srikrishnag
Assignees:	-
Labels:	area-System.IO.Compression, untriaged
Milestone:	-

[adamsitnik]

Triage: related to #1545, marked as "Bottom Up" to hopefully include it in .NET 7

[jeffhandley]

Provide the reason for failure in extracting text for zip files, eg: A file was password protected etc

@srikrishnag Could you provide a little more detail for how you'd like to have this reason exposed? Likely the most efficient thing is for you to show some code where you're currently using the System.IO.Compression APIs to decompress, highlighting what "failure" looks like now, and illustrating what you'd like the API call to do differently to make the reason available.

I suspect we'll want to find an approach that somehow:

1. Doesn't add new APIs
2. Doesn't require the caller to parse the exception message

I'm not sure what that could look like though.

Thanks!

[srikrishnag]

We're using the code as below:

try
{
    using (var entryStream = entry.Open())
    {
        entryStream.CopyTo(documentStream);
    }
}
catch (InvalidDataException)
{
    // When we get the invalidDataException **we can't distinguish this exception is caused by password protected error or 
  // other error**. 
}

[jeffhandley]

If your intent is to still handle the exception and interrogate it to determine if it was caused by password protected error vs. another error, then we could consider using Exception.Data to get a flag passed through.

However, from a cursory peek through the implementation, I'm seeing that the native layer is surfacing the "unknown compression method" error that results in the InvalidDataException getting thrown. That leads me to believe the implementation of handling this in the exception path would not be trivial.

In the exception handling approach, we'd also need to think this through from a threat modeling perspective--would including a flag in the exception that indicates the archive was password protected allow anything to leak out that could be used maliciously? 🤔

Another approach would be through new APIs related to #1545 where we want to support password-protected zip archives. In the scenario you're describing, you don't know the password or encryption algorithm, so the specific APIs proposed on that issue would not suffice. I think this leads to the inevitable conclusion that you need a new API that exposes the encryption mode for the entry/archive. Then before calling entryStream.CopyTo, you'd check the property and bail out if it's password protected.

@carlossanlop Do you have other thoughts on how this could be approached?

[kasperk81]

my read is user wants System.NotSupportedException with message "Password protected archives are not supported", until #1545 is forged into a reality. this (parse-like analysis) can be done without new public api.

[jeffhandley]

NotSupportedException

The exception that is thrown when an invoked method is not supported, or when there is an attempt to read, seek, or write to a stream that does not support the invoked functionality.

I'm not sure NotSupportedException fits the scenario, nor do I see another existing exception type that fits. The invoked method is supported, and the stream supports the invoked functionality. Also, while we do occasionally change exception types (as breaking changes in new versions), we do so when we're able to make the exception type more accurate, which doesn't match what we'd be doing here.

Checking/parsing exception messages is a very brittle approach, and it's discouraged.

While I understand the motive for having a light-touch change to accomplish this, we also need to think about this from the "if we were designing this from scratch, how would we design it" angle. Then we can potentially look for a bridge. Since this is about control flow, I don't think we'd start with an exception-based design. "DO NOT use exceptions for the normal flow of control, if possible"

[danmoseley]

Internal request from 3 partners: https://dev.azure.com/devdiv/DevDiv/_workitems/edit/1486845

[jozkee]

I'm seeing that the native layer is surfacing the "unknown compression method" error that results in the InvalidDataException getting thrown

This is not always true, I have decompressed ZipCrypto (weak) encrypted entries without exception being thrown but the result is a garbled file content.

We should take into account if the solution to this problem may action a change in our existing one-liner APIs e.g: ZipFile.ExtractToDirectory.

[jeffhandley]

@srikrishnag - Can you review the API that was approved to confirm that it would meet your needs? It is a very minimal addition, simply adding a bool ZipArchiveEntry.IsEncrypted property.
[API Proposal]: Add property to ZipArchiveEntry to indicate if the entry is encrypted · Issue #68897 · dotnet/runtime

Please note that this new API would be available in .NET 7+.