test: add script to create 0-dns-cert.pem by shigeki · Pull Request #11579 · nodejs/node

shigeki · 2017-02-27T13:00:05Z

0-dns-cert.pem and 0-dns-key.pem were stored in test/fixtures/key
directory, but the cert file cannot be created with the openssl
command via Makefile. make clean removes them but we could not re-create them.

This added a script to create it with using asn1.js and
asn1.js-rfc5280 and moved them out of key directory and put into
test/fixtures/0-dns.

The domains listed in the cert were also changed into example.com and
example.org to show the use for only testing.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
commit message follows commit guidelines

Affected core subsystem(s)

test, tls

R: @indutny

0-dns-cert.pem and 0-dns-key.pem were stored in `test/fixtures/key` directory, but the cert file cannot be created with the openssl command via Makefile. Added a script to create it with using `asn1.js` and `asn1.js-rfc5280` and moved them out of key directory and put into `test/fixtures/0-dns`. The domains listed in the cert were also changed into example.com and example.org to show the use for only testing.

shigeki · 2017-02-27T13:01:41Z

CI is running on https://ci.nodejs.org/job/node-test-pull-request/6602/ .

shigeki · 2017-02-27T13:04:05Z

To be the fix of #10228.

shigeki · 2017-02-28T07:45:22Z

The ci jobs was somehow removed.
A new job is https://ci.nodejs.org/job/node-test-pull-request/6623/ and all is green.

sam-github · 2017-02-28T22:10:17Z

+
+$ node ./createCert.js
+$ openssl x509 -text -in 0-dns-cert.pem
+(You can not see evel.example.com in subjectAltName field)


sam-github · 2017-02-28T22:15:25Z

+
+const private_key = fs.readFileSync('./0-dns-key.pem');
+// public key file can be generated from the private key with
+// openssl rsa -in 0-dns-key.pem -RSAPublicKey_out -outform der \


you can wrap to 80 columns, and the backslash isn't necessary at end of line

sam-github · 2017-02-28T22:15:38Z

+const crypto = require('crypto');
+const rfc5280 = require('asn1.js-rfc5280');
+const asn1 = require('asn1.js');
+const BN = asn1.bignum;


sort requires

sam-github · 2017-02-28T22:17:33Z

@@ -0,0 +1,75 @@
+'use strict';


js files would be named create-cert.js

sam-github · 2017-02-28T22:25:46Z

+const asn1 = require('asn1.js');
+const BN = asn1.bignum;
+
+const id_at_commonName = [ 2, 5, 4, 3 ];


inconsistent casing, sometimes snake_case, sometimes camelCase, it looks like test/fixtures doesn't get linted

id_at_commonName is named after the ASN.1 notation in RFC5280 which would come from OID name.

sam-github · 2017-02-28T22:26:39Z

