Security-wg: [ANN]: refactor report id field to be auto-generating, Deadline: 10.11.2018

[lirantal]

Proposal

Instead of manually choosing report ids for vulnerabilities which is error-prone, hard to automate, and cumbersome manual process, we'd like to propose an automatically generated ID that will be assigned for reports.

Specifically, to convert the current millisecond timestamp to a large base representation, i.e:

new Date().getTime().toString(36)
// something like jlmn6im3

Which would be sustainable for quite a while - if we're agreeing to use a milliseconds precision as base then we are currently at 8 characters length for the ID, which remains up until 2059 or so:

new Date('2059').getTime().toString(36)
// zu96s5c0

So that's sustainable as a short 8 chars id for the next few decades.
After that, we can still use the same method except the length is incremented by one more char for the next century or so :-)

new Date('2060').getTime().toString(36)
// 108qqi2o0

--

This is a request-for-comments announcement, and we'd be happy to discuss and consider other alternatives.

Report Format

To stick with the current system, the new report ID can be formatted like so:

References

Discussed in https://github.com/nodejs/security-wg/issues/359#issuecomment-417505159 as part of moving to new database repository (https://github.com/nodejs/security-wg/issues/359) but not coupled to that change.

cc @nodejs/security-wg @nodejs/ecosystem-security

[bnb]

Which would be sustainable for quite a while.

How long is "quite a while" 😅

My concern is that this dataset should be able to be added to for some time – as in, 10-20 years. I'm guessing this would still be fine within that timeframe, but always good to check.

[dgonzalez]

I am happy with it.

[pxlpnk]

👍

[MarcinHoppe]

👍

[vdeturckheim]

LGTM

[lirantal]

@bnb

How long is "quite a while" 😅

If we're agreeing to use a milliseconds precision as base then we are currently at 8 characters length for the ID, which remains up until 2059 or so:

new Date('2059').getTime().toString(36)
// zu96s5c0

So that's sustainable as a short 8 chars id for the next few decades.
After that, we can still use the same method except the length is incremented by one more char for the next century or so :-)

new Date('2060').getTime().toString(36)
// 108qqi2o0

I'll update the description above with this info too.

[lirantal]

@vdeturckheim @MarcinHoppe @pxlpnk @dgonzalez @bnb

Report Format

To stick with the current system, the new report ID can be formatted like so:

or as upper-case letters:

--

I'm thinking the first option where it's all lower case is more readable (for example differentiating between zero (0) and the letter o (O).

WDYT?

[pxlpnk]

Lowercase makes sense I think.
Would the filename be something like:

/npm/$packagName/NSWG-ECO-zu96s5c0.json
and respective:
/core/NSWG-ECO-zu96s5c1.json

Or should core stay as it is in terms of naming?

[lirantal]

With the slight correction that core will probably be /core/NSWG-CORE-zu96s5c1.json (notice CORE vs ECO) but that's one option indeed.

Another question is how would the change affect the report id field?
should it be:

{
  "id": "zu96s5c0",
  "// rest of the fields": ""
}

or

{
  "id": "NSWG-ECO-zu96s5c0",
  "// rest of the fields": ""
}

?

I'm voting for the 2nd option where it's full name is used.

[pxlpnk]

Yes, I think number two is also the way to go. I totally missed the ECO part btw 😸

[mhdawson]

+1

[lirantal]

@grnd @evilpacket @dgonzalez @cjihrig @rvagg would be great to get some input from you as well on the suggested proposed on this issue for the numbering/naming of vulnerability reports that the Security WG maintains.

[cjihrig]

Sounds like a fine plan to me. +1

[lirantal]

Currently all the feedback is positive for the change but another ping to @nodejs/security-wg if anyone wants to chime on this before we go forward.

[sam-github]

+1

[sam-github]

I think changing to a more structured ID for new reports is a great idea, but I'm worried that changing the IDs for existing reports will cause a lot of difficulty for existing users.

We should move forward quickly on this, changing to a better ID format isn't controversial, as far as I can see. However, I can't see anything in this report that gives an indication that it was the intention to change existing IDs.

[lirantal]

@sam-github (and @nodejs/security-wg), following the concern raised in the last agenda meeting involving the possible breaking change of the report ids, here are some summary points:

• report ids changing are not the only breaking change - we will be moving the vuln db to another repository altogether and this will break customers integrating with us as well.
• this was announced 3 months back in both the working group issue queue as well as twitter media for awareness

Path for report ids changing

Nevertheless, the change we want to intoruce to the report ids is indeed potentially breaking our report API for consumers and the following is a plan to roll the change with mitigation points:

1. The new report id format will replace the existing id field for existing reports and will be used as the new format for that identification field in new reports created.
2. The old reports will be changed to introduce a new field called alternativeIds which will be an array of objects that will identify an old report with its old report id. For example an old report id after the change:

{
 "id": "NSWG-ECO-dcbiu2",
 "title": "lorem ipsum",
 "alternativeIds": [
    {
      "id": 458,
      "source": "Node.js Foundation Security Working Group"
    }
 ]
}

1. To further help consumers of our database to make this change we will create a mapping file with the old report id and the new report id for all the affected reports in both a JSON and CSV format.

[sam-github]

So, I'm just one guy with an opinion, if no one shares my concerns, I won't block this, but:

report ids changing are not the only breaking change - we will be moving the vuln db to another repository altogether and this will break customers integrating with us as well.

Moving data, and making the data format different are not breaking changes of the same sort.

this was announced 3 months back in both the working group issue queue as well as twitter media for awareness

I've reread this entire thread and found not a single mention of the intention to change the ID of existing, already issued vulnerabilities, did I miss it? I don't think saying "people have known about it for months" is fair. I'd say most of the positive review has been for having a structured auto-generated format. That's an obviously great idea, IMO (and all the reviewers!).

But extending that to "no one has concerns about their current data imports being broken" is a stretch.

I would turn this around: what possible benefit is there to churning our currently issued vulnerability IDs? Moving our existing ID into the alternatives field gives people a migration path, that's true, but why are we forcing the migration at all? It seems we are offering a work around for a problem we created, and its not clear to me why we wouldn't just _not create the problem_.

It could be there is a good reason to retroactively reidentify our existing vulnerabilities, but I'm not seeing it yet.

[lirantal]

You may be one guy but of course I value your comments! ;-)

Correct about specifically the older reports being changed is not mentioned. I don't want to excuse us by saying "we announced it" but rather my intentions are that even when announcing this general change there was little if at all any feedback altogether from external parties (as in, not the sec wg themselves).

Keeping the old report ids as they are means that:

1. future references to them will look odd further down the road as they will not be associated with the NSWG, and simply may just create more confusion.
2. requires more effort from our tooling like validation and vulnerability management which right now is just creating reports or linting existing but might grow in the future to encompass much more than that.
3. makes the old and new reports inconsistent in their identifiers.

We have made other "breaking changes" in the past to our vulnerability formats, and yes updating an identifier might be the biggest of them.

P.S. with that said, there is an idea to abstract the whole vuln db for future consuming clients but that's mostly WIP in me and vladimir's head.