Skip to content

Conversation

@sumangala-patki
Copy link
Contributor

@sumangala-patki sumangala-patki commented Mar 31, 2021

Delegation SAS tokens are created using various parameters for specifying details such as permissions and validity. The requests are logged, along with values of all the query parameters. This change will partially mask values logged for the following object IDs representing the security principal: skoid, saoid, suoid

@steveloughran steveloughran self-requested a review March 31, 2021 15:53
@steveloughran
Copy link
Contributor

this security/privacy issues, or just due diligence?

@sumangala-patki
Copy link
Contributor Author

Hi @steveloughran this is for privacy (no issue yet); the values masked identify the security principal (user/app)

public void maskSASObjectIDs() {
int oidStartIdx, ampIdx, oidEndIndex, qpStrIdx;
for (String qpKey : SAS_OID_PARAM_KEYS) {
qpStrIdx = maskedUrl.indexOf('&' + qpKey);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this.maskedUrl to be consistent with rest of file

}

public void maskSASObjectIDs() {
int oidStartIdx, ampIdx, oidEndIndex, qpStrIdx;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move to point of first use

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@sumangala-patki
Copy link
Contributor Author

TEST RESULTS

HNS Account Location: East US 2
NonHNS Account Location: East US 2, Central US

HNS OAuth

[INFO] Tests run: 93, Failures: 0, Errors: 0, Skipped: 0
[WARNING] Tests run: 512, Failures: 0, Errors: 0, Skipped: 71
[WARNING] Tests run: 261, Failures: 0, Errors: 0, Skipped: 50

HNS SharedKey

[INFO] Tests run: 93, Failures: 1, Errors: 0, Skipped: 0
[WARNING] Tests run: 506, Failures: 0, Errors: 0, Skipped: 26 
[ERROR] Errors: ITestAbfsFileSystemContractSecureDistCp>AbstractContractDistCpTest.testDistCpWithIterator:631 ? TestTimedOut
[INFO]
[ERROR] Tests run: 251, Failures: 0, Errors: 1, Skipped: 40

Non-HNS SharedKey

[INFO] Tests run: 93, Failures: 1, Errors: 0, Skipped: 0
[WARNING] Tests run: 506, Failures: 0, Errors: 0, Skipped: 249
[WARNING] Tests run: 261, Failures: 0, Errors: 0, Skipped: 40

if (maskedEncodedUrl != null) {
return maskedEncodedUrl;
}
return UriUtils.encodedUrlStr(getMaskedUrl());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assign the value to this.maskedEncodedUrl, then return the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

@bilaharith bilaharith left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the comments

@sumangala-patki
Copy link
Contributor Author

TEST RESULTS

HNS Account Location: East US 2
NonHNS Account Location: East US 2, Central US

HNS-OAuth

[INFO] Tests run: 95, Failures: 0, Errors: 0, Skipped: 0
[WARNING] Tests run: 494, Failures: 0, Errors: 0, Skipped: 71
[WARNING] Tests run: 261, Failures: 0, Errors: 0, Skipped: 50

HNS-SharedKey

[INFO] Tests run: 95, Failures: 0, Errors: 0, Skipped: 0
[WARNING] Tests run: 503, Failures: 0, Errors: 0, Skipped: 26
[WARNING] Tests run: 261, Failures: 0, Errors: 0, Skipped: 40

NonHNS-SharedKey

[INFO] Tests run: 95, Failures: 0, Errors: 0, Skipped: 0
[WARNING] Tests run: 506, Failures: 0, Errors: 0, Skipped: 249
[WARNING] Tests run: 261, Failures: 0, Errors: 0, Skipped: 40

Copy link
Contributor Author

@sumangala-patki sumangala-patki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

check comments

@apache apache deleted a comment from hadoop-yetus Apr 22, 2021
@apache apache deleted a comment from hadoop-yetus Apr 22, 2021
@sumangala-patki
Copy link
Contributor Author

TEST RESULTS

HNS Account Location: East US 2
NonHNS Account Location: East US 2, Central US

HNS OAuth

[INFO] Tests run: 99, Failures: 0, Errors: 0, Skipped: 0
[WARNING] Tests run: 519, Failures: 0, Errors: 0, Skipped: 70
[WARNING] Tests run: 261, Failures: 0, Errors: 0, Skipped: 50

HNS SharedKey

[INFO] Tests run: 99, Failures: 0, Errors: 0, Skipped: 0
[WARNING] Tests run: 507, Failures: 0, Errors: 0, Skipped: 26
[WARNING] Tests run: 251, Failures: 0, Errors: 0, Skipped: 40

Non-HNS SharedKey

[INFO] Tests run: 99, Failures: 0, Errors: 0, Skipped: 0
[WARNING] Tests run: 519, Failures: 0, Errors: 0, Skipped: 248
[ERROR] Errors:
[ERROR]   ITestAbfsReadWriteAndSeek.testReadAndWriteWithDifferentBufferSizesAndSeek:62->testReadWriteAndSeek:78 ? TestTimedOut
[ERROR] Tests run: 261, Failures: 0, Errors: 1, Skipped: 40

JIRA to track timeout failure: ITestAbfsReadWriteAndSeek

@sumangala-patki sumangala-patki marked this pull request as ready for review April 23, 2021 17:05
@apache apache deleted a comment from hadoop-yetus Apr 27, 2021
@apache apache deleted a comment from hadoop-yetus Apr 27, 2021
@sumangala-patki
Copy link
Contributor Author

Hi @steveloughran, thanks for the review. Have addressed the comments, please take a look. Thank you!

@apache apache deleted a comment from hadoop-yetus Aug 4, 2021
@apache apache deleted a comment from hadoop-yetus Aug 4, 2021
@apache apache deleted a comment from hadoop-yetus Aug 4, 2021
@apache apache deleted a comment from hadoop-yetus Aug 4, 2021
Copy link
Contributor

@steveloughran steveloughran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with this.

@sumangala-patki -can you rebase/merge with trunk and push up for yetus to recheck?

+1 pending yetus being happy

@sumangala-patki
Copy link
Contributor Author

TEST RESULTS

HNS Account Location: East US 2
NonHNS Account Location: East US 2, Central US

AppendBlob-HNS-OAuth

[WARNING] Tests run: 105, Failures: 0, Errors: 0, Skipped: 1
[ERROR] Failures: 
[ERROR]   ITestAbfsStreamStatistics.testAbfsStreamOps:140->Assert.assertTrue:42->Assert.fail:89 The actual value of 99 was not equal to the expected value
[ERROR] Errors: 
[ERROR]   ITestAzureBlobFileSystemLease.testTwoWritersCreateAppendNoInfiniteLease:178->twoWriters:166 » IO
[ERROR] Tests run: 558, Failures: 1, Errors: 1, Skipped: 98
[WARNING] Tests run: 259, Failures: 0, Errors: 0, Skipped: 76

HNS-OAuth

[WARNING] Tests run: 105, Failures: 0, Errors: 0, Skipped: 1
[WARNING] Tests run: 558, Failures: 0, Errors: 0, Skipped: 98
[WARNING] Tests run: 259, Failures: 0, Errors: 0, Skipped: 52

HNS-SharedKey

[WARNING] Tests run: 105, Failures: 0, Errors: 0, Skipped: 2
[ERROR] Failures: 
[ERROR]   ITestAbfsRestOperationException.testCustomTokenFetchRetryCount:93->testWithDifferentCustomTokenFetchRetry:122->Assert.assertTrue:42->Assert.fail:89 Number of token fetch retries (4) done, does not match with fs.azure.custom.token.fetch.retry.count configured (0)
[ERROR] Tests run: 558, Failures: 1, Errors: 0, Skipped: 54
[WARNING] Tests run: 259, Failures: 0, Errors: 0, Skipped: 40

NonHNS-SharedKey

[ERROR] Failures: 
[ERROR]   TestAbfsClientThrottlingAnalyzer.testManySuccessAndErrorsAndWaiting:171->fuzzyValidate:49 The actual value 16 is not within the expected range: [5.60, 8.40].
[ERROR] Tests run: 105, Failures: 1, Errors: 0, Skipped: 2
[WARNING] Tests run: 558, Failures: 0, Errors: 0, Skipped: 276
[WARNING] Tests run: 259, Failures: 0, Errors: 0, Skipped: 40

JIRAs to track failures: Appendblob lease, TestAbfsClientThrottlingAnalyzer, StreamOps

@steveloughran steveloughran merged commit 3450522 into apache:trunk Aug 4, 2021
@steveloughran
Copy link
Contributor

thanks, merged to trunk. Do a test run after cherrypicking to 3.3 and we can merge it there too

sumangala-patki added a commit to sumangala17/hadoop that referenced this pull request Aug 6, 2021
…e#2845)

Contributed by Sumangala Patki

(cherry picked from commit 3450522)
@apache apache deleted a comment from hadoop-yetus Aug 6, 2021
@apache apache deleted a comment from hadoop-yetus Aug 6, 2021
@apache apache deleted a comment from hadoop-yetus Aug 6, 2021
@apache apache deleted a comment from hadoop-yetus Aug 6, 2021
@apache apache deleted a comment from hadoop-yetus Aug 6, 2021
@apache apache deleted a comment from hadoop-yetus Aug 6, 2021
@apache apache deleted a comment from hadoop-yetus Aug 6, 2021
@apache apache deleted a comment from hadoop-yetus Aug 6, 2021
@apache apache deleted a comment from hadoop-yetus Aug 17, 2021
steveloughran pushed a commit that referenced this pull request Sep 9, 2021
Contributed by Sumangala Patki

(cherry picked from commit 3450522)
kiran-maturi pushed a commit to kiran-maturi/hadoop that referenced this pull request Nov 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants