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 Fri, 31 Jan 2014 04:38:56 GMT
With the char set know, it would be easy to decode the password than the previous encoded version.

This is a concern because even for ldap users we also check authentication against db. 

Thanks a lot for taking Daan and Ian. 


~Rajani



On 31-Jan-2014, at 1:20 am, Ian Duffy <ian@ianduffy.ie> wrote:

> Hi,
> 
> Sorry about the delay in replying to this have been doing exams at uni all
> week.
> 
> Daan's change looks to change Rajani concern.
> 
> Might be me being naive but I fail to understand the concern fully...
> The given character selection was roughly 72 with a string of length 20. If
> my math is correct thats 72^20 different possible combinations...
> 
> Anyways, thanks for taking care of this Daan.
> 
> On 30 January 2014 13:28, Daan Hoogland <daan.hoogland@gmail.com> wrote:
> 
>> Guys,
>> 
>> I found two reported issues of category 'scary'. To satify Rajuri's
>> concerns I would like to revert Ian's commit and checkin two changes
>> that change returning
>> byte[].toString()
>> into
>> Arrays.toString(byte[])
>> on return statements of generatePassword methods.
>> 
>> if no objections come in within a few...,
>> Daan
>> 
>> On Thu, Jan 30, 2014 at 1:53 PM, Daan Hoogland <daan.hoogland@gmail.com>
>> wrote:
>>> Animesh, Ian,
>>> 
>>> Can you comment on this?
>>> 
>>> I couldn't find any findbugs issues of the the scariest kind in
>>> yesterdays version of the 4.3 branch. What was solved that needs to go
>>> in in spite of Rajuri's reservations?
>>> 
>>> thanks,
>>> Daan
>>> 
>>> On Thu, Jan 30, 2014 at 11:21 AM, Daan Hoogland <daan.hoogland@gmail.com>
>> wrote:
>>>> Sorry Rajani,
>>>> 
>>>> I had seen no reaction to Ian's explenation  and the request by
>>>> Animesh to pull it so I just did. let me look into it for a minute
>>>> 
>>>> On Thu, Jan 30, 2014 at 5:36 AM, Rajani Karuturi
>>>> <Rajani.Karuturi@citrix.com> wrote:
>>>>> 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