Do you mean at init walk the list of "sensitive" classes somehow, followed by "sensitive" Params and build the REGEX to be used at run time? Basically split the existing logic into 2 parts? That sounds like a good idea. I will need to find out how to do it but sounds doable.
Thanks, Mandar On Mon, Apr 14, 2014 at 10:45 AM, Daan Hoogland <daan.hoogl...@gmail.com>wrote: > How about augmenting on loadtime? > > mobile bilingual spell checker used > Op 14 apr. 2014 07:06 schreef "Mandar Barve" <mandar.ba...@sungardas.com>: > >> This solution for which I have posted a pilot patch has following >> potential >> drawbacks: >> >> 1. For a sensitive API we need to load all "Param/Parameter" annotations >> iteratively. This can be time consuming. >> 2. We also have to iterate multiple times in the cleanString utility >> function ensuring every identified sensitive keyword is stripped. >> 3. This adds multiple iterations in the code path for stripping sensitive >> data. >> >> Other potential solution to think about could be: >> 1. Augment the list of "hard coded" keywords with what we know as the >> additional sensitive keywords (by carefully going through various response >> key words, which will be required either ways). Hopefully this won't come >> out to be too big a list. >> 2. Device a scheme of tagging sensitive API request/response parameters >> with a well known prefix or a suffix. The filter REGEX can be augmented >> further to look for such sub strings. This can remove the need for >> iterative code. >> >> Thanks, >> Mandar >> >> >> On Tue, Apr 8, 2014 at 1:32 PM, Mandar Barve <mandar.ba...@sungard.com >> >wrote: >> >> > >> > ----------------------------------------------------------- >> > 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 >> > >> > >> >
