Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LibreSSL Support #5

Merged
merged 7 commits into from Jan 23, 2018
Merged

LibreSSL Support #5

merged 7 commits into from Jan 23, 2018

Conversation

bompi88
Copy link
Contributor

@bompi88 bompi88 commented Jan 19, 2018

Generate static config files to support LibreSSL (issue #4 ). Also do not provide random generator files if LibreSSL is detected, as it's not a viable CLI option. @erossignon If there's something you want me to change, let me know.

@bompi88 bompi88 mentioned this pull request Jan 19, 2018
@coveralls
Copy link

coveralls commented Jan 19, 2018

Coverage Status

Coverage increased (+1.1%) to 86.216% when pulling bfc1eba on bompi88:libressl into 5c076b5 on node-opcua:master.

@erossignon
Copy link
Member

Thank you @bompi88 , would it be possible to adapt the travis script so we run tests on MacOS ?
That would be awesome.

@@ -469,7 +478,9 @@ CertificateAuthority.prototype.verifyCertificate = function(certificate_file,cal

var options = {cwd: self.rootDir};
toolbox.setEnv("OPENSSL_CONF", toolbox.make_path(self.configFile));
Copy link
Member

Choose a reason for hiding this comment

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

Could we avoid code duplication ?

@@ -264,6 +273,33 @@ function displaySubtitle(str, option_callback) {

exports.displaySubtitle = displaySubtitle;

function getEnvironmentVarNames() {
return Object.keys(process.env).map(function(varName) { return { key: varName, pattern: '\\$ENV\\:\\:' + varName }; });
Copy link
Member

Choose a reason for hiding this comment

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

Would be good to export only env variable that are set by us , not the one that exists on the system .
( this could create a backdoor)

@@ -341,21 +341,21 @@ CertificateAuthority.prototype.revokeCertificate = function(certificate,params,c
"certificateHold", "removeFromCRL"
];

var options = {cwd: self.rootDir , openssl_conf:toolbox.make_path(self.configFile) };
var configFile = toolbox.generateStaticConfig("conf/caconfig.cnf", { cwd: self.rootDir });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@erossignon Should conf/caconfig.cnf be self.configFile?


if (["OPENSSL_CONF"].indexOf(varName) >= 0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@erossignon I assume OPENSSL_CONF is needed in systems` environment vars?

@bompi88
Copy link
Contributor Author

bompi88 commented Jan 22, 2018

@erossignon I see I triggered a lot of builds in travis, which seems to stall, feel free to cancel all except the last one. Seems like macos builds is not always starting and hangs for a long time. Should maybe consider only linux build matrix with openssl and libressl.

@erossignon erossignon merged commit cd20836 into node-opcua:master Jan 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants