Conversation
|
Congratulations on your first Pull Request and welcome to the Apache CloudStack community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/cloudstack/blob/main/CONTRIBUTING.md)
|
| DataStoreTO store = safeCmd.getDataStore(); | ||
| if (store instanceof S3TO) { | ||
| ((S3TO) store).setAccessKey("***REDACTED***"); | ||
| ((S3TO) store).setSecretKey("***REDACTED***"); | ||
| } | ||
| logger.debug(LogUtils.logGsonWithoutException("Executing command %s [%s].", safeCmd.getClass().getSimpleName(), safeCmd)); |
There was a problem hiding this comment.
code looks good, but it seems this should be in LogUtils. There is other obfuscation code also scattered across the code base, so definately not a 👎 but a mere suggestion.
0dedb70 to
a43d34c
Compare
...er/src/test/java/org/apache/cloudstack/storage/resource/NfsSecondaryStorageResourceTest.java
Outdated
Show resolved
Hide resolved
…udstack/storage/resource/NfsSecondaryStorageResourceTest.java
|
@blueorangutan LLpackage |
|
@DaanHoogland a [LL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #10811 +/- ##
=============================================
- Coverage 16.57% 3.91% -12.67%
=============================================
Files 5745 415 -5330
Lines 510847 33793 -477054
Branches 62140 6078 -56062
=============================================
- Hits 84696 1322 -83374
+ Misses 416677 32313 -384364
+ Partials 9474 158 -9316
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Packaging result [LL]: ✖️ el7 ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 6219 |
|
@blueorangutan package |
|
@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 14149 |
|
@jerome079 can you have a look at this PR? |
|
This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch. |
|
@jerome079 can you resolve the conflicts and retarget the PR to the 4.22 branch? |
@jerome079 @rajujith , I think if any retargeting for this one is done it should be for 4.20 (20.3 release) |
Description
This PR addresses a security issue where S3 credentials used for Secondary Storage were being logged in plain text in CloudStack logs (
access.logandmanagement-server.log). Even when debug logging is enabled, secret credentials such asaccessKeyandsecretKeyshould never appear in logs.Fix details:
accessKeyandsecretKeyfrom theS3TOobject before loggingDownloadCommandinNfsSecondaryStorageResource.java.NfsSecondaryStorageResourceTest.javato verify that credentials are redacted.Steps to reproduce the issue:
/var/log/cloudstack/management/access.logormanagement-server.log— credentials will be printed.Fixes: #10339
Types of changes
Bug Severity
How Has This Been Tested?
S3TOand verifies thatsetAccessKey("***REDACTED***")andsetSecretKey("***REDACTED***")are called duringexecuteRequest.