HDFS-16563. Namenode WebUI prints sensitive information on Token expiry by prasad-acit · Pull Request #4241 · apache/hadoop

prasad-acit · 2022-04-27T17:55:21Z

Description of PR

How was this patch tested?

For code changes:

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

prasad-acit · 2022-04-27T18:00:26Z

@hemanthboyina @Hexiaoqiao Can you please review the PR when you get time?

hemanthboyina · 2022-04-28T04:59:00Z

thank you @prasad-acit for the PR, looks the test case failures are related, can you have a look once

Hexiaoqiao · 2022-04-28T05:14:31Z

@prasad-acit Thanks for involving me here. IMO, the key and sensitive information is DelegationKey/Password for DelegationToken, the output message here does not include this information right? So I don't think it it security issue. Do you mind to more information about this output or stack demo? Thanks.

steveloughran · 2022-04-28T09:56:57Z

looks like there are some big assumptions about the nested stack trace coming back. so please restore that change and see what happens

if the issue is that toString leaks a secret, it should be fixed at that level, as it is likely to end up in logs. we don't want any output to expose secrets.

prasad-acit · 2022-04-28T19:52:46Z

Thanks @hemanthboyina @Hexiaoqiao @steveloughran for the quick review & feedback.

the key and sensitive information is DelegationKey/Password for DelegationToken, the output message here does not include this information right?

Yes, there is no password printed in it. But as per our internal security guidelines displaying the complete Token info is also prohibited. So, suppressed the token from being displayed in the browser.

if the issue is that toString leaks a secret, it should be fixed at that level, as it is likely to end up in logs. we don't want any output to expose secrets.

Logging exception or full stack has no issue in this case. We are trying to avoid the token in the browser and keep the message abstract to the end-user. Here additional information is not necessary which can be avoided in the browser.

Failed tests corrected, please review the changes.

jojochuang

It was quite useful to find the root cause of a few token renewal bugs (KMS, YARN.. etc) back in the days. If it doesn't actually expose token secrets, would it make sense to at least log the token id at debug level?

prasad-acit · 2022-04-29T06:38:03Z

Thanks @jojochuang I have added logs with Token Info.
I will consider other error / improvement scenario and analyze further.

prasad-acit · 2022-04-29T12:25:28Z

Javadoc issues are not related.
Checkstyle issue addressed & pushed.

hadoop-yetus · 2022-04-29T16:18:25Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	1m 7s		Docker mode activated.
			_ Prechecks _
+1 💚	dupname	0m 0s		No case conflicting files found.
+0 🆗	codespell	0m 2s		codespell 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	40m 11s		trunk passed
+1 💚	compile	25m 6s		trunk passed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04
+1 💚	compile	21m 46s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	1m 30s		trunk passed
+1 💚	mvnsite	2m 1s		trunk passed
-1 ❌	javadoc	1m 37s	/branch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt	hadoop-common in trunk failed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.
+1 💚	javadoc	2m 5s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	3m 6s		trunk passed
+1 💚	shadedclient	25m 58s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	1m 7s		the patch passed
+1 💚	compile	30m 4s		the patch passed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04
+1 💚	javac	30m 4s		the patch passed
+1 💚	compile	23m 12s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	23m 12s		the patch passed
+1 💚	blanks	0m 1s		The patch has no blanks issues.
+1 💚	checkstyle	1m 25s		the patch passed
+1 💚	mvnsite	2m 0s		the patch passed
-1 ❌	javadoc	1m 27s	/patch-javadoc-hadoop-common-project_hadoop-common-jdkUbuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.txt	hadoop-common in the patch failed with JDK Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04.
+1 💚	javadoc	2m 4s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	3m 3s		the patch passed
+1 💚	shadedclient	25m 37s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	18m 11s		hadoop-common in the patch passed.
+1 💚	asflicense	1m 17s		The patch does not generate ASF License warnings.
		234m 26s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4241/4/artifact/out/Dockerfile
GITHUB PR	#4241
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell
uname	Linux 2b45026f7a30 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / 7579eafa0498b6a7fa49e10751c92ab4b7c42616
Default Java	Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Ubuntu-11.0.14.1+1-Ubuntu-0ubuntu1.20.04 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4241/4/testReport/
Max. process+thread count	2469 (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-4241/4/console
versions	git=2.25.1 maven=3.6.3 spotbugs=4.2.2
Powered by	Apache Yetus 0.14.0-SNAPSHOT https://yetus.apache.org

This message was automatically generated.

steveloughran · 2022-05-11T10:10:15Z

LGTM; will leave final vote to @jojochuang

prasad-acit · 2022-05-24T01:43:18Z

Thanks @steveloughran for the review.
@jojochuang can you please review the latest?

steveloughran

I'm happy with this; it's logging at info.

one minor change, just switching to the slf4j {} expansion strings. we should do this for all new log entries, -a lot of the existing stuff is a migration from the restricted commong logging api

steveloughran · 2022-05-30T12:55:45Z

...n/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java

can you use

LOG.info("{}, Token={}", err, formatTokenId(identifier));

steveloughran · 2022-05-30T12:55:57Z

...n/java/org/apache/hadoop/security/token/delegation/AbstractDelegationTokenSecretManager.java

can you use

LOG.warn("{}, Token={}", err, formatTokenId(identifier));

hadoop-yetus · 2022-06-02T23:56:16Z

💔 -1 overall

Vote	Subsystem	Runtime	Logfile	Comment
+0 🆗	reexec	1m 2s		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	40m 1s		trunk passed
+1 💚	compile	25m 22s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	compile	21m 55s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	checkstyle	1m 31s		trunk passed
+1 💚	mvnsite	1m 59s		trunk passed
+1 💚	javadoc	1m 32s		trunk passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	1m 5s		trunk passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	3m 5s		trunk passed
+1 💚	shadedclient	26m 14s		branch has no errors when building and testing our client artifacts.
			_ Patch Compile Tests _
+1 💚	mvninstall	1m 4s		the patch passed
+1 💚	compile	24m 27s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javac	24m 27s		the patch passed
+1 💚	compile	21m 48s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	javac	21m 48s		the patch passed
+1 💚	blanks	0m 0s		The patch has no blanks issues.
+1 💚	checkstyle	1m 23s		the patch passed
+1 💚	mvnsite	1m 57s		the patch passed
+1 💚	javadoc	1m 22s		the patch passed with JDK Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1
+1 💚	javadoc	1m 5s		the patch passed with JDK Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
+1 💚	spotbugs	3m 2s		the patch passed
+1 💚	shadedclient	26m 9s		patch has no errors when building and testing our client artifacts.
			_ Other Tests _
+1 💚	unit	18m 11s		hadoop-common in the patch passed.
+1 💚	asflicense	1m 17s		The patch does not generate ASF License warnings.
		226m 26s

Subsystem	Report/Notes
Docker	ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4241/5/artifact/out/Dockerfile
GITHUB PR	#4241
Optional Tests	dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient spotbugs checkstyle codespell detsecrets
uname	Linux 3b8faa10beee 4.15.0-175-generic #184-Ubuntu SMP Thu Mar 24 17:48:36 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
Build tool	maven
Personality	dev-support/bin/hadoop.sh
git revision	trunk / `98fe774`
Default Java	Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Multi-JDK versions	/usr/lib/jvm/java-11-openjdk-amd64:Private Build-11.0.15+10-Ubuntu-0ubuntu0.20.04.1 /usr/lib/jvm/java-8-openjdk-amd64:Private Build-1.8.0_312-8u312-b07-0ubuntu1~20.04-b07
Test Results	https://ci-hadoop.apache.org/job/hadoop-multibranch/job/PR-4241/5/testReport/
Max. process+thread count	1934 (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-4241/5/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.

steveloughran

+1, merging. thanks!

…ry (#4241) Contributed by Renukaprasad C Change-Id: I5cd2cec1dd79917f810207821b3bdf4fe1a5d24c

prasad-acit · 2022-06-07T04:07:41Z

Thanks @steveloughran

…ry (apache#4241) Contributed by Renukaprasad C

prasad-acit force-pushed the HDFS-16563 branch from b15218b to 9986c15 Compare April 28, 2022 19:34

jojochuang reviewed Apr 28, 2022

View reviewed changes

prasad-acit force-pushed the HDFS-16563 branch from 9986c15 to aac4044 Compare April 29, 2022 06:29

prasad-acit force-pushed the HDFS-16563 branch from aac4044 to 7579eaf Compare April 29, 2022 12:22

apache deleted a comment from hadoop-yetus May 11, 2022

apache deleted a comment from hadoop-yetus May 30, 2022

steveloughran requested changes May 30, 2022

View reviewed changes

HDFS-16563. Namenode WebUI prints sensitive information on Token expiry

98fe774

prasad-acit force-pushed the HDFS-16563 branch from 7579eaf to 98fe774 Compare June 2, 2022 20:08

steveloughran approved these changes Jun 3, 2022

View reviewed changes

steveloughran merged commit 7bd4ac3 into apache:trunk Jun 3, 2022

asfgit pushed a commit that referenced this pull request Jun 6, 2022

HDFS-16563. Namenode WebUI prints sensitive information on Token expi…

0c15daa

…ry (#4241) Contributed by Renukaprasad C Change-Id: I5cd2cec1dd79917f810207821b3bdf4fe1a5d24c

slfan1989 mentioned this pull request Jun 18, 2022

HADOOP-18284. Remove Unnecessary semicolon ';' #4422

Merged

steveloughran mentioned this pull request Jun 20, 2022

HADOOP-18305. Release Hadoop 3.3.4: minor update of hadoop-3.3.3 #4345

Closed

HarshitGupta11 pushed a commit to HarshitGupta11/hadoop that referenced this pull request Nov 28, 2022

HDFS-16563. Namenode WebUI prints sensitive information on Token expi…

9a44935

…ry (apache#4241) Contributed by Renukaprasad C

Conversation

prasad-acit commented Apr 27, 2022

Description of PR

How was this patch tested?

For code changes:

Uh oh!

prasad-acit commented Apr 27, 2022

Uh oh!

hemanthboyina commented Apr 28, 2022

Uh oh!

Hexiaoqiao commented Apr 28, 2022

Uh oh!

steveloughran commented Apr 28, 2022

Uh oh!

prasad-acit commented Apr 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jojochuang left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prasad-acit commented Apr 29, 2022

Uh oh!

prasad-acit commented Apr 29, 2022

Uh oh!

hadoop-yetus commented Apr 29, 2022

Uh oh!

steveloughran commented May 11, 2022

Uh oh!

prasad-acit commented May 24, 2022

Uh oh!

steveloughran left a comment

Choose a reason for hiding this comment

Uh oh!

steveloughran May 30, 2022

Choose a reason for hiding this comment

Uh oh!

steveloughran May 30, 2022

Choose a reason for hiding this comment

Uh oh!

hadoop-yetus commented Jun 2, 2022

Uh oh!

steveloughran left a comment

Choose a reason for hiding this comment

Uh oh!

prasad-acit commented Jun 7, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Comments

prasad-acit commented Apr 28, 2022 •

edited

Loading

jojochuang left a comment •

edited

Loading