HADOOP-19031. Enhance access control for RunJar. #6427

Hexiaoqiao · 2024-01-09T12:28:35Z

Description of PR

Make sure that the work directory of RunJar is only accessible by the current user.

How was this patch tested?

For code changes:

[Y] Does the title or this PR starts with the corresponding JIRA issue id (e.g. 'HADOOP-17799. Your PR title ...')?
[Y] Object storage: have the integration tests been executed and the endpoint declared according to the connector-specific documentation?
[Y] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
[Y] If applicable, have you updated the LICENSE, LICENSE-binary, NOTICE-binary files?

hadoop-yetus · 2024-01-09T14:37:08Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 21s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
-1 ❌	test4tests	0m 0s		The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
			_ trunk Compile Tests _
+1 💚	mvninstall	32m 0s		trunk passed
-1 ❌	compile	6m 12s	/branch-compile-root-jdkUbuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04.txt	root in trunk failed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04.
-1 ❌	compile	5m 39s	/branch-compile-root-jdkPrivateBuild-1.8.0_392-8u392-ga-1~20.04-b08.txt	root in trunk failed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08.
+1 💚	checkstyle	0m 38s		trunk passed
+1 💚	mvnsite	0m 53s		trunk passed
+1 💚	javadoc	0m 41s		trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	0m 28s		trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	spotbugs	1m 27s		trunk passed
+1 💚	shadedclient	20m 37s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 30s		the patch passed
+1 💚	compile	7m 54s		the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	javac	7m 54s		the patch passed
+1 💚	compile	7m 29s		the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	javac	7m 29s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 33s		the patch passed
+1 💚	mvnsite	0m 55s		the patch passed
+1 💚	javadoc	0m 35s		the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	0m 31s		the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	spotbugs	1m 27s		the patch passed
+1 💚	shadedclient	20m 22s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	15m 38s		hadoop-common in the patch passed.
+1 💚	asflicense	0m 33s		The patch does not generate ASF License warnings.
		127m 30s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6427/1/artifact/out/Dockerfile
GITHUB PR	#6427
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux b6d7aa2768bf 5.15.0-88-generic #98-Ubuntu SMP Mon Oct 2 15:18:56 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `85108b8`
Default Java	Private Build-1.8.0_392-8u392-ga-1~20.04-b08
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~20.04-b08
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6427/1/testReport/
Max. process+thread count	1290 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6427/1/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

ayushtkn

We are creating the directory & then setting the permissions, so there is some time b/w the two operations, theoritically where things can go nasty.

As of now we are using File.createTempFile("hadoop-unjar", "", tmpDir); can we explore using Files.createTempFile(path, null, "", PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("<perm>")));(If it works)? or just Files.createTempFile

The javadoc reads
The attrs parameter is optional file-attributes to set atomically when creating the file. Each attribute is identified by its name. If more than one attribute of the same name is included in the array then all but the last occurrence is ignored. When no file attributes are specified, then the resulting file may have more restrictive access permissions to files created by the File.createTempFile(String, String, File) method.
I guess it might do things atomically

Hexiaoqiao · 2024-01-10T04:12:36Z

@ayushtkn Thanks for your comments. I want to fix it simple and don't involve more changes since I didn't know the background of the current implement: 'create - delete - mkdir'.

We are creating the directory & then setting the permissions, so there is some time b/w the two operations, theoritically where things can go nasty.

True, this way may include some potential issue. Will try to update it later. Thanks.

zhangshuyan0 · 2024-01-11T12:42:44Z

I think we don't need consider atomicity here. The directory is created with permission 755 by default, so no one else can write any data to it. And because this is still an empty directory, no data can be read from it. @Hexiaoqiao @ayushtkn

Hexiaoqiao · 2024-01-15T08:11:58Z

Attach Javadoc about Files.createTempDirectory(). https://docs.oracle.com/javase/8/docs/api/java/nio/file/Files.html#createTempDirectory-java.nio.file.Path-java.lang.String-java.nio.file.attribute.FileAttribute...-

Considering that this method can loop generate random names until file or directory can be created, And invoked to check write access when creating the directory. So it didn't need to delete and mkdir again IMO. FYI.
cc @ayushtkn @slfan1989 @zhangshuyan0

hadoop-yetus · 2024-01-15T10:32:39Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 23s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
-1 ❌	test4tests	0m 0s		The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
			_ trunk Compile Tests _
