Expect UnauthorizedAccessException in tvOS if extracting fifo from tar without elevation #71175

carlossanlop · 2022-06-23T00:05:15Z

I saw a tvOS failure in one of my PRs. I had not seen it before, so I'm unsure if it's intermittent or consistent.

The ExtractToFile_SpecialFile_Unelevated_Throws method threw UnauthorizedAccessException when attempting to extract the fifo from the archive into disk:

https://github.com/dotnet/runtime/blame/051b4828c7d3a0cad3289830ef9fd2120f45bb2b/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.ExtractToFile.Tests.Unix.cs#L39

In other Unix platforms, extracting fifos does not throw, but apparently it does on tvOS.

Here's the callstack:

[14:55:07.7170830] 2022-06-22 14:55:07.706 System.Formats.Tar.Tests[58680:78698627]    Exception messages: System.UnauthorizedAccessException : Access to the path '/private/var/mobile/Containers/Data/Application/11374FD2-B51C-49C4-844D-F40F062ACE94/tmp/1pslf5q1.gf2/output' is denied.
[14:55:07.7171130] ---- System.IO.IOException : Operation not permitted
[14:55:07.7189880] 2022-06-22 14:55:07.708 System.Formats.Tar.Tests[58680:78698627]    Exception stack traces:    at Interop.ThrowExceptionForIoErrno(ErrorInfo errorInfo, String path, Boolean isDirectory, Func`2 errorRewriter)
[14:55:07.7190220]    at System.Formats.Tar.TarEntry.ExtractAsFifo(String destinationFileName)
[14:55:07.7190300] 2022-06-22 14:55:07.708 System.Formats.Tar.Tests[58680:78698627]    at System.Formats.Tar.TarEntry.ExtractToFileInternal(String filePath, String linkTargetPath, Boolean overwrite)
[14:55:07.7190360]    at System.Formats.Tar.TarEntry.ExtractToFile(String destinationFileName, Boolean overwrite)
[14:55:07.7201670] 2022-06-22 14:55:07.709 System.Formats.Tar.Tests[58680:78698627]    at System.Formats.Tar.Tests.TarReader_ExtractToFile_Tests.ExtractToFile_SpecialFile_Unelevated_Throws()

ghost · 2022-06-23T00:05:27Z

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

Issue Details

I saw a tvOS failure in one of my PRs. I had not seen it before, so I'm unsure if it's intermittent or consistent.

The ExtractToFile_SpecialFile_Unelevated_Throws method threw UnauthorizedAccessException when attempting to extract the fifo from the archive into disk:

https://github.com/dotnet/runtime/blame/051b4828c7d3a0cad3289830ef9fd2120f45bb2b/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.ExtractToFile.Tests.Unix.cs#L39

In other Unix platforms, extracting fifos does not throw, but apparently it does on tvOS.

Here's the callstack:

[14:55:07.7170830] 2022-06-22 14:55:07.706 System.Formats.Tar.Tests[58680:78698627]    Exception messages: System.UnauthorizedAccessException : Access to the path '/private/var/mobile/Containers/Data/Application/11374FD2-B51C-49C4-844D-F40F062ACE94/tmp/1pslf5q1.gf2/output' is denied.
[14:55:07.7171130] ---- System.IO.IOException : Operation not permitted
[14:55:07.7189880] 2022-06-22 14:55:07.708 System.Formats.Tar.Tests[58680:78698627]    Exception stack traces:    at Interop.ThrowExceptionForIoErrno(ErrorInfo errorInfo, String path, Boolean isDirectory, Func`2 errorRewriter)
[14:55:07.7190220]    at System.Formats.Tar.TarEntry.ExtractAsFifo(String destinationFileName)
[14:55:07.7190300] 2022-06-22 14:55:07.708 System.Formats.Tar.Tests[58680:78698627]    at System.Formats.Tar.TarEntry.ExtractToFileInternal(String filePath, String linkTargetPath, Boolean overwrite)
[14:55:07.7190360]    at System.Formats.Tar.TarEntry.ExtractToFile(String destinationFileName, Boolean overwrite)
[14:55:07.7201670] 2022-06-22 14:55:07.709 System.Formats.Tar.Tests[58680:78698627]    at System.Formats.Tar.Tests.TarReader_ExtractToFile_Tests.ExtractToFile_SpecialFile_Unelevated_Throws()

Author:	carlossanlop
Assignees:	carlossanlop
Labels:	`area-System.IO`
Milestone:	7.0.0

carlossanlop · 2022-06-23T00:05:40Z

/azp run runtime-extra-platforms

azure-pipelines · 2022-06-23T00:05:54Z

Azure Pipelines successfully started running 1 pipeline(s).

danmoseley · 2022-06-23T00:34:43Z

Instead of wondering whether it's consistent or not, I suggest to leave the code as it is but wrap in a try catch for UnauthorizedAccessException and in the catch simply Assert that you're on tvOS.

carlossanlop · 2022-07-12T01:09:52Z

The file got renamed in my last PR, so I'll force push a new commit. Also, I realized the problem might be that this test is executed always, regardless if the process is or is not elevated. I should explicitly check if the process is not elevated as a ConditionalFact. I was able to repro this problem when executing the tests in Unix, both with a normal user and with sudo. There were some weird failures when running as sudo.

