apex-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vlad Rozov <vro...@apache.org>
Subject Re: checking dependencies for known vulnerabilities
Date Wed, 01 Nov 2017 18:13:48 GMT
Any other concerns regarding merging the PR? By looking at the active 
PRs on the apex core the entire conversation looks to be at the moot point.

Thank you,

Vlad

On 10/30/17 18:50, Vlad Rozov wrote:
> On 10/30/17 17:30, Pramod Immaneni wrote:
>> On Sat, Oct 28, 2017 at 7:47 AM, Vlad Rozov <vrozov@apache.org> wrote:
>>
>>> Don't we use unit test to make sure that PR does not break an existing
>>> functionality? For that we use CI environment that we do not control 
>>> and do
>>> not introduce any changes to, but for example Apache infrastructure 
>>> team
>>> may decide to upgrade Jenkins and that may impact Apex builds. The same
>>> applies to CVE. It is there to prevent dependencies with severe
>>> vulnerabilities.
>>>
>> Infrastructure changes are quite different, IMO, from this proposal. 
>> While
>> they are not in our control, in majority of the cases, the changes 
>> maintain
>> compatibility so everything on top will continue to run the same. In 
>> this
>> case a CVE will always fail all PRs, the code changes have nothing to do
>> with introducing the CVE. I did make the exception that if a PR is 
>> bringing
>> in the CVE yes do fail it.
> There were just two recent changes, one on Travis CI side and another 
> on Jenkins side that caused builds for all PRs to fail and none of 
> them was caused by code changes in any of open PRs, so I don't see how 
> it is different.
>
> A code change may or may not have relation to CVE introduced. For 
> newly introduced dependencies, there may be known CVEs. In any case I 
> don't think it is important to differentiate how CVE is introduced, it 
> is important to eliminate dependencies with known CVEs.
>>
>>
>>> There is no "stick" in a failed build or keeping PR open until 
>>> dependency
>>> issue is resolved or unit test failure is fixed. Unless an employer
>>> punishes its employee for not delivering PR based on that vendor 
>>> priority,
>>> there is no "stick". As we already discussed, the community does not 
>>> have a
>>> deadline for a PR merge or for a release to go out. In a case there 
>>> is a
>>> problematic dependency (with CVE or category X license) you as a PMC
>>> suppose to -1 a release (at least I will). Will you consider -1 as a
>>> "stick"? For me, it is not about punishing an individual 
>>> contributor, it is
>>> a priority and focus shift for the entire community, not a "stick" 
>>> for an
>>> individual contributor.
>>>
>> The stick I am referring to is failing all PRs hoping that will make 
>> people
>> address CVEs. It's got nothing to do with an employer, people 
>> contributing
>> to open source can't expect they can control what the outcome will be or
>> what form it will take. You may be confusing this with some other 
>> issue. In
>> some of the arguments, you are assuming this scenario is similar to 
>> build
>> failures from failing unit tests and I am arguing that premise. I don't
>> think we should bring regular development to a halt whenever a 
>> matching CVE
>> is discovered, unless there is some other secondary reason like 
>> merging a
>> PR will make it difficult for a CVE fix to be made. Have you given a
>> thought to what I said about having a separate build that will notify 
>> about
>> CVEs.
> As I mentioned, there is no stick, no deadlines and no issues keeping 
> PRs open until builds can be verified on CI environment. Fixing a 
> failed build is a priority for the *community* not a stick for an 
> individual contributor.
>
> I don't see why keeping PRs open (for whatever reason) brings regular 
> development to a halt. Nobody is going to put github repo offline. 
> Contributors may continue to open new PR, collaborate on existing PRs 
> and add more changes (and need to be patient for those changes to be 
> reviewed and accepted). The regular development will continue with the 
> only exception that the next commit to be merged must address the 
> build issue (whether it is a failed unit test or newly found CVE).
>
> I don't see much value in a separate build and do not plan to put 
> effort in that direction. Additionally, will not a separate build that 
> only checks for CVE will trigger your initial concern of disclosing 
> CVE in public?
>>
>> Thank you,
>>> Vlad
>>>
>>>
>>> On 10/27/17 14:28, Pramod Immaneni wrote:
>>>
>>>> On Fri, Oct 27, 2017 at 11:51 AM, Vlad Rozov <vrozov@apache.org> 
>>>> wrote:
>>>>
>>>> On 10/26/17 11:46, Pramod Immaneni wrote:
>>>>> On Thu, Oct 26, 2017 at 10:39 AM, Vlad Rozov <vrozov@apache.org>

