-
Notifications
You must be signed in to change notification settings - Fork 53
MLE-30259: Replace rejectUnauthorized:false With Proper CA Certificates #1097
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,10 @@ | ||
| /* | ||
| * Copyright (c) 2015-2025 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved. | ||
| * Copyright (c) 2015-2026 Progress Software Corporation and/or its subsidiaries or affiliates. All Rights Reserved. | ||
| */ | ||
| const fs = require('fs'); | ||
| const path = require('path'); | ||
| const strictTlsVerification = process.env.ML_TEST_SSL_STRICT === 'true'; | ||
|
|
||
| var testHost = 'localhost'; | ||
|
|
||
| var restPort = '8016'; | ||
|
|
@@ -23,7 +27,8 @@ var restEvaluatorPassword = 'x'; | |
|
|
||
| var testServerName = 'node-client-api-ssl-server'; | ||
|
|
||
| // For SSL without client cert, use rejectUnauthorized: false | ||
| // Do NOT disable TLS certificate validation in production. | ||
| // Prefer providing ca with a trusted certificate instead (CWE-295). | ||
| module.exports = { | ||
| testServerName: testServerName, | ||
| testHost: testHost, | ||
|
|
@@ -63,13 +68,16 @@ module.exports = { | |
| password: restAdminPassword, | ||
| authType: manageAuthType | ||
| }, | ||
| // Do NOT disable TLS certificate validation in production. | ||
| // Prefer providing ca with a trusted certificate instead (CWE-295). | ||
| restSslConnection: { | ||
| host: testHost, | ||
| port: restPort, | ||
| user: restAdminUser, | ||
| password: restAdminPassword, | ||
| authType: 'BASIC', | ||
| rejectUnauthorized: false, | ||
| ssl: true | ||
| ssl: true, | ||
| rejectUnauthorized: strictTlsVerification, | ||
| ca: fs.readFileSync(path.join(__dirname, '../test-app/src/main/ml-config/self-signed-ca.pem')) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could not see self-signed-ca.pem is actually validated in the tests anywhere. It seems the certificate for SSL port (8017) is generated by MarkLogic's GenerateTemporaryCertificateCommand (in test-app\build.gradle). So I guess self-signed-ca.pem can't validate anything and may not be needed or replace by a correct certificate. |
||
| } | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| -----BEGIN CERTIFICATE----- | ||
| MIIDMTCCAhmgAwIBAgIUQNezV5ib3hdBYBsrk0yIwMQsiZQwDQYJKoZIhvcNAQEL | ||
| BQAwKDEmMCQGA1UEAwwdTWFya0xvZ2ljLU5vZGUtQ2xpZW50LVRlc3QtQ0EwHhcN | ||
| MjYwNzE0MTY0MTE2WhcNMzYwNzExMTY0MTE2WjAoMSYwJAYDVQQDDB1NYXJrTG9n | ||
| aWMtTm9kZS1DbGllbnQtVGVzdC1DQTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCC | ||
| AQoCggEBALUYP9mZnXWHnWyzf5PXe/gL0JRyDB8ResRveRnWXdy3Uoa9IXiJDfnA | ||
| cq9cIdrG/AP2VmwWFbbqqaYTQLftfwL9b3HYpBaXLg6yvYhAV3+yuQWMMkDk8iFP | ||
| UV5JLTd7/9/RI6zA6jhCXoXdZAAdev+ZX8tqDSVjsfwMLWLERpiA1qk/p9AoRLOE | ||
| +cm6wI5HVaLziX0VUizB4rPGvCLlwG/IjXCDq6rAOh/jskyim3FMSIwqx+69Huxp | ||
| eFXTfsc4ien2/a39mMMZilytTse8JUD2LlhC+SPIIKCz2BxptVcqTIRKhfzH8yC/ | ||
| Uy9rXLl2hCC+19jXUQZH/AHrSbTNNdUCAwEAAaNTMFEwHQYDVR0OBBYEFKqKSeNN | ||
| aCq3avC9IxzvF5Dz2MLPMB8GA1UdIwQYMBaAFKqKSeNNaCq3avC9IxzvF5Dz2MLP | ||
| MA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBAB+w8YTBN4vNYkrP | ||
| NOfq2879vhCMcfBxk5xQk1A9pq/1Pod2zY+k6yH6mPXpXyUzMGF02fqcHXace903 | ||
| gHc+02hBBhxsyETBzoHk3LoPVCDd/493tPMDv8I4Zu25oL6CBxZXp2ZlrxxTIAYV | ||
| bFxZpV0qUUthBfu8aa+eFPFUKL4gnr2VqBqYOlHHkSeIKpb0/niaAIOtUllwXvoo | ||
| cWGnK8fN9xPSyjjtQR6UvKTdJehzNx3gdiz2u05teqZbwVTslEdm+N65Snc/2tkO | ||
| iR2tItCd83PFkJZi1e2T4372ZIuGzysrZjwFO8QXk9zb/wn5nmF8dclZyheBtaKj | ||
| OqEDGxs= | ||
| -----END CERTIFICATE----- |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,8 @@ let should = require('should'); | |
| let marklogic = require('../'); | ||
| const { exec } = require('child_process'); | ||
| const testlib = require("../etc/test-lib"); | ||
| // This suite validates TLS protocol min/max behavior, not certificate trust chain handling. | ||
| testconfig.restConnectionForTls.rejectUnauthorized = false; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why here overwrite the default configuration in test-config.js? The test file re-introduces rejectUnauthorized = false |
||
| let db = marklogic.createDatabaseClient(testconfig.restConnectionForTls); | ||
| let serverConfiguration = {}; | ||
| let host = testconfig.testHost; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would ML_TEST_SSL_STRICT be set anywhere? If ML_TEST_SSL_STRICT is not set, strictTlsVerification would be false. Have you tested ML_TEST_SSL_STRICT=true?