Bug 220561 - [NEW PORT] security/go-cve-dictionary: Local CVE database
Summary: [NEW PORT] security/go-cve-dictionary: Local CVE database
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Richard Gallamore
URL:
Keywords: feature
Depends on:
Blocks: 220328
  Show dependency treegraph
 
Reported: 2017-07-08 15:07 UTC by Alexandru Ciobanu
Modified: 2017-07-28 18:30 UTC (History)
2 users (show)

See Also:
koobs: maintainer-feedback+


Attachments
vuls-cve-dictionary patch (19.54 KB, patch)
2017-07-08 15:23 UTC, Alexandru Ciobanu
no flags Details | Diff
UIDs.diff (316 bytes, patch)
2017-07-08 15:23 UTC, Alexandru Ciobanu
no flags Details | Diff
GIDs.diff (255 bytes, patch)
2017-07-08 15:24 UTC, Alexandru Ciobanu
no flags Details | Diff
go-cve-dictionary.diff (19.84 KB, patch)
2017-07-14 23:58 UTC, Alexandru Ciobanu
no flags Details | Diff
go-cve-dictionary.diff (20.20 KB, patch)
2017-07-15 12:57 UTC, Alexandru Ciobanu
no flags Details | Diff
go-cve-dictionary.diff (21.15 KB, patch)
2017-07-25 12:09 UTC, Alexandru Ciobanu
no flags Details | Diff
Review diff (21.33 KB, patch)
2017-07-27 08:55 UTC, Alexandru Ciobanu
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexandru Ciobanu 2017-07-08 15:07:05 UTC
This is tool to build a local copy of the National Vulnerabilities Database(NVD) and the Japan Vulnerability Notes (JVN). NVD and JVN contain security vulnerabilities according to their CVE identifiers, including exhaustive information and a risk score. The local copy is generated in sqlite format, and the tool has a server mode for easy querying.

