This is just so that this repo can be reviewed #1

jjhoughton · 2019-07-06T21:07:26Z

https://github.com/ripjar/workflow-server/pull/182/files
https://ripjar.atlassian.net/browse/LP-2254

moxonj · 2019-07-08T09:17:31Z

@jfmesse would be a good person to review this

moxonj · 2019-07-09T16:32:39Z

test.js

+`;
+
+try {
+  console.log("My rsa private key is", modulus.RSAPrivateKey(rsakey));


i think you should add a test in this file to make sure you get the same as using the command line

maybe add a benchmark test comparison too

moxonj · 2019-07-09T16:33:31Z

package.json

+  "private": true,
+  "gypfile": true,
+  "scripts": {
+    "start": "node-gyp configure build && node module.js",


its weird to have a start script for a lib like this

It doesn't work it needs deleting

moxonj · 2019-07-09T16:53:29Z

main.c

+  char *buf;
+  napi_status status = napi_ok;
+
+  BIO *bio;


i think you should link to the openssl docs were this came from

moxonj · 2019-07-09T16:54:28Z

main.c

+    }
+  if (napi_get_value_string_utf8 (env, argv[0], NULL, 0, &size) != napi_ok)
+    {
+      napi_throw_error (env, NULL, "Failed to parse string");


can you add more detail in all these errors?

moxonj · 2019-07-09T16:54:38Z

main.c

+    }
+  if (argc < 1)
+    {
+      napi_throw_error (env, NULL, "This function requires one argument");


shouldn't this say what the argument is supposed to be

moxonj · 2019-07-09T16:55:07Z

main.c

+  if (napi_get_value_string_utf8 (env, argv[0], buf,
+				  size, &size) != napi_ok)
+    {
+      napi_throw_error (env, NULL, "Failed to parse string");


this is the same error message as above, id change it so you know where it wrong

It's because it's the same function as above. The one above just checks the size of the string and the second copies the string to a c string

moxonj · 2019-07-09T16:56:11Z

test.js

@@ -0,0 +1,58 @@
+const modulus = require(".");


id probably rename this to lib or something

Yep i changed the name of the project at some point

moxonj · 2019-07-09T16:57:18Z

package.json

+  "gypfile": true,
+  "scripts": {
+    "start": "node-gyp configure build && node module.js",
+    "test": "node test.js"


id have thought you'd use a testing lib to make sure your code meets some expectations, otherwise its jut ascript you run

moxonj · 2019-07-09T16:57:30Z

package.json

+  "version": "0.1.0",
+  "main": "main.js",
+  "private": true,
+  "gypfile": true,


what does this do

Makes it compile when you run yarn

moxonj

i get this error building on woof 2

[ripjar@wf-dev2 node-openssl]$ /apps/libs/node-8/bin/npm install

[email protected] install /home/ripjar/github-ripjar/node-openssl
node-gyp rebuild

make: Entering directory /home/ripjar/github-ripjar/node-openssl/build' CC(target) Release/obj.target/node_openssl/main.o ../main.c:20:22: fatal error: node_api.h: No such file or directory #include <node_api.h> ^ compilation terminated. make: *** [Release/obj.target/node_openssl/main.o] Error 1 make: Leaving directory /home/ripjar/github-ripjar/node-openssl/build'
gyp ERR! build error
gyp ERR! stack Error: make failed with exit code: 2
gyp ERR! stack at ChildProcess.onExit (/apps/libs/node-8.10.0/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:258:23)
gyp ERR! stack at emitTwo (events.js:106:13)
gyp ERR! stack at ChildProcess.emit (events.js:191:7)
gyp ERR! stack at Process.ChildProcess._handle.onexit (internal/child_process.js:219:12)
gyp ERR! System Linux 3.10.0-693.17.1.el7.x86_64
gyp ERR! command "/apps/libs/node-6.13.1/bin/node" "/apps/libs/node-8.10.0/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd /home/ripjar/github-ripjar/node-openssl
gyp ERR! node -v v6.13.1
gyp ERR! node-gyp -v v3.6.2
gyp ERR! not ok
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] install: node-gyp rebuild
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the [email protected] install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm WARN Local package.json exists, but node_modules missing, did you mean to install?

npm ERR! A complete log of this run can be found in:
npm ERR! /home/ripjar/.npm/_logs/2019-07-09T17_03_40_404Z-debug.log

jjhoughton · 2019-07-10T08:30:07Z

It compiles alright you just need to change the PATH to use node 8 like so

[ripjar@wf-dev2 node-openssl]$ export PATH=/apps/libs/tmux/bin:/apps/libs/redis/bin:/apps/libs/node-8/bin:/apps/libs/node-10/bin-10:/apps/libs/mongodb-4.0.7/bin:/apps/libs/git/bin:/apps/libs/autossh/bin:/usr/local/bin:/usr/bin:/usr/local/sbin:/usr/sbin:/home/ripjar/.local/bin:/home/ripjar/bin
[ripjar@wf-dev2 node-openssl]$ ls
asm.c  asm.s  binding.gyp  lgpl-2.1.txt  main.c  main.js  main.s  node_modules  package.json  README.md  test.js  yarn.lock
[ripjar@wf-dev2 node-openssl]$ yarn
yarn install v1.3.2
[1/5] Validating package.json...
[2/5] Resolving packages...
success Nothing to install.
success Saved lockfile.
$ node-gyp rebuild
gyp info it worked if it ends with ok
gyp info using [email protected]
gyp info using [email protected] | linux | x64
gyp info spawn /usr/bin/python2
gyp info spawn args [ '/apps/libs/node-8.10.0/lib/node_modules/npm/node_modules/node-gyp/gyp/gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'make',
gyp info spawn args   '-I',
gyp info spawn args   '/home/ripjar/node-openssl/build/config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/apps/libs/node-8.10.0/lib/node_modules/npm/node_modules/node-gyp/addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/home/ripjar/.node-gyp/8.10.0/include/node/common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=/home/ripjar/.node-gyp/8.10.0',
gyp info spawn args   '-Dnode_gyp_dir=/apps/libs/node-8.10.0/lib/node_modules/npm/node_modules/node-gyp',
gyp info spawn args   '-Dnode_lib_file=/home/ripjar/.node-gyp/8.10.0/<(target_arch)/node.lib',
gyp info spawn args   '-Dmodule_root_dir=/home/ripjar/node-openssl',
gyp info spawn args   '-Dnode_engine=v8',
gyp info spawn args   '--depth=.',
gyp info spawn args   '--no-parallel',
gyp info spawn args   '--generator-output',
gyp info spawn args   'build',
gyp info spawn args   '-Goutput_dir=.' ]
gyp info spawn make
gyp info spawn args [ 'BUILDTYPE=Release', '-C', 'build' ]
make: Entering directory `/home/ripjar/node-openssl/build'
  CC(target) Release/obj.target/node_openssl/main.o
  SOLINK_MODULE(target) Release/obj.target/node_openssl.node
  COPY Release/node_openssl.node