+1 💚	mvninstall	31m 43s		trunk passed
+1 💚	compile	8m 17s		trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	compile	7m 39s		trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	checkstyle	0m 37s		trunk passed
+1 💚	mvnsite	0m 53s		trunk passed
+1 💚	javadoc	0m 40s		trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	0m 31s		trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	spotbugs	1m 26s		trunk passed
+1 💚	shadedclient	20m 27s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 27s		the patch passed
+1 💚	compile	7m 51s		the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	javac	7m 51s		the patch passed
+1 💚	compile	7m 30s		the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	javac	7m 30s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 38s		the patch passed
+1 💚	mvnsite	0m 50s		the patch passed
+1 💚	javadoc	0m 43s		the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	0m 33s		the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	spotbugs	1m 43s		the patch passed
+1 💚	shadedclient	24m 54s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
-1 ❌	unit	29m 19s	/patch-unit-hadoop-common-project_hadoop-common.txt	hadoop-common in the patch passed.
+1 💚	asflicense	0m 32s		The patch does not generate ASF License warnings.
		149m 56s

Reason	Tests
Failed junit tests	hadoop.ipc.TestCallQueueManager
	hadoop.ipc.TestSaslRPC
	hadoop.metrics2.source.TestJvmMetrics

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6427/2/artifact/out/Dockerfile
GITHUB PR	#6427
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 5486d97b37f0 5.15.0-88-generic #98-Ubuntu SMP Mon Oct 2 15:18:56 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `872cc23`
Default Java	Private Build-1.8.0_392-8u392-ga-1~20.04-b08
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~20.04-b08
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6427/2/testReport/
Max. process+thread count	1152 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6427/2/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

Hexiaoqiao · 2024-01-15T12:45:21Z

All failed unit tests are related with 'Caused by: java.lang.OutOfMemoryError: unable to create new native thread'. Any changes about CI recently? I am confused this PR didn't create any new thread here but why thread overload.

ayushtkn · 2024-01-16T05:48:24Z

this unable to create native thread is intermittent, can trigger the build again & it should be sorted...

if the build comes clean, the change looks good to me, I haven't tried locally, do give a check once before merging.

Hexiaoqiao · 2024-01-17T02:22:22Z

Trigger CI again, let's wait what it will say.

hadoop-yetus · 2024-01-17T04:35:10Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	0m 21s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 0s		codespell was not available.
+0 🆗	detsecrets	0m 0s		detect-secrets was not available.
+1 💚	@author	0m 0s		The patch does not contain any @author tags.
-1 ❌	test4tests	0m 0s		The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
			_ trunk Compile Tests _
+1 💚	mvninstall	32m 19s		trunk passed
+1 💚	compile	8m 17s		trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	compile	7m 28s		trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	checkstyle	0m 39s		trunk passed
+1 💚	mvnsite	0m 51s		trunk passed
+1 💚	javadoc	0m 41s		trunk passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	0m 30s		trunk passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	spotbugs	1m 26s		trunk passed
+1 💚	shadedclient	22m 15s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	0m 31s		the patch passed
+1 💚	compile	7m 56s		the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	javac	7m 56s		the patch passed
+1 💚	compile	7m 30s		the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	javac	7m 30s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	0m 37s		the patch passed
+1 💚	mvnsite	0m 55s		the patch passed
+1 💚	javadoc	0m 40s		the patch passed with JDK Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04
+1 💚	javadoc	0m 32s		the patch passed with JDK Private Build-1.8.0_392-8u392-ga-1~20.04-b08
+1 💚	spotbugs	1m 29s		the patch passed
+1 💚	shadedclient	20m 29s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	15m 50s		hadoop-common in the patch passed.
+1 💚	asflicense	0m 35s		The patch does not generate ASF License warnings.
		133m 36s

Subsystem	Report/Notes
Docker	ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6427/4/artifact/out/Dockerfile
GITHUB PR	#6427
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 7c820e23b6b5 5.15.0-88-generic #98-Ubuntu SMP Mon Oct 2 15:18:56 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `872cc23`
Default Java	Private Build-1.8.0_392-8u392-ga-1~20.04-b08
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.21+9-post-Ubuntu-0ubuntu120.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_392-8u392-ga-1~20.04-b08
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6427/4/testReport/
Max. process+thread count	3150 (vs. ulimit of 5500)
modules	C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
Console output	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-6427/4/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0 https://yetus.apache.org

This message was automatically generated.

… by He Xiaoqiao. Signed-off-by: Shuyan Zhang <[email protected]> Signed-off-by: Shilun Fan <[email protected]> Signed-off-by: Ayush Saxena <[email protected]> (cherry picked from commit 9634bd3)