Databases generated by vuls-cve-dictionary are used by vuls (bug #220328).

WWW: https://github.com/kotakanbe/go-cve-dictionary/
Comment 1 Alexandru Ciobanu 2017-07-08 15:23:32 UTC
Created attachment 184183 [details]
vuls-cve-dictionary patch
Comment 2 Alexandru Ciobanu 2017-07-08 15:23:49 UTC
Created attachment 184184 [details]
UIDs.diff
Comment 3 Alexandru Ciobanu 2017-07-08 15:24:04 UTC
Created attachment 184185 [details]
GIDs.diff
Comment 4 Kubilay Kocak freebsd_committer freebsd_triage 2017-07-14 09:36:22 UTC
Thank you for your contribution Alexandru. Please:

* Confirm this change passes QA (portlint and poudriere in particular). For more information on testing, see: https://www.freebsd.org/doc/en_US.ISO8859-1/books/porters-handbook/testing.html

* Include the addition of the port to category/Makefile (since it was submitted as a diff)

* Combine all changes into a single unified diff, rather than one diff per file.

* Consider renaming vuls-cve-dictionary to go-cve-dictionary. I could not find (google) references to a more canonical upstream name than the name of the project (github repository) and none for vuls-cve-dictionary.

* Consider trimming COMMENT (portlint should pick this up)
Comment 5 Alexandru Ciobanu 2017-07-14 23:58:21 UTC
Created attachment 184367 [details]
go-cve-dictionary.diff
Comment 6 Alexandru Ciobanu 2017-07-15 00:02:20 UTC
(In reply to Kubilay Kocak from comment #4)
Kubilay, thank you for the pointers. I've updated the port and did the QA.
For "Include the addition of the port to category/Makefile" isn't setting CATEGORIES sufficient? Or is there something else I'm missing?
Comment 7 Kubilay Kocak freebsd_committer freebsd_triage 2017-07-15 03:09:16 UTC
(In reply to Alexandru Ciobanu from comment #6)

Thank you. New port additions need entries added to the respective (primary) category/Makefile as well as have CATEGORIES set. In this case for example:

security/Makefile:

...
SUBDIR += go-cve-dictionary
...

See also:

19.1. Adding a New Port

https://www.freebsd.org/doc/en/articles/committers-guide/ports.html#ports-qa-add-new

Could you confirm the details of the QA performed? For example:

portlint: OK (looks fine.)
testport: OK (poudriere: <freebsd versions>, <archs> tested)
Comment 8 Alexandru Ciobanu 2017-07-15 12:57:24 UTC
Created attachment 184372 [details]
go-cve-dictionary.diff

portlint: OK (looks fine.)
testport: OK (poudriere: 10.3-RELEASE-p20, 11.0-RELEASE-p11, amd64 and i386 tested)

Upstream recommends using go 1.7.1+, I've updated USES accordingly.
Comment 9 Kubilay Kocak freebsd_committer freebsd_triage 2017-07-15 14:02:10 UTC
Nice work Alexandru *thumbs up*
Comment 10 Alexandru Ciobanu 2017-07-15 14:14:08 UTC
(In reply to Kubilay Kocak from comment #9)
Thank you for your help.
Comment 11 Richard Gallamore freebsd_committer freebsd_triage 2017-07-21 22:25:51 UTC
I found a few more things. Once they are fixed, i'll do one more runtime check to verify working order and it should be ready for commit.


This port requires security/ca_root_nss to download the database because it uses https. It will fail due to unable to validate the ssl certificate. This should be added to runtime depends.

The STRIP= # variable can be removed, stripping is always safe. Back in 2010-2011, there were a couple go bugs that improperly generated ELF, long since fixed. [1] is an article about if you are interested in more details.

The pkg-message is clever, I like it! To make it easier for new users to get up and running, and also make run-time testing easier, I suggest adding something along these lines:

chown vuls:vuls /var/db/vuls/*

To enable go-cve-dictionary and start
sysrc go_cve_dictionary_enable="YES"
service go-cve-dictionary start


The do-build should not install directly into the staging area. Change this to ${WRKDIR} or ${WRKSRC} or just somewhere in the working area. Use ${INSTALL_PROGRAM} to install into staging area during do-install target.

Take a look at Mk/Uses/go.mk and look at the do-build and do-install targets there and see if these targets can be removed entirely or partially and move the bit that is needed to post-*. If they can't, change them to include the other environment variables and use ${GO_CMD} instead of calling the command directly.

One last thing that should be addressed, there are many hard coded items to this port and should be changed to a single variable in the Makefile. For instance, /var/db/vuls in rc script should be changed to %%DBDIR%% or %%DB_DIR%% and add a var DB_DIR= /var/db/vuls, add to SUB_LIST. This will change all those entries appropriately when processed. Similar, LOG_DIR should also be added. These hard coded entries should also be addressed in the patch files where /var/db/vuls and /var/log/vuls, change to %%DB_DIR%% then in post-patch: add a ${REINPLACE_CMD} to change to the corrected value.

Some of these items also apply to security/vuls. Please adapt where appropriate.

[1] https://dominik.honnef.co/posts/2016/10/go-and-strip
Comment 12 Alexandru Ciobanu 2017-07-25 12:09:12 UTC
Created attachment 184698 [details]
go-cve-dictionary.diff

* added security/ca_root_nss runtime depends
* removed strip (thank you for the article)
* updated pkg-message with step-by-step instructions
* update do-build and do-install targets
* removed hard coded values

portlint warnings:
WARN: Makefile: possible use of absolute pathname "/var/db/vuls".
WARN: Makefile: possible use of absolute pathname "/var/log/vuls".

testport: OK (poudriere: 10.3-RELEASE-p20, 11.0-RELEASE-p11, amd64 and i386 tested)
Comment 13 Alexandru Ciobanu 2017-07-26 14:42:51 UTC
I've tested the port on 11.1 too, everything builds fine.

testport: OK (poudriere: 10.3-RELEASE-p20, 11.0-RELEASE-p11, 11.1-RELEASE amd64 and i386 tested)
Comment 14 Richard Gallamore freebsd_committer freebsd_triage 2017-07-27 01:17:15 UTC
Found a couple small items, I went ahead and fixed them and is now pending final review for commit. If you would like to follow, https://reviews.freebsd.org/D11745 and D11746 is for vuls.

Thanks for all your contributions, this will definitely be a big benefit to the community!
Comment 15 Alexandru Ciobanu 2017-07-27 08:55:48 UTC
Created attachment 184756 [details]
Review diff

Updated diff based on the review.
Comment 16 Richard Gallamore freebsd_committer freebsd_triage 2017-07-27 19:36:37 UTC
Updated review, the port requires PORTVERSION to be set. I also removed the the hardcoding that I suggested as Nikolai noted.
Comment 17 Alexandru Ciobanu 2017-07-27 19:50:01 UTC
Great! Thank you.
Comment 18 Richard Gallamore freebsd_committer freebsd_triage 2017-07-27 22:45:22 UTC
I was wrong about the PORTVERSION, you had this correct.
Comment 19 commit-hook freebsd_committer freebsd_triage 2017-07-28 18:23:00 UTC
A commit references this bug:

Author: ultima
Date: Fri Jul 28 18:22:24 UTC 2017
New revision: 446843
URL: https://svnweb.freebsd.org/changeset/ports/446843

Log:
  This is tool to build a local copy of the National Vulnerabilities Database(NVD)
  and the Japan Vulnerability Notes (JVN). NVD and JVN contain security
  vulnerabilities according to their CVE identifiers, including exhaustive
  information and a risk score. The local copy is generated in sqlite format, and
  the tool has a server mode for easy querying.

  WWW: https://github.com/kotakanbe/go-cve-dictionary/

  PR:		220561
  Submitted by:	Alexandru Ciobanu <iscandr@gmail.com> (maintainer)
  Reviewed by:	matthew (mentor), koobs, mat
  Approved by:	matthew (mentor)
  Differential Revision:	https://reviews.freebsd.org/D11745

Changes:
  head/GIDs
  head/UIDs
  head/security/Makefile
  head/security/go-cve-dictionary/
  head/security/go-cve-dictionary/Makefile
  head/security/go-cve-dictionary/distinfo
  head/security/go-cve-dictionary/files/
  head/security/go-cve-dictionary/files/go-cve-dictionary.in
  head/security/go-cve-dictionary/files/patch-commands_fetchjvn.go
  head/security/go-cve-dictionary/files/patch-commands_fetchnvd.go
  head/security/go-cve-dictionary/files/patch-commands_server.go
  head/security/go-cve-dictionary/files/pkg-message.in
  head/security/go-cve-dictionary/pkg-descr
  head/security/go-cve-dictionary/pkg-plist
Comment 20 Richard Gallamore freebsd_committer freebsd_triage 2017-07-28 18:30:52 UTC
Committed, thanks!