>>>>> wrote:
>>>>>> I guess you are mostly concerned regarding new CVE in an existing
>>>>>>
>>>>>>> dependency. Once such CVE is added to a database, will it be

>>>>>>> better to
>>>>>>> know
>>>>>>> about it or postpone discovery till we cut release candidate?
In 
>>>>>>> case
>>>>>>> it
>>>>>>> is
>>>>>>> reported only during release cycle, there is much less time for
the
>>>>>>> community to take an action and it still needs to be taken (as
a 
>>>>>>> PMC
>>>>>>> member
>>>>>>> you are responsible for preventing release with severe security

>>>>>>> issue
>>>>>>> going
>>>>>>> out). If it is reported once the information becomes available,

>>>>>>> there
>>>>>>> is
>>>>>>> more time to research and either upgrade the dependency with
newly
>>>>>>> found
>>>>>>> CVE, agree that it does not impact the project.
>>>>>>>
>>>>>>> This would be the more commonly occurring scenario. We can 
>>>>>>> always know
>>>>>>>
>>>>>> about the CVEs because of your changes. We don't need to fail 
>>>>>> builds to
>>>>>> do
>>>>>> that. I am not asking you to remove the reporting. There is no 
>>>>>> set time
>>>>>> for
>>>>>> a release so having less time during release for addressing relevant
>>>>>> CVEs
>>>>>> does not come up. There is also nothing preventing anyone from 
>>>>>> looking
>>>>>> at
>>>>>> these reports and taking action earlier.
>>>>>>
>>>>>> I don't see why it will be more commonly occurring scenario, but

>>>>>> I think
>>>>> it is equally important to prevent new dependency with severe
>>>>> vulnerabilities being introduced into the project and check existing
>>>>> dependencies for newly discovered severe vulnerabilities.
>>>>>
>>>>> How will we know about CVE if it is removed from CI build? Why 
>>>>> require
>>>>> manual verification when it can be done during CI build and does not
>>>>> affect
>>>>> builds done by individual contributors?
>>>>>
>>>>> While there is no set time for a release, there is no set time for 
>>>>> a PR
>>>>> merge as well.
>>>>>
>>>>> Yes, nothing prevents anyone from looking at the dependency
>>>>> vulnerabilities, but there is a better chance that "anyone" does 
>>>>> not mean
>>>>> nobody if CI build fails.
>>>>>
>>>>>
>>>>>> I still do not understand why you value an individual contributor

>>>>>> and PR
>>>>>>
>>>>>>> over the community and the project itself. Once there is a severe
>>>>>>> security
>>>>>>> vulnerability, it affects everyone who cares about the project,
>>>>>>> including
>>>>>>> all contributors. I don't see a problem with a PR being in a

>>>>>>> pending
>>>>>>> (not
>>>>>>> merged) open state till a build issue is resolved.
>>>>>>>
>>>>>>> That is a mischaracterization that you have stated before as

>>>>>>> well. A
>>>>>>>
>>>>>> project cannot grow without contributions and without policies that
>>>>>> create
>>>>>> a supportive enviroment where that can happen, I don't see the 
>>>>>> need to
>>>>>> put
>>>>>> unnecessary obstacles for legitimate contributions that are not the
>>>>>> cause
>>>>>> of a problem. Everytime there is a matching CVE the PRs are going

>>>>>> to get
>>>>>> blocked till that CVE is addressed and I am not confident we have

