Add CVSS mapping #86

tessereth · 2017-09-12T06:09:37Z

This PR continues the work started in #33. After the conversation there, I decided to add the mapping to a separate file and given how much that changes things, I thought I'd make it a new PR. Also noting we're not planning on using CWE right away, this only adds CVSS for simplicity. But we should be able to add CWE later without too much hassle. My assumption is, any additional mappings would be added as a separate file in the mappings directory.

cvss_v3.json (and possible other mapping files to come later) has the following properties:

The cvss_v3 attribute can be specified at any depth. The most specific value will be used.
If cvss_v3 is provided on a parent node, the children attribute does not need to be provided and if it is, not all children are required. Only include children where you need to override the mapped value.

This also updates validate_vrt.py to check that:

cvss_v3.json matches its schema file
every leaf node in the VRT has a cvss mapping
every id in the cvss mapping file is a valid VRT id

TODO

Finish off the last few CVSS vectors (marked with TODO)
Update the schema file to not allow TODO as a valid CVSS vector
Update README.md

adamrdavid · 2017-09-12T19:50:16Z

.gitignore

@@ -0,0 +1,2 @@
+*.pyc


the sys.dont_write_bytecode = True is supposed to keep these out. did that not work?

Hm. I moved that line into main() because it looked silly in the imports and I didn't know what it did. But apparently it needs to be before the import of validate_deprecated_mapping to stop it generating validate_deprecated_mapping.pyc. Is there any particular reason to suppress generating .pyc files other than cluttering up the directory? Because I would assume there are (minor) performance benefits from keeping them.

ya good point, better to just delete that line.

adamrdavid · 2017-09-14T17:16:22Z

validate_vrt.py

@@ -2,6 +2,7 @@
 import json
 import jsonschema
 import sys
+# Don't create *.pyc files
 sys.dont_write_bytecode = True


Are we removing this? I don't recall if it had something to do with the build process or if its just better solved by the gitignore.

Oh right. Gone now.

adamrdavid · 2017-09-14T17:16:52Z

mappings/cvss_v3.json

+{
+  "metadata": {
+    "default": "AV:N/AC:L/PR:N/UI:N/S:U/C:N/I:N/A:N"
+  },


what do you think about putting the version in the metadata instead of the filename?

But what happens if we ever decide to add cvss v2 or a theoretical cvss v4? Having a file cvss.json with metadata.{cvss_}version = 3.0 would make that harder. Not that I think those are likely anytime soon but I'd rather not block it if it's easily avoided.

I could be misunderstanding the situation, but I was thinking of treating it the same way we do with the vrt json. Since no single version of vrt will have mappings to multiple versions of cvss, when a new cvss version comes out we would need to create a new version of the vrt to include the updated mapping. All nodes from previous vrt versions will always map to the latest cvss version through the deprecated node mapping.

@barnett or @plr0man may have more insight on the intended use?

another idea could be to only have the latest_version in the metadata and mappings to different versions as keys in the json like:

{ "id": "server_security_misconfiguration", "children": [ { "id": "unsafe_cross_origin_resource_sharing", "cvss_v3": "AV:N/AC:L/PR:N/UI:R/S:U/C:H/I:L/A:N", "cvss_v4": "AV:N/AC:L/PR:N/UI:R/S:U/C:H/I:L/A:N" } ] }

I think the default CVSS version is more a matter of customers' preference. If CVSS v4 ever comes out we would still be using CVSS v3 by default, until we would see most of our customers switch over to v4, which might or might not happen. If we had customers that prefer v2, we might consider adding that mapping too.

Ah ok. I didn't realize that we'd be maintaining multiple cvss mappings simultaneously. Good to know.

adamrdavid · 2017-09-14T17:19:59Z

README.md

@@ -105,6 +105,46 @@ _2 nodes being collapsed into 1_
 }
 ```

+### Mapping to other systems


adamrdavid · 2017-09-14T17:23:37Z

What do you think of adding validation that the mapping is declared at the highest possible level? i.e. if child nodes share a mapping it is only declared on the parent.

tessereth · 2017-09-15T00:11:41Z

What do you think of adding validation that the mapping is declared at the highest possible level? i.e. if child nodes share a mapping it is only declared on the parent.

I'm not sure. I thought about it, but then I was looking at some nodes with only one child and it seemed better from a readability point of view to include the child. @plr0man as someone actually editing these files, would you find it useful to be alerted if all children have the same value or just annoying that you're forced to move things up a tier when you don't want to?

adamrdavid · 2017-09-15T03:45:07Z

I'm not sure. I thought about it, but then I was looking at some nodes with only one child and it seemed better from a readability point of view to include the child. @plr0man as someone actually editing these files, would you find it useful to be alerted if all children have the same value or just annoying that you're forced to move things up a tier when you don't want to?

This is true. I was merely thinking from a consistency point of view it would be better to either declare them all at the highest possible level or at every leaf node. Since I'm not the one who will primarily be maintaining it, I'm happy to go with whatever you or @plr0man think works best.

plr0man · 2017-09-15T14:45:46Z

From my perspective flexibility when all children share the same mapping is best

plr0man · 2017-09-15T14:53:47Z

mappings/cvss_v3.schema.json

+{
+  "$schema": "http://json-schema.org/draft-04/schema#",
+  "title": "Vulnerability Rating Taxonomy",
+  "description": "A Taxonomy of potential vulnerabilities with suggested technical priority rating",


Do we want to update the title and description?

plr0man · 2017-09-15T15:04:34Z

mappings/cvss_v3.schema.json

+      "required": ["default"]
+    },
+    "VRTid": { "type": "string", "pattern": "^[a-z_][a-z_0-9]*$" },
+    "CVSSv3": { "type": "string", "pattern": "^AV:./AC:./PR:./UI:./S:./C:./I:./A:.$" },


How about "^AV:[NALP]/AC:[LH]/PR:[NLH]/UI:[NR]/S:[UC]/C:[NLH]/I:[NLH]/A:[NLH]$"?

adamrdavid

Update the contributing doc to include steps for adding cvss

adamrdavid · 2017-09-19T01:11:38Z

So I added a new node and still got All VRT nodes have cvss_v3 mappings
branch with new node: test-branch

tessereth · 2017-09-19T02:49:50Z

Ah, yeah. It only checks that all leaf nodes have mappings and because the node you added has a children attribute, it doesn't consider it a leaf node. If you add

{
  "id": "new_node",
  "name": "new node",
  "type": "category",
  "priority": null
}

It does fail validation. I'm pretty sure you have to have at least one children, so I might update vrt.schema.json to cover that.

As for whether we should have mappings for internal nodes... I assume given they don't have a priority, not having a cvss mapping is reasonable.

adamrdavid

The previous validation is a little bit confusing. If you add a node with no children or priority, it says that children is required. That should definitely not block this though.

tessereth · 2017-09-19T23:26:39Z

Yeah, that error message a problem with the jsonschema library we use to do that validation. Not a lot we can do about it.

plr0man

Thank you @tessereth for putting so much thought and effort into it. @adamrdavid great thanks to you too for overlooking every step of the implementation. You guys crushed it and made sure none of SecOps requests were left unanswered! And this is not just CVSS, we broke ground for any future mappings.

tessereth added 6 commits September 12, 2017 11:09

Add .gitignore

9880723

Add CVSS V3 mapping file

60bfb9c

Update CHANGELOG for CVSS V3

47f5082

Validate CVSS mapping against its schema

b65bdb7

Add mapping validations

1d0ad37

Add newline at end of file

2329e28

tessereth mentioned this pull request Sep 12, 2017

Mapping to CWE and CVSS #33

Closed

Allow mapping to be set on internal nodes

daf6c76

tessereth mentioned this pull request Sep 12, 2017

CVSS v3 mapping bugcrowd/vrt-ruby#5

Merged

5 tasks

adamrdavid reviewed Sep 12, 2017

View reviewed changes

tessereth and others added 4 commits September 13, 2017 11:49

Properly stop creation of *.pyc files

b43ee32

Move cvss scores higher in the tree where appropriate

78ed438

Update mapping values

1e7a0be

Add default cvss mapping and update schema

0cdd258

tessereth force-pushed the add-cvss-mapping branch from 6f2695b to 9e604c5 Compare September 14, 2017 02:04

Add mapping information to README

78d8fdd

tessereth force-pushed the add-cvss-mapping branch from 9e604c5 to 78d8fdd Compare September 14, 2017 02:04

adamrdavid reviewed Sep 14, 2017

View reviewed changes

README.md Outdated

@@ -105,6 +105,46 @@ _2 nodes being collapsed into 1_

}

```