make: Leaving directory `/home/ripjar/node-openssl/build'
gyp info ok 
Done in 0.79s.

jjhoughton · 2019-07-10T08:33:35Z

main.c

+
+  if (napi_create_function (env, NULL, 0, rsa_priv_key,
+                            NULL, &rsa_fn) != napi_ok)
+    napi_throw_error (env, NULL, "Unable to wrap native rsa function");


These should have return statements all of them. I don't think it stop when it hits an error

jjhoughton · 2019-07-10T10:12:20Z

Drum thinks i should add some comments describing what section is what. I agree

Signed-off-by: James Moxon <[email protected]> Signed-off-by: Joshua Houghton <[email protected]>

Joshua Houghton and others added 10 commits July 6, 2019 22:06

initial commit

2111a4a

returns all the things about an rsa key

2a6660a

package.json: change package name

97776de

add lgpl license

e8f5235

add some x509 stuff

3d60bb9

get public key from x509 cert

c30fa20

change the name of some files

f9970b9

add some missing files

3a66b85

fix typo in error message

8f44825

add readme

ef764c3

jjhoughton requested review from moxonj and sandra-d July 6, 2019 21:24

jjhoughton added 3 commits July 7, 2019 13:39

sign copyright over to ripjar

243fecd

consider the size of the cert/key before allocating memory

9ed5dc5

do a bit more error checking

ec99d71

jjhoughton force-pushed the master branch from f762e07 to ec99d71 Compare July 7, 2019 15:35

moxonj reviewed Jul 9, 2019

View reviewed changes

main.c

char *buf;

napi_status status = napi_ok;

BIO *bio;

Copy link

moxonj Jul 9, 2019

Choose a reason for hiding this comment

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

i think you should link to the openssl docs were this came from

moxonj reviewed Jul 9, 2019

View reviewed changes

moxonj requested changes Jul 9, 2019

View reviewed changes

jjhoughton commented Jul 10, 2019

View reviewed changes

jjhoughton and others added 5 commits November 26, 2019 10:06

add the ability to specify a passphase for the private key (#2)

7e4eddd

Signed-off-by: James Moxon <[email protected]> Signed-off-by: Joshua Houghton <[email protected]>

fix rsa_st not being defined in node12

a9e37ba

Get working on openssl 3.0

0e6d9ef

Automatically synchronising release.yml from develop to master

cc8b710

Updates .github/workflows/release.yml with new #li-releases channel id

c11f236

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

This is just so that this repo can be reviewed #1

This is just so that this repo can be reviewed #1

jjhoughton commented Jul 6, 2019 •

edited

Loading

moxonj commented Jul 8, 2019

moxonj Jul 9, 2019

moxonj Jul 9, 2019

moxonj Jul 9, 2019

jjhoughton Jul 10, 2019

moxonj Jul 9, 2019

moxonj Jul 9, 2019 •

edited

Loading

moxonj Jul 9, 2019 •

edited

Loading

moxonj Jul 9, 2019

jjhoughton Jul 10, 2019

moxonj Jul 9, 2019

jjhoughton Jul 10, 2019

moxonj Jul 9, 2019

moxonj Jul 9, 2019

jjhoughton Jul 10, 2019

moxonj left a comment

jjhoughton commented Jul 10, 2019 •

edited

Loading

jjhoughton Jul 10, 2019

jjhoughton commented Jul 10, 2019

This is just so that this repo can be reviewed #1

Are you sure you want to change the base?

This is just so that this repo can be reviewed #1

Conversation

jjhoughton commented Jul 6, 2019 • edited Loading

moxonj commented Jul 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moxonj Jul 9, 2019 • edited Loading

Choose a reason for hiding this comment

moxonj Jul 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moxonj left a comment

Choose a reason for hiding this comment

jjhoughton commented Jul 10, 2019 • edited Loading

Choose a reason for hiding this comment

jjhoughton commented Jul 10, 2019

jjhoughton commented Jul 6, 2019 •

edited

Loading

moxonj Jul 9, 2019 •

edited

Loading

moxonj Jul 9, 2019 •

edited

Loading

jjhoughton commented Jul 10, 2019 •

edited

Loading