>>>>>> the
>>>>>> bandwidth in the community to address this expediently. It is also
>>>>>> inaccurate to equate this to PR not being merged till build 
>>>>>> issues are
>>>>>> resolved as it derives from an assumption that CVE is same as a 
>>>>>> build
>>>>>> failure.
>>>>>>
>>>>>> While project can't grow without individual contributions, 
>>>>>> project is a
>>>>> result of a large number of contributions from a number of 
>>>>> contributors.
>>>>> Some of those contributors are not active anymore and will not 
>>>>> provide
>>>>> any
>>>>> fixes should a vulnerability be found in their contribution. It 
>>>>> becomes a
>>>>> shared responsibility of all currently active community members 
>>>>> and those
>>>>> who submit PR are part of the community and share that 
>>>>> responsibility,
>>>>> are
>>>>> not they? If a contributor considers him/herself as part of a 
>>>>> community,
>>>>> why he or she can't wait for the build issue to be resolved or better
>>>>> take
>>>>> an action on resolving the issue? The only possible explanation 
>>>>> that I
>>>>> see
>>>>> is the one that I already mentioned on this thread.
>>>>>
>>>>> If you see this as unnecessary obstacles for legitimate 
>>>>> contributions,
>>>>> why
>>>>> to enforce code style, it is also unnecessary obstacle. Unit test? 
>>>>> Should
>>>>> it be considered to be optional for a PR to pass unit tests as 
>>>>> well? What
>>>>> if an environment change on CI side causes build to fail similar 
>>>>> to what
>>>>> happened recently? Should we disable CI builds too and rely on a
>>>>> committer
>>>>> or a release manager to run unit tests?  If CI build fails for 
>>>>> whatever
>>>>> reason, how can you be sure that if it fails for another PR as 
>>>>> well, that
>>>>> they both fail for the same reason and there is no any other 
>>>>> reasons that
>>>>> will cause a problem with a PR?
>>>>>
>>>> I don't know how failing PRs because of CVE, which we don't introduce,
>>>> don't control, no idea of and possibly unrelated would fall in the 
>>>> same
>>>> bucket as unit tests. I am at a loss of words for that. There is no 
>>>> reason
>>>> to block legitimate development for this. This can be handled 
>>>> separtely
>>>> and
>>>> in parallel. Maybe there is a way we can setup an independent job on a
>>>> build server that runs nightly, fails if there are new CVEs 
>>>> discovered and
>>>> sends an email out to the security or dev group. You could even 
>>>> reduce the
>>>> CVE threshold for this. I don't believe in a stick approach (carrot 
>>>> and
>>>> stick metaphor) and believe in proportional measures.
>>>>
>>>>
>>>>
>>>>> Thank you,
>>>>>>> Vlad
>>>>>>>
>>>>>>>
>>>>>>> On 10/26/17 09:42, Pramod Immaneni wrote:
>>>>>>>
>>>>>>> On Thu, Oct 26, 2017 at 12:08 AM, Vlad Rozov <vrozov@apache.org>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> There is a way to add an exception, but it needs to be 
>>>>>>>> discussed on a
>>>>>>>> case
>>>>>>>>
>>>>>>>> by case basis. Note that CVEs are not published until a fix
is
>>>>>>>>> available.
>>>>>>>>> For severity 8 CVEs I expect patches to become available
for the
>>>>>>>>> reported
>>>>>>>>> version unless it is an obsolete version in which case,
the 
>>>>>>>>> upgrade
>>>>>>>>> to
>>>>>>>>> a
>>>>>>>>> supported version is already overdue.
>>>>>>>>>
>>>>>>>>> I think we should retain the ability to make that choice
of 
>>>>>>>>> what and
>>>>>>>>> when
>>>>>>>>>
>>>>>>>>> to upgrade rather than hard enforce it. Like I mentioned
the 
>>>>>>>>> CVE may
>>>>>>>> not
>>>>>>>> apply to us (it has happened before), even though it may
be 
>>>>>>>> beneficial
>>>>>>>> upgrade generally when its not applicable, there may not
be the
>>>>>>>> bandwidth
>>>>>>>> in community to do the necessary changes to upgrade to a
newer 
>>>>>>>> version
>>>>>>>> especially if those dependencies don't follow semver (has
happened
>>>>>>>> before
>>>>>>>> as well, remember effort with ning). My caution comes from
>>>>>>>> experiencing
>>>>>>>> this situation before.
>>>>>>>>
>>>>>>>>
>>>>>>>> I don't see how reporting helps. If a build succeeds, I don't

>>>>>>>> expect
>>>>>>>>
>>>>>>>> anyone to look into the report, it is only when CI build
fails,
>>>>>>>>> committers
>>>>>>>>> and reviewers look into the details.
>>>>>>>>>
>>>>>>>>> We can add a mandatory step during release that we need
to assess
>>>>>>>>> CVEs
>>>>>>>>>
>>>>>>>>> matching this criteria before proceeding with the release.

>>>>>>>>> This could
>>>>>>>> end
>>>>>>>> up requiring upgrade of some dependencies and in other cases
it 
>>>>>>>> may
>>>>>>>> not
>>>>>>>> be
>>>>>>>> needed. This assessment can also happen more often in adhoc