Hexiaoqiao · 2024-01-17T07:09:03Z

Committed to trunk, branch-3.4, branch-3.4.0. Thanks @zhangshuyan0 @ayushtkn @slfan1989 .

…ibuted by He Xiaoqiao. Signed-off-by: Shuyan Zhang <[email protected]> Signed-off-by: Shilun Fan <[email protected]> Signed-off-by: Ayush Saxena <[email protected]>

… by He Xiaoqiao. Signed-off-by: Shuyan Zhang <[email protected]> Signed-off-by: Shilun Fan <[email protected]> Signed-off-by: Ayush Saxena <[email protected]> (cherry picked from commit 9634bd3)

…6427). Contributed by He Xiaoqiao. Signed-off-by: Shuyan Zhang <[email protected]> Signed-off-by: Shilun Fan <[email protected]> Signed-off-by: Ayush Saxena <[email protected]> Change-Id: Ic0ef7f58c8285c8e51e58156cd07e5a5c14074e4

…ibuted by He Xiaoqiao. Signed-off-by: Shuyan Zhang <[email protected]> Signed-off-by: Shilun Fan <[email protected]> Signed-off-by: Ayush Saxena <[email protected]>

… (apache#6427). Contributed by He Xiaoqiao. Signed-off-by: Shuyan Zhang <[email protected]> Signed-off-by: Shilun Fan <[email protected]> Signed-off-by: Ayush Saxena <[email protected]>

… (apache#6427). Contributed by He Xiaoqiao. Signed-off-by: Shuyan Zhang <[email protected]> Signed-off-by: Shilun Fan <[email protected]> Signed-off-by: Ayush Saxena <[email protected]> (cherry picked from commit 913a542)

Hexiaoqiao requested review from ayushtkn, slfan1989 and zhangshuyan0 January 9, 2024 12:28

github-actions bot added trunk Common labels Jan 9, 2024

ayushtkn reviewed Jan 9, 2024

View reviewed changes

zhangshuyan0 approved these changes Jan 11, 2024

View reviewed changes

slfan1989 approved these changes Jan 12, 2024

View reviewed changes

HADOOP-19031. Enhance access control for RunJar.

872cc23

Hexiaoqiao force-pushed the accesscontrol branch from 85108b8 to 872cc23 Compare January 15, 2024 08:01

zhangshuyan0 approved these changes Jan 16, 2024

View reviewed changes

ayushtkn approved these changes Jan 17, 2024

View reviewed changes

Hexiaoqiao merged commit 9634bd3 into apache:trunk Jan 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

HADOOP-19031. Enhance access control for RunJar. #6427

HADOOP-19031. Enhance access control for RunJar. #6427

Uh oh!

Hexiaoqiao commented Jan 9, 2024

Uh oh!

hadoop-yetus commented Jan 9, 2024

Uh oh!

ayushtkn left a comment

Uh oh!

Hexiaoqiao commented Jan 10, 2024

Uh oh!

zhangshuyan0 commented Jan 11, 2024

Uh oh!

Hexiaoqiao commented Jan 15, 2024

Uh oh!

hadoop-yetus commented Jan 15, 2024

Uh oh!

Hexiaoqiao commented Jan 15, 2024

Uh oh!

ayushtkn commented Jan 16, 2024

Uh oh!

Hexiaoqiao commented Jan 17, 2024

Uh oh!

hadoop-yetus commented Jan 17, 2024

Uh oh!

Hexiaoqiao commented Jan 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

HADOOP-19031. Enhance access control for RunJar. #6427

HADOOP-19031. Enhance access control for RunJar. #6427

Uh oh!

Conversation

Hexiaoqiao commented Jan 9, 2024

Description of PR

How was this patch tested?

For code changes:

Uh oh!

hadoop-yetus commented Jan 9, 2024

Uh oh!

ayushtkn left a comment

Choose a reason for hiding this comment

Uh oh!

Hexiaoqiao commented Jan 10, 2024

Uh oh!

zhangshuyan0 commented Jan 11, 2024

Uh oh!

Hexiaoqiao commented Jan 15, 2024

Uh oh!

hadoop-yetus commented Jan 15, 2024

Uh oh!

Hexiaoqiao commented Jan 15, 2024

Uh oh!

ayushtkn commented Jan 16, 2024

Uh oh!

Hexiaoqiao commented Jan 17, 2024

Uh oh!

hadoop-yetus commented Jan 17, 2024

Uh oh!

Hexiaoqiao commented Jan 17, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants