cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Mandar Barve" <mandar.ba...@sungard.com>
Subject Review Request 20117: Pilot patch for CS JIRA 6213
Date Tue, 08 Apr 2014 08:02:06 GMT

-----------------------------------------------------------
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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message