+const subject = PrintStr.encode('evil.example.com', 'der');
+
+const tbs =
+ { version: 'v3',


this isn't generally how node indents object literals

sam-github · 2017-02-28T22:27:33Z

+
+const cert = {
+ tbsCertificate: tbs,
+ signatureAlgorithm: { algorithm: sha256WithRSAEncryption, parameters: null_},


lacking before closing } here and a couple lines onwards

sam-github · 2017-02-28T22:28:42Z

 const common = require('../common');
 const assert = require('assert');

+// check getPeerCertificate can properly handle '\0' for fix CVE-2009-2408


Good to have a test description, it should be a capitalized and period terminated sentence.

sam-github · 2017-02-28T22:31:26Z

Nice use of asn1 to craft the cert. I think the js code should follow the same coding conventions we use elswhere, even though test/fixtures seems to be exempted from linting at the moment.

shigeki · 2017-03-01T05:29:21Z

I made eslint for test/fixtures/0-dns/create-cert.js and only one error of missing common module appeared.

$ ./node tools/eslint/bin/eslint.js --rulesdir=tools/eslint-rules test/fixtures/0-dns/create-cert.js --no-ignore
/home/sotsu/github/node/test/fixtures/0-dns/create-cert.js
1:1 error Mandatory module "common" must be loaded required-modules
✖ 1 problem (1 error, 0 warnings)

I think it is not necessary for the script in fixtures.

sam-github

Its still a mixture of snake_case and camelCase for vars, but basically LGTM

sam-github · 2017-03-01T19:42:09Z

+Z4CCF58oC4b7MrfFo1LXW8EdSjfK5ejFse6xZe6WNAahi7vDS7RJEyoq3EeZ4+A0
+DvrSjCuretoVAC/U7gUxs467yM3ZCujqZ4OANVQF6knRziTJEF5c1aXOzM563FT3
+ufeH36lO0ImzE+a2g6et3BZDL2PQLmBF3F6clQ==
+-----END CERTIFICATE-----


missing newline

shigeki · 2017-03-02T01:36:00Z

camelCase vars are used to follow ASN.1 notation in ~~RFC5289~~ RFC5280.

shigeki · 2017-03-02T01:41:40Z

@indutny Please take a look of this if you have time for you are the author of asn.1 and asn1.js-rfc5280.

shigeki · 2017-03-07T14:27:53Z

I will land this tomorrow if there are no any comments.

shigeki · 2017-03-10T08:32:02Z

Landed in dacaaa5. Thanks.

0-dns-cert.pem and 0-dns-key.pem were stored in `test/fixtures/key` directory, but the cert file cannot be created with the openssl command via Makefile. Added a script to create it with using `asn1.js` and `asn1.js-rfc5280` and moved them out of key directory and put into `test/fixtures/0-dns`. The domains listed in the cert were also changed into example.com and example.org to show the use for only testing. Fixes: #10228 PR-URL: #11579 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com>

0-dns-cert.pem and 0-dns-key.pem were stored in `test/fixtures/key` directory, but the cert file cannot be created with the openssl command via Makefile. Added a script to create it with using `asn1.js` and `asn1.js-rfc5280` and moved them out of key directory and put into `test/fixtures/0-dns`. The domains listed in the cert were also changed into example.com and example.org to show the use for only testing. Fixes: nodejs#10228 PR-URL: nodejs#11579 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com>

0-dns-cert.pem and 0-dns-key.pem were stored in `test/fixtures/key` directory, but the cert file cannot be created with the openssl command via Makefile. Added a script to create it with using `asn1.js` and `asn1.js-rfc5280` and moved them out of key directory and put into `test/fixtures/0-dns`. The domains listed in the cert were also changed into example.com and example.org to show the use for only testing. Fixes: #10228 PR-URL: #11579 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com>

0-dns-cert.pem and 0-dns-key.pem were stored in `test/fixtures/key` directory, but the cert file cannot be created with the openssl command via Makefile. Added a script to create it with using `asn1.js` and `asn1.js-rfc5280` and moved them out of key directory and put into `test/fixtures/0-dns`. The domains listed in the cert were also changed into example.com and example.org to show the use for only testing. Fixes: nodejs/node#10228 PR-URL: nodejs/node#11579 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com>

nodejs-github-bot added the test Issues and PRs related to the tests. label Feb 27, 2017

shigeki added the tls Issues and PRs related to the tls subsystem. label Feb 27, 2017

shigeki assigned indutny Feb 27, 2017

jasnell approved these changes Feb 27, 2017
View reviewed changes

sam-github suggested changes Feb 28, 2017
View reviewed changes

fixup
fd05b71

shigeki force-pushed the create_0-dns_cert branch from 0920c23 to fd05b71Compare March 1, 2017 05:22

gibfahn mentioned this pull request Mar 1, 2017
parallel/test-debug-port-numbers - intermittent failure #11619
Closed

sam-github suggested changes Mar 1, 2017
View reviewed changes

add newline to pem
68cd928

sam-github approved these changes Mar 2, 2017
View reviewed changes

shigeki unassigned indutny Mar 10, 2017

shigeki closed this Mar 10, 2017

italoacasas mentioned this pull request Mar 13, 2017
7.7.3 Release Proposal #11831
Merged

MylesBorins added the land-on-v6.x label Apr 17, 2017

MylesBorins mentioned this pull request Apr 19, 2017
v6.10.3 proposal #12498
Merged

Uh oh!

Uh oh!

Conversation

shigeki commented Feb 27, 2017• edited by gibfahn Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Affected core subsystem(s)

Uh oh!

shigeki commented Feb 27, 2017

Uh oh!

shigeki commented Feb 27, 2017

Uh oh!

shigeki commented Feb 28, 2017

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!

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!

sam-github commented Feb 28, 2017

Uh oh!

shigeki commented Mar 1, 2017

Uh oh!

sam-github left a comment

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!

shigeki commented Mar 2, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shigeki commented Mar 2, 2017

Uh oh!

shigeki commented Mar 7, 2017

Uh oh!

shigeki commented Mar 10, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

shigeki commented Feb 27, 2017•
edited by gibfahn
Loading

shigeki commented Mar 2, 2017•
edited
Loading