>>>>>>>> fashion
>>>>>>>> offline
>>>>>>>> before release based upon interest from community. I am also

>>>>>>>> open to
>>>>>>>> making
>>>>>>>> it mandatory with every PR, in future, like you are suggesting,

>>>>>>>> if we
>>>>>>>> see
>>>>>>>> sufficient uptake in community on these issues. From experience

>>>>>>>> this
>>>>>>>> is
>>>>>>>> not
>>>>>>>> there currently and hence I don't want to do this now.
>>>>>>>>
>>>>>>>>
>>>>>>>> IMO, it does not matter how CVE is introduced. It may be
a new
>>>>>>>> dependency
>>>>>>>>
>>>>>>>> with an existing CVE or it can be a new CVE for an existing
>>>>>>>>> dependency.
>>>>>>>>> In
>>>>>>>>> both cases, dependency with the CVE needs to be fixed.
>>>>>>>>>
>>>>>>>>> In one case the PR is not directly responsible for the
issue and
>>>>>>>>> hence
>>>>>>>>>
>>>>>>>> we
>>>>>>>> should avoid penalizing them or block them.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Thank you,
>>>>>>>>
>>>>>>>>> Vlad
>>>>>>>>>
>>>>>>>>> On 10/25/17 11:58, Pramod Immaneni wrote:
>>>>>>>>>
>>>>>>>>> Thanks that sounds mostly fine except what happens if
there is 
>>>>>>>>> a cve
>>>>>>>>>
>>>>>>>>> matching that severity in a dependency but it doesnt
affect us
>>>>>>>>>> because
>>>>>>>>>> let's say we don't exercise that part of functionality
*and* 
>>>>>>>>>> there
>>>>>>>>>> isn't a
>>>>>>>>>> fix available or there is a fix but the upgrade requires