Instead of wondering whether it's consistent or not, I suggest to leave the code as it is but wrap in a try catch for UnauthorizedAccessException and in the catch simply Assert that you're on tvOS.

Sounds good. I'll do that too (if needed).

…uted in elevated mode.

carlossanlop · 2022-07-12T01:22:17Z

src/libraries/System.Formats.Tar/tests/TarReader/TarReader.TarEntry.ExtractToFile.Tests.Unix.cs

-        [Fact]
-        public async Task SpecialFile_Unelevated_Throws_Async()
+        [ConditionalFact(nameof(IsUnixButNotSuperUser))]
+        public void SpecialFile_Unelevated_Throws()


I noticed this test, which was supposed to be the synchronous one, was written as the asynchronous one.

carlossanlop · 2022-07-12T02:35:02Z

/azp run runtime-extra-platforms

azure-pipelines · 2022-07-12T02:35:23Z

Azure Pipelines successfully started running 1 pipeline(s).

adamsitnik

LGTM, thank you @carlossanlop

adamsitnik · 2022-07-12T06:53:49Z

...aries/System.Formats.Tar/tests/TarReader/TarReader.TarEntry.ExtractToFileAsync.Tests.Unix.cs

    {
        [PlatformSpecific(TestPlatforms.AnyUnix & ~TestPlatforms.tvOS)] // https://github.com/dotnet/runtime/issues/68360
-        [Fact]
+        [ConditionalFact(nameof(IsUnixButNotSuperUser))]


all the affected tests belong to *.Unix.cs files so we can assume that they are executed only on Unix and just use the existing IsSuperUser property:

Suggested change

[ConditionalFact(nameof(IsUnixButNotSuperUser))]

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsSuperUser))]

Right, but I need them to not be executed as a super user, ever. That's why I created the extra property for the conditional fact. I don't know if you can negate a conditional fact condition, can you?

The Unix can be removed, yes.

@carlossanlop you are right, I am sorry I've missed the negation

carlossanlop · 2022-07-12T17:10:14Z

The test failures are unrelated, which means the change worked. I can remove the Unix portion of the condition in my next PR with test fixes, @adamsitnik .

carlossanlop added the area-System.IO label Jun 23, 2022

carlossanlop added this to the 7.0.0 milestone Jun 23, 2022

carlossanlop self-assigned this Jun 23, 2022

carlossanlop added 2 commits July 11, 2022 18:15

Add ConditionalFact to 'unelevated' tests to ensure they are not exec…

26bf89f

…uted in elevated mode.

Fix test that was incorrectly executed async

bab22e7

carlossanlop marked this pull request as ready for review July 12, 2022 01:21

carlossanlop commented Jul 12, 2022

View reviewed changes

carlossanlop requested a review from adamsitnik July 12, 2022 01:22

runfoapp bot mentioned this pull request Jul 12, 2022

System.Runtime.InteropServices.JavaScript.Tests.JSImportExportTest.JSImportObject #71990

Closed

adamsitnik approved these changes Jul 12, 2022

View reviewed changes

carlossanlop merged commit cfe2731 into dotnet:main Jul 12, 2022

carlossanlop deleted the FixElevationTarFifo branch July 12, 2022 17:10

This was referenced Jul 14, 2022

matrix4x4createfromaxisangletest failing on NativeAOT #72149

Closed

System.Net.Http.Function.Tests and System.Net.Websockets.Client.Tests work items failing on runtime-extra-platforms Mono_Release runs #72209

Closed

karelz mentioned this pull request Jul 22, 2022

System.Threading.Tasks.Dataflow tests timing out on coreclr Linux_musl arm #71475

Closed

runfoapp bot mentioned this pull request Jul 22, 2022

System.Text.Json.SourceGeneration intermittent failures #72101

Closed

ghost locked as resolved and limited conversation to collaborators Aug 12, 2022

jeffhandley added area-System.Formats.Tar and removed area-System.IO labels Nov 21, 2022

	[ConditionalFact(nameof(IsUnixButNotSuperUser))]
	[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsSuperUser))]

Expect UnauthorizedAccessException in tvOS if extracting fifo from tar without elevation #71175

Expect UnauthorizedAccessException in tvOS if extracting fifo from tar without elevation #71175

Uh oh!

Conversation

carlossanlop commented Jun 23, 2022

Uh oh!

ghost commented Jun 23, 2022

Uh oh!

carlossanlop commented Jun 23, 2022

Uh oh!

azure-pipelines bot commented Jun 23, 2022

Uh oh!

danmoseley commented Jun 23, 2022

Uh oh!

carlossanlop commented Jul 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

carlossanlop Jul 12, 2022

Choose a reason for hiding this comment

Uh oh!

carlossanlop commented Jul 12, 2022

Uh oh!

azure-pipelines bot commented Jul 12, 2022

Uh oh!

adamsitnik left a comment

Choose a reason for hiding this comment

Uh oh!

adamsitnik Jul 12, 2022

Choose a reason for hiding this comment

Uh oh!

carlossanlop Jul 12, 2022

Choose a reason for hiding this comment

Uh oh!

carlossanlop Jul 12, 2022

Choose a reason for hiding this comment

Uh oh!

adamsitnik Jul 13, 2022

Choose a reason for hiding this comment

Uh oh!

carlossanlop commented Jul 12, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

carlossanlop commented Jul 12, 2022 •

edited

Loading