cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rajani Karuturi <Rajani.Karut...@citrix.com>
Subject Re: Findbugs report on 4.3-forward
Date Thu, 30 Jan 2014 04:36:33 GMT
I see that the commit 9776e1af1c92486f5081b1ee8fa95cf0edb86b97 is already pushed to 4.3. I
don’t see any response on my concern as well. 
Is it just me or anyone else sees a security issue with the generate password change?
Ian/Animesh/Daan, can you please respond? 

Thanks,
~Rajani



On 29-Jan-2014, at 10:59 am, Rajani Karuturi <Rajani.Karuturi@citrix.com> wrote:

> Hi Ian,
> Before it is pushed to 4.3, can you fix the generate password change like i suggested
in the other mail? This current change would make it less secure.
> 
> Thanks,
> ~Rajani
> 
> 
> 
> On 29-Jan-2014, at 8:03 am, Ian Duffy <ian@ianduffy.ie> wrote:
> 
>> Hi Animesh,
>> 
>> Tested all those changes to detail. Those lines were removed due to
>> unexpected behavior that I had not spotted until now.
>> 
>> They were suppose to allow for better fall over between multiple domain
>> controllers, how ever they were causing caching to occur. This meant if a
>> users password was reset in LDAP the old password was still allowing login
>> for a limited time.
>> 
>> Please pull the changes forward,
>> Thanks
>> 
>> Ian.
>> On 29 Jan 2014 00:07, "Animesh Chaturvedi" <animesh.chaturvedi@citrix.com>
>> wrote:
>> 
>>> If I look at this commit for example
>>> 
>>> 
>>> https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=commitdiff;h=92b4f66d73562e4211d2d787554ff229dbeb5705
>>> 
>>> It removes the two lines from LdapContextFactory.java
>>> 
>>> environment.put("com.sun.jndi.ldap.read.timeout", "500");-
>>> environment.put("com.sun.jndi.ldap.connect.pool", "true");
>>> 
>>> Is that reported by find bug? I don’t know this code  so not sure if it is
>>> intentional or not ?
>>> 
>>> The point is there may be unintended risks in allowing late changes.
>>> 
>>> 
>>> 
>>> -----Original Message-----
>>> From: Animesh Chaturvedi [mailto:animesh.chaturvedi@citrix.com]
>>> Sent: Tuesday, January 28, 2014 3:35 PM
>>> To: dev@cloudstack.apache.org
>>> Subject: RE: Findbugs report on 4.3-forward
>>> 
>>> Are you sure all of the ones are needed. A quick look at 20+ commits from
>>> Daan  show many formatting changes that may not be necessary and hinder
>>> quick review.
>>> 
>>> -----Original Message-----
>>> From: Hugo Trippaers [mailto:trippie@gmail.com]
>>> Sent: Tuesday, January 28, 2014 3:16 PM
>>> To: dev@cloudstack.apache.org
>>> Subject: Re: Findbugs report on 4.3-forward
>>> 
>>> 
>>> 
>>> Sent from my iPhone
>>> 
>>>> On 28 jan. 2014, at 23:50, Animesh Chaturvedi <
>>> animesh.chaturvedi@citrix.com> wrote:
>>>> 
>>>> 
>>>> -----Original Message-----
>>>> From: Hugo Trippaers [mailto:trippie@gmail.com]
>>>> Sent: Tuesday, January 28, 2014 2:37 PM
>>>> To: dev@cloudstack.apache.org
>>>> Cc: dev@cloudstack.apache.org
>>>> Subject: Re: Findbugs report on 4.3-forward
>>>> 
>>>> Hey Animesh,
>>>> 
>>>> Not in agreement here. These are squashed bugs and we want as less bugs
>>> in the release as possible.
>>>> [Animesh] I understand but once we enter RC phase we only limit
>>> important fixes. I have pulled in  2 commits from yours and 1 from Daan.
>>> 
>>> We limited our fixes to only the important issues that we found. The other
>>> 6000 issues between coverity and findbugs are still being triaged and will
>>> probably not make it into this release.
>>> 
>>>> 
>>>> This is why we test any RC before we release it.
>>>> [Animesh] Of course but timing is a bit off, if this was done a month
>>> back it would have been fine.
>>>> I say include all the big fixes we have in the release. If that means
>>> more testing before we cut the RC then that is what it is. I can't
>>> rightfully vote for a release with known issues with existing fixes.
>>>> [Animesh] Any release will have known issues, if we have fixes but can't
>>> be sure on regression impact then we have to make a choice.
>>> 
>>> Agreed, but we just don't agree on what that choice should be yet ;-)
>>> 
>>>> Quality over release schedule would be my vote then.
>>>> [Animesh] But why so late? Why was this activity not planned early on? I
>>> have been reminding community to call out issues early on since like
>>> mid-December.
>>> 
>>> On November 4 I sent the mail to the dev list that static code analysis
>>> (coverity) found 6000+ issues that needed to be triaged. I worked on quite
>>> a few with my colleagues, but it's a big task for just the four of us.
>>> Findbugs just helped us to quickly identify the real scary issues among
>>> them.
>>> So I agree that the timing is less than ideal, but we should do our utmost
>>> best to ship the highest quality piece of software we can.
>>> 
>>>> 
>>>> Cheers,
>>>> Hugo
>>>> 
>>>> Sent from my iPhone
>>>> 
>>>>> On 28 jan. 2014, at 18:48, Animesh Chaturvedi <
>>> animesh.chaturvedi@citrix.com> wrote:
>>>>> 
>>>>> Folks these issues reported by find-bugs have existed for some time.
I
>>> am not confident in picking them up now for 4.3 as it may break code that
>>> assumed old way of working. We can take them up for 4.3 maintenance
>>> release. I wish we had done this exercise and not waited until now.
>>>>> 
>>>>> I will pick Hugo's commit for which he called -1.
>>>>> 
>>>>> Thanks
>>>>> Animesh
>>>>> 
>>>>> -----Original Message-----
>>>>> From: Trippie [mailto:trippie@gmail.com] On Behalf Of Hugo Trippaers
>>>>> Sent: Tuesday, January 28, 2014 1:29 AM
>>>>> To: dev
>>>>> Subject: Re: Findbugs report on 4.3-forward
>>>>> 
>>>>> Hey Animesh,
>>>>> 
>>>>> I agree with Daan here. We focussed on the bugs with a findbugs
>>> annotation of scariest. I think that would warrant them to be included in
>>> the 4.3 release, so please cherry-pick them all.
>>>>> 
>>>>> 
>>>>> Cheers,
>>>>> 
>>>>> Hugo
>>>>> 
>>>>>> On 28 jan. 2014, at 09:32, Daan Hoogland <daan.hoogland@gmail.com>
>>> wrote:
>>>>>> 
>>>>>> Animesh,
>>>>>> 
>>>>>> I took up a lot of messages from findbugs in the server package over
>>>>>> the weekend. Not that I will attach my soul to the shipping of my
>>>>>> fixes but some of them are == vs eq and some are really nasty
>>>>>> nullpointer issues (a chack after first use is very common). You
can
>>>>>> cherry-pick them or not. I don't think you should leave any of them
>>>>>> behind. Even with David being right we should go against him at
>>>>>> every convenient time, running the risk of being called his wife.
>>>>>> 
>>>>>>> On Tue, Jan 28, 2014 at 6:25 AM, Ian Duffy <ian@ianduffy.ie>
wrote:
>>>>>>> Hi Animesh,
>>>>>>> 
>>>>>>> Can you cherry-pick the below commit from from 4.3-forward to
4.3
>>> branch?
>>>>>>> 
>>>>>>> Fix findbug issues within LDAP authenticator commit
>>>>>>> 92b4f66d73562e4211d2d787554ff229dbeb5705
>>>>>>> 
>>>>>>> Thanks,
>>>>>>> Ian
>>>>>>> 
>>>>>>> On 28 January 2014 03:48, Animesh Chaturvedi
>>>>>>> <animesh.chaturvedi@citrix.com>wrote:
>>>>>>> 
>>>>>>>> Hugo I was reviewing your commits to 4.3-forward and looked
at
>>>>>>>> your commits
>>>>>>>> 
>>>>>>>> 
>>>>>>>> https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=commit;
>>>>>>>> h = f18c5a1910b6370585a1d61638b8310c3ecba5ef
>>>>>>>> 
>>>>>>>> https://git-wip-us.apache.org/repos/asf?p=cloudstack.git;a=commit;
>>>>>>>> h = 60ac12780bfa1604902a89d5dc7937a8b9334e0d
>>>>>>>> I think you want the last one which has fixes for NetUtils
and
>>>>>>>> XenServerStorageMotionStrategy for which you had put -1 in
first
>>>>>>>> RC but the commit includes more files. Can you make limited
>>>>>>>> changes directly to 4.3? I want to build another RC later
tonight
>>>>>>>> 
>>>>>>>> Animesh
>>>>>>>> 
>>>>>>>> 
>>>>>>>> -----Original Message-----
>>>>>>>> From: Animesh Chaturvedi [mailto:animesh.chaturvedi@citrix.com]
>>>>>>>> Sent: Monday, January 27, 2014 1:30 PM
>>>>>>>> To: dev@cloudstack.apache.org
>>>>>>>> Subject: RE: Findbugs report on 4.3-forward
>>>>>>>> 
>>>>>>>> Agreed
>>>>>>>> 
>>>>>>>> We need to fix the most important ones for 4.3. There may
be
>>>>>>>> assumptions in the code which we may not know and may get
broken
>>>>>>>> if these issues are fixed late. I will pull in the one Hugo
casted
>>>>>>>> his
>>>>>>>> -1 for the first vote, any others?
>>>>>>>> 
>>>>>>>> Animesh
>>>>>>>> 
>>>>>>>> -----Original Message-----
>>>>>>>> From: David Nalley [mailto:david@gnsa.us]
>>>>>>>> Sent: Monday, January 27, 2014 11:46 AM
>>>>>>>> To: dev@cloudstack.apache.org
>>>>>>>> Subject: Re: Findbugs report on 4.3-forward
>>>>>>>> 
>>>>>>>> So just curious if I am the only one concerned about a ton
of
>>>>>>>> fixes going in at the last minute. If the fixes are for serious
>>>>>>>> bugs and we have consensus around their severity being high
enough,
>>> indeed lets fix things.
>>>>>>>> My concern is that much of the QA we do is manual; and while
we
>>>>>>>> are getting better; fixing tons of things at the last minute
may
>>>>>>>> have unintended consequences that we don't know about and
won't
>>> easily find.
>>>>>>>> 
>>>>>>>> I yearn for the day when our automated testing is broad enough
>>>>>>>> that we can do fixes right up to the wire and know that things
>>>>>>>> still work, I am just not sure that I have confidence that
we are
>>> there yet.
>>>>>>>> Thoughts? I am being paranoid?
>>>>>>>> 
>>>>>>>> --David
>>>>>>>> 
>>>>>>>> On Mon, Jan 27, 2014 at 3:11 AM, Daan Hoogland
>>>>>>>> <daan.hoogland@gmail.com>
>>>>>>>> wrote:
>>>>>>>>> Animesh, I commented the once i made yesterday with findbugs:
>>>>>>>>> 
>>>>>>>>> I allready send a few and will get you a list of the
rest later
>>> today.
>>>>>>>>> 
>>>>>>>>> regards,
>>>>>>>>> 
>>>>>>>>> On Mon, Jan 27, 2014 at 3:48 AM, Animesh Chaturvedi
>>>>>>>>> <animesh.chaturvedi@citrix.com> wrote:
>>>>>>>>>> Good job fellas. I see a number of commits 20+ into
4.3-forward
>>> branch.
>>>>>>>> Are their specific commits you want me to pick up out of
these?
>>>>>>>>>> 
>>>>>>>>>> Animesh
>>>>>>>>>> 
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: Daan Hoogland [mailto:daan.hoogland@gmail.com]
>>>>>>>>>> Sent: Sunday, January 26, 2014 2:41 AM
>>>>>>>>>> To: dev
>>>>>>>>>> Subject: Re: Findbugs report on 4.3-forward
>>>>>>>>>> 
>>>>>>>>>> I didn't get very far last night and will be looking
at the
>>>>>>>>>> server
>>>>>>>> package again this afternoon.
>>>>>>>>>> 
>>>>>>>>>> bon appétit,
>>>>>>>>>> 
>>>>>>>>>>> On Sun, Jan 26, 2014 at 1:36 AM, Ian Duffy <ian@ianduffy.ie>
>>> wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>> 
>>>>>>>>>>> Fixed the issues highlighted in the ldap user
authentication
>>> package.
>>>>>>>>>>> 
>>>>>>>>>>> Have pushed to 4.3-forward.
>>>>>>>>>>> 
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Ian
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> On 25 January 2014 22:26, Daan Hoogland
>>>>>>>>>>> <daan.hoogland@gmail.com>
>>>>>>>> wrote:
>>>>>>>>>>> 
>>>>>>>>>>>>> or reply to this mail with the filename
you are working on
>>>>>>>>>>>> I'll be looking at the server package as
it seems to contain
>>>>>>>>>>>> the most issues.
>>>>>>>>>>>> 
>>>>>>>>>>>> On Sat, Jan 25, 2014 at 4:00 PM, Hugo Trippaers
>>>>>>>>>>>> <hugo@trippaers.nl>
>>>>>>>> wrote:
>>>>>>>>>>>>> I've also added a job to master with
the Findbugs report and
>>>>>>>>>>>>> the
>>>>>>>>>>>> cobertura code coverage report.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Good stuff, we have a 12% coverage of
our classes with unit
>>> tests.
>>>>>>>>>>>>> Huge
>>>>>>>>>>>> improvement over the last release where we
had 4% iirc. We
>>>>>>>>>>>> have
>>>>>>>>>>>> 306 reports from Findbugs, of which the majority
are
>>>>>>>>>>>> internationalization
>>>>>>>> issues.
>>>>>>>>>>>> (String.getBytes without charset mostly).
On the coverity site
>>>>>>>>>>>> we have
>>>>>>>>>>>> 6000+ issues still open, but at least that
number is
>>>>>>>>>>>> 6000+ relatively stable, we
>>>>>>>>>>>> fix as much issues as we introduce and it's
untuned so we can
>>>>>>>>>>>> assume a large number of false positives
there.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> I think that on average the automated
tools tell us that code
>>>>>>>>>>>>> quality is
>>>>>>>>>>>> improving, which a good thing. Combined with
the functional
>>>>>>>>>>>> testing and the simulator build we can prove
that we are doing
>>>>>>>>>>>> quite well on the code quality angle.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> http://jenkins.buildacloud.org/job/build-master-slowbuild/
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Hugo
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>> On 25 jan. 2014, at 14:13, Daan Hoogland
>>>>>>>>>>>>> <daan.hoogland@gmail.com>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>> 
>>>>>>>>>>>>>> H Hugo,
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> I'll spend some time on it tonight.
Do you have a work load
>>>>>>>>>>>>>> distribution scheme or is it random
access?
>>>>>>>>>>>>>> ;)
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> regards
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> On Sat, Jan 25, 2014 at 12:39 PM,
Hugo Trippaers
>>>>>>>>>>>>>> <hugo@trippaers.nl>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>> Hey all,
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> I've made Jenkins run the findbugs
analysis on 4.3-forward.
>>>>>>>>>>>>>>> Is there
>>>>>>>>>>>> somebody who is willing to help triage the
findings? Maybe
>>>>>>>>>>>> there is some stuff that we need to fix?
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> the url is
>>>>>>>>>>>> http://jenkins.buildacloud.org/job/cloudstack-4.3-forward-mave
>>>>>>>>>>>> n
>>>>>>>>>>>> -
>>>>>>>>>>>> bui
>>>>>>>>>>>> ld
>>>>>>>>>>>> /3/findbugsResult/
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> Hugo
>>>>> 
>>> 
> 


Mime
View raw message