Guava claims to use the apache 2.0 license on it's site. so that would not be a problem.
On Mon, Apr 14, 2014 at 7:33 PM, Alena Prokharchyk <alena.prokharc...@citrix.com> wrote: > 1. Agree with Daan. Putting the flag on the command level as a gate would > eliminate unneeded check for non-sensitive command¹s parameters. > Especially considering that majority of the commands are not sensitive. > > 2. I would really like some performance test to run on a top of the fix, > Mandar. As a result, I would like to see the difference between ³before > and after² ways. Also have you considered using Guava CharMatcher > https://code.google.com/p/guava-libraries/wiki/StringsExplained#CharMatcher > ? > > Daan, I¹m not really sure if the Guava libraries can be shipped under > Apache license, please advise on this part. > > Thanks, > Alena. > > On 4/14/14, 10:06 AM, "Daan Hoogland" <daan.hoogl...@gmail.com> wrote: > >>you got my drift. we could also replace the regex with a tree of flags >>to search that contains flags or method names. Not sure how that >>impacts performance. >> >>On Mon, Apr 14, 2014 at 11:14 AM, Mandar Barve >><mandar.ba...@sungardas.com> wrote: >>> 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.ja >>>>>va >>>>> > 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/Hyp >>>>>ervDirectConnectResource.java >>>>> > 12fc39d >>>>> > >>>>> > >>>>> > >>>>>plugins/storage/image/default/src/org/apache/cloudstack/storage/datasto >>>>>re/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 >>>>> > >>>>> > >>> >>> >> >> >> >>-- >>Daan > -- Daan