### Mapping to other systems

Copy link

Contributor

adamrdavid Sep 14, 2017

Choose a reason for hiding this comment

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

💯

Let python create .pyc files if it wants to

a61d289

plr0man reviewed Sep 15, 2017

View reviewed changes

Update schema description and CVSSv3 regex

241eba1

adamrdavid reviewed Sep 19, 2017

View reviewed changes

Require at least one child if children is specified

c6af790

tessereth added 3 commits September 19, 2017 12:55

Add reminder to update mappings to CONTRIBUTING.md

3e87c89

Merge remote-tracking branch 'origin/master' into add-cvss-mapping

5c964c1

Update headings to titlecase

10fcf42

adamrdavid approved these changes Sep 19, 2017

View reviewed changes

plr0man approved these changes Sep 20, 2017

View reviewed changes

barnett added this to the v1.3 milestone Sep 20, 2017

barnett approved these changes Sep 20, 2017

View reviewed changes

barnett merged commit 90bee4f into master Sep 20, 2017

barnett deleted the add-cvss-mapping branch September 20, 2017 18:32

barnett mentioned this pull request Sep 20, 2017

Add CWE and CVSS (Base) vector mappings #32

Closed

jcran mentioned this pull request Oct 13, 2017

CWE mapping #99

Closed

barnett added the mappings Relating to external mapping taxonomies label Nov 21, 2019

Add CVSS mapping #86

Add CVSS mapping #86

Uh oh!

Conversation

tessereth commented Sep 12, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamrdavid commented Sep 14, 2017

Uh oh!

tessereth commented Sep 15, 2017

Uh oh!

adamrdavid commented Sep 15, 2017

Uh oh!

plr0man commented Sep 15, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adamrdavid left a comment

Choose a reason for hiding this comment

Uh oh!

adamrdavid commented Sep 19, 2017

Uh oh!

tessereth commented Sep 19, 2017

Uh oh!

adamrdavid left a comment

Choose a reason for hiding this comment

Uh oh!

tessereth commented Sep 19, 2017

Uh oh!

plr0man left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tessereth commented Sep 12, 2017 •

edited

Loading