>>>>>>>>>> significant
>>>>>>>>>> effort
>>>>>>>>>> (for example if we need to move to a new major version
of the
>>>>>>>>>> dependency
>>>>>>>>>> or
>>>>>>>>>> something like that). Is there a way to add an exception
like 
>>>>>>>>>> we did
>>>>>>>>>> for
>>>>>>>>>> checkstyle in the interim. How about reporting instead
of 
>>>>>>>>>> failing
>>>>>>>>>> the
>>>>>>>>>> builds. One of the steps in release process could
be to 
>>>>>>>>>> investigate
>>>>>>>>>> these
>>>>>>>>>> reports before proceeding with the release. If a
PR 
>>>>>>>>>> introduces new
>>>>>>>>>> cves
>>>>>>>>>> by
>>>>>>>>>> virtue of adding a new dependency or changing an
existing 
>>>>>>>>>> version,
>>>>>>>>>> that
>>>>>>>>>> would be of interest and can be subject to failure.
Is there 
>>>>>>>>>> a way
>>>>>>>>>> to
>>>>>>>>>> distinguish that?
>>>>>>>>>>
>>>>>>>>>> On Wed, Oct 25, 2017 at 8:52 AM Vlad Rozov <vrozov@apache.org>
>>>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>> A CVE (should there be a vulnerability in existing
or a newly
>>>>>>>>>> introduced
>>>>>>>>>>
>>>>>>>>>> dependency) will not be exposed during the CI build,
but the 
>>>>>>>>>> build
>>>>>>>>>>
>>>>>>>>>>> will
>>>>>>>>>>> fail if the CVE has severity 8 or above. To get
the details, it
>>>>>>>>>>> will
>>>>>>>>>>> be
>>>>>>>>>>> necessary to run dependency check manually.
>>>>>>>>>>>
>>>>>>>>>>> Thank you,
>>>>>>>>>>>
>>>>>>>>>>> Vlad
>>>>>>>>>>>
>>>>>>>>>>> On 10/24/17 16:27, Pramod Immaneni wrote:
>>>>>>>>>>>
>>>>>>>>>>> There was a lot of discussion on this but looks
like there 
>>>>>>>>>>> was no
>>>>>>>>>>> final
>>>>>>>>>>>
>>>>>>>>>>> agreement. Can you summarize what your PR does?
Are we 
>>>>>>>>>>> disclosing
>>>>>>>>>>>> the
>>>>>>>>>>>> actual vulnerabilities as part of the automated
build for 
>>>>>>>>>>>> every
>>>>>>>>>>>> PR?
>>>>>>>>>>>> That
>>>>>>>>>>>> would be a no-no for me. If it is something
that requires 
>>>>>>>>>>>> manual
>>>>>>>>>>>> steps,
>>>>>>>>>>>>
>>>>>>>>>>>> for
>>>>>>>>>>>>
>>>>>>>>>>>> example as part of a release build, that
would be fine.
>>>>>>>>>>> On Mon, Oct 23, 2017 at 1:16 PM, Vlad Rozov <vrozov@apache.org>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Please see https://github.com/apache/apex-core/pull/585
and
>>>>>>>>>>>> APEXCORE-790.
>>>>>>>>>>>> Thank you,
>>>>>>>>>>>>
>>>>>>>>>>>> Vlad
>>>>>>>>>>>>
>>>>>>>>>>>>> On 9/14/17 09:35, Vlad Rozov wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> Do you expect anything else from the
community to recognize a
>>>>>>>>>>>>>
>>>>>>>>>>>>> contribution other than committing it
to the code line? Once
>>>>>>>>>>>>> there
>>>>>>>>>>>>>
>>>>>>>>>>>>>> is
>>>>>>>>>>>>>> a
>>>>>>>>>>>>>> steady flow of quality contributions,
the community/PMC will
>>>>>>>>>>>>>> recognize
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> a
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> contributor by making that contributor
a committer.
>>>>>>>>>>>> Thank you,
>>>>>>>>>>>>
>>>>>>>>>>>>> Vlad
>>>>>>>>>>>>>> On 9/12/17 13:05, Sanjay Pujare wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> For a vendor too, quality ought to
be as important as 
>>>>>>>>>>>>>> security
>>>>>>>>>>>>>> so
>>>>>>>>>>>>>> I
>>>>>>>>>>>>>> don't
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> think we disagree on the cost benefit
analysis. But I get 
>>>>>>>>>>>>>> your
>>>>>>>>>>>>>>
>>>>>>>>>>>>> drift.
>>>>>>>>>>>>> By "creative incentive" I didn't imply
any material incentive
>>>>>>>>>>>>>
>>>>>>>>>>>>> (although a
>>>>>>>>>>>>>> gift card would be nice :-)) but
more along the lines of 
>>>>>>>>>>>>>> what a
>>>>>>>>>>>>>>
>>>>>>>>>>>>> community
>>>>>>>>>>>>> can do to recognize such contribution.
>>>>>>>>>>>>> Sanjay
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Tue, Sep 12, 2017 at 8:10 AM, Vlad
Rozov 
>>>>>>>>>>>>> <vrozov@apache.org>
>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I guess we have a different view
on the benefit and cost
>>>>>>>>>>>>> definition.
>>>>>>>>>>>>> For
>>>>>>>>>>>>> me the benefit of fixing CI build, flaky
unit test, severe
>>>>>>>>>>>>> security
>>>>>>>>>>>>> issue
>>>>>>>>>>>>>
>>>>>>>>>>>>> is huge for the community and is possibly
small (except for a
>>>>>>>>>>>>>>> security
>>>>>>>>>>>>>>>> issues) for a vendor.
>>>>>>>>>>>>>> By "creative" I hope you don't mean
that other community
>>>>>>>>>>>>> members,
>>>>>>>>>>>>>
>>>>>>>>>>>>> users
>>>>>>>>>>>>>>> and customers send a contributor
a gift cards to 
>>>>>>>>>>>>>>> compensate for
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> cost
>>>>>>>>>>>>> :). For me PR that is blocked on a failed
CI build is
>>>>>>>>>>>>>> sufficiently
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> incentive for a contributor to look
into why it fails and 
>>>>>>>>>>>>>> fixing
>>>>>>>>>>>>> it.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thank you,
>>>>>>>>>>>>>>> Vlad
>>>>>>>>>>>>>>>> On 9/11/17 23:58, Sanjay
Pujare wrote:
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> I don't want to speak for
others and I don't want to
>>>>>>>>>>>>>>>> generalize.
>>>>>>>>>>>>>>>> But
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> an
>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>> obvious answer could be "cost-benefit
analysis".
>>>>>>>>>>>>>> In any case we should come up with
a creative way to
>>>>>>>>>>>>> "incentivize"
>>>>>>>>>>>>>
>>>>>>>>>>>>> members
>>>>>>>>>>>>>>> to do these tasks.
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>>
>


Mime
View raw message