Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add third party vuln process #57

Merged
merged 1 commit into from
Dec 8, 2017
Merged

add third party vuln process #57

merged 1 commit into from
Dec 8, 2017

Conversation

vdeturckheim
Copy link
Member

@vdeturckheim vdeturckheim commented Oct 22, 2017

Workflow as decided in #54

@@ -0,0 +1,86 @@
This document describes the management of vulnerabilities within the Node.js
ecosystem. Vulneabilities in Node.js core are out of this scope.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vulneabilities -> Vulnerabilities

# Definitions

* package: in this document, a package is a module available widely to use
with Node.js.Most modules are hosted on npmjs.org repository.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Space after Node.js. Also add "the" before npmjs.org.


# Process

Individuals who found potential vulnerabilities in a package are invited
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

found -> find

# Process

Individuals who found potential vulnerabilities in a package are invited
to fill a vulnerability report on the dedicated HackerOne organization:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fill -> complete


[https://hackerone.com/nodesecurity](https://hackerone.com/nodesecurity)

When a potential vulnerability is reported, the following actions are taken
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a colon after taken.


**Delay:** 48 hours

Within 72 hours, a member of the triage team provides a first answer to the
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the turnaround time 48 or 72 hours?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦‍♂️

individual who submitted the potential vulnerability. The possible responses
can be:

* acceptation: what was reported is considered as a new vulnerability
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

acceptation -> Acceptance

## Case of vulnerabilities found outside this process

Vulnerabilities found and fixed outside this process should be added into
the vulnerability base. This can be done by anyon through a Pull Request on
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

anyon -> anyone

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review!

@@ -0,0 +1,86 @@
This document describes the management of vulnerabilities within the Node.js
ecosystem. Vulnerabilities in Node.js core are out of this scope.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saying that, we will end up collecting vulnerabilities for Front-end packages.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps link here to https://nodejs.org/en/security/ or something like that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the nsp database includes a couple front-end packages, as does snyk's database. There are lots of FE code distributed via npmjs.org. Node "app" developers (or users of those apps when they are insecure) don't necessarily distinguish between a "vuln in js code evaluated by node" and "vuln in js code served by node to a FE, and evaluated by a web browser". We need to be really clear, though, on what the scope is if the DB managed by the Node Foundation. Is it packages in npmjs.org? Or, is it node javascript packages (no matter where accessed from)? Both?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We historically considered anything within npm to be in scope for what to include in our ecosystem initiative.

the vulnerability base. This can be done by anyone through a Pull Request on
this repository.

# The triage team
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add a part discussing NDA

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once #56 lands, the team membership and NDA can be shared.

Copy link
Member

@bengl bengl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who manages the HackerOne org should be mentined (i.e. this WG).

@@ -0,0 +1,86 @@
This document describes the management of vulnerabilities within the Node.js
ecosystem. Vulnerabilities in Node.js core are out of this scope.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps link here to https://nodejs.org/en/security/ or something like that?

**Delay:** 45 days

When a vulnerability is confirmed, a member of the triage is
designated as follow up on this topic.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

designated to follow up is clearer I think.


Within 45 days after the triage date, the vulnerability must be made public.

If a patch is being developed by the package maintainer, a delay of 30 days
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

being developed -> being actively developed


Within HackerOne, this is handled through a "public disclosure request".

## Case of vulnerabilities found outside this process
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Case of vulnerabilities -> Vulnerabilities


## Triage

**Who:** The triage team
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this list who the members of the triage team are? I assume this is made up of the current recipients of the security@ messages?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or is this specifically only for module vulnerabilities?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jasnell This is a separate group for triage of non-core vulnerabilities.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

once #56 lands, this PR can also include addition of a Team to the doc describing privacy policy, because the same privacy policy should apply to thirdparty vulns as to core (I think)


Individuals who find potential vulnerabilities in a package are invited
to complete a vulnerability report on the dedicated HackerOne organization:

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should also be possible for a vulnerability to be reported via email (which can be forwarded to HackerOne and will auto-create a vuln report).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have an issue on setting up this on H1 side right now. I have pinged H1 on that for assistance.


## Triage

**Who:** The triage team
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

once #56 lands, this PR can also include addition of a Team to the doc describing privacy policy, because the same privacy policy should apply to thirdparty vulns as to core (I think)


Within 45 days after the triage date, the vulnerability must be made public.

If a patch is being developed by the package maintainer, a delay of 30 days
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have to be so specific about the 30 days? Can we make it more wish-washy, something like "by majority agreement and sole discretion of the triage team, publicization may be delayed. This will only be done if there is some clear proposal for a timeline for publicization, and justification for the delay."? This basically allows the triage team to "do the right thing" under unusual situations, rather than being bound to overly strict rules.

the vulnerability base. This can be done by anyone through a Pull Request on
this repository.

# The triage team
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once #56 lands, the team membership and NDA can be shared.


# Definitions

* package: in this document, a package is a module available widely to use
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest

a package available for use with Node.js and hosted on the npmjs.org repository.

* Acceptance: what was reported is considered as a new vulnerability
* Rejection: what was reported is not considered as a new vulnerability
* Need more information: the triage team needs more information in order to
evaluate what was reported.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to be lined up with Need for proper markdown formatting.


**Delay:** 45 days

When a vulnerability is confirmed, a member of the triage is
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

triage -> triage team

**Delay:** 45 days

When a vulnerability is confirmed, a member of the triage is
designated to follow up on this topic.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

topic -> report

## Vulnerabilities found outside this process

Vulnerabilities found and fixed outside this process should be added into
the vulnerability base. This can be done by anyone through a Pull Request on
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

base -> database


# The triage team

The triage team is composed of 7 to 10 members of the security working group.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need a number here ? May be better to just leave it up to the security wg team to decide when people volunteer.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some stated floor for the minimal number has value to me that at least expresses the intent to ensure that this is not a role that can be distilled down to an individual or lesser number who feel they can/should take it on alone. Not sure how binding this is as a process doc vs. a charter for the group which is where that would ideally be documented, but I don't think that exists atm.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A minimum number would not be a concern for me. I was more worried about when we hit X and then somebody volunteers.... Probably a good problem to have. Maybe we say something like composed of Y or more members ..

@mhdawson
Copy link
Member

Overall looks good to me. We just need to fill in the email address and ensure the hackerone org is active before landing.

@vdeturckheim
Copy link
Member Author

We can now set up email alias to our H1 org. PR has been opened in the email repo nodejs/email#75

@vdeturckheim
Copy link
Member Author

This should be good to merge now, does anyone wants to review one last time?

Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@mhdawson
Copy link
Member

mhdawson commented Dec 7, 2017

I see its passed on smartos15 and smartos16, so maybe its just that the versions of Node.js that we are testing don't run on smartos14 and we should just remove that. @cjihrig what is the oldest version of Node.js that runs on smartos 14 ?

@cjihrig
Copy link
Contributor

cjihrig commented Dec 8, 2017

@mhdawson wrong issue?

@vdeturckheim vdeturckheim merged commit 91d3a8b into nodejs:master Dec 8, 2017
@vdeturckheim vdeturckheim deleted the third_party_vuln_process branch December 8, 2017 09:02
@mhdawson
Copy link
Member

mhdawson commented Dec 8, 2017

@cjihrig yes, will move to right one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants