-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/20117/
-----------------------------------------------------------
Review request for cloudstack and Alena Prokharchyk.
Repository: cloudstack-git
Description
-------
Hi Alena,
I am attaching a pilot patch for the problem we are trying to solve. Please
let me know your thoughts, comments, suggestions on the approach and code. We
can make widespread code changes after agreeing on the approach and any other
details.
Problem: When stripping sensitive parameters from the response log string, the
strip logic should be generic. Current cleanString code strips few hardcoded
keywords from the response string.
Approach: As discussed in the context of CS JIRA 4406 I have modified
@Parameter annotation applied to fields of command classes and @Param
annotation applied to fields of response classes.
1. Annotations modified to add a new flag that conveys sensitivity of the
parameter for log, default set to false.
2. cleanString utility function is modified to process an array of strings
passed to it so it can strip all.
3. To keep this backward compatible (and also don't know the code change
implications at other places at this time) made sure old cleanString code will
continue to strip hardcoded keywords when zero sized filter array is passed.
This can change if we think so and when we think so. This change I am putting
is minimal focussed to solve current problem.
4. In ApiServer code where we are loading APICommand annotation to check if the
command response carried sensitive data, additional code is added to load
response class signature Param and SerializedName annotations to get the name
of the field that is flagged to carry sensitive information
5. A list of such names is built and passed to cleanString to strip
6. All code areas that got affected by cleanString signature change are
modified to pass zero sized filter arrays to cleanString
7. pom.xml is modified for server project to include gson dependency
8.StringUtil unit test code modified to use new signature for cleanString. (New
tests will need to be added in the final patch for testing the new functions of
cleanString)
On final note this addresses handling the audit logging of response strings
alone. I haven't looked into audit logging of request strings and what will
need to be done there.
This pilot patch is tested for listUsers command response. The code strips
apikey, secretkey and additional parameter userid (only meant for testing) as
they are tagged to be sensitive.
Thanks,
Mandar
Diffs
-----
api/src/com/cloud/serializer/Param.java 3e6f852
api/src/org/apache/cloudstack/api/Parameter.java 7ee6897
api/src/org/apache/cloudstack/api/command/user/vm/ResetVMPasswordCmd.java
d15ea47
api/src/org/apache/cloudstack/api/response/UserResponse.java 40e1561
core/src/com/cloud/agent/transport/Request.java b5890d9
plugins/hypervisors/hyperv/src/com/cloud/hypervisor/hyperv/resource/HypervDirectConnectResource.java
12fc39d
plugins/storage/image/default/src/org/apache/cloudstack/storage/datastore/lifecycle/CloudStackImageStoreLifeCycleImpl.java
7675e94
server/pom.xml 6e60fc4
server/src/com/cloud/api/ApiServer.java 42ac8b7
server/src/com/cloud/api/ApiServlet.java e78bf38
server/src/com/cloud/api/query/dao/ImageStoreJoinDaoImpl.java f1f873c
server/src/com/cloud/api/query/dao/StoragePoolJoinDaoImpl.java 1d89b19
utils/src/com/cloud/utils/StringUtils.java 1600488
utils/test/com/cloud/utils/StringUtilsTest.java 5a90300
Diff: https://reviews.apache.org/r/20117/diff/
Testing
-------
Tested the strip logic in the pilot patch for listUsers command response.
Thanks,
Mandar Barve
