Skip to content

Read root certificates from OS cert stores #412

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

Open
jglogan opened this issue Feb 14, 2023 · 11 comments
Open

Read root certificates from OS cert stores #412

jglogan opened this issue Feb 14, 2023 · 11 comments
Assignees
Labels
feature-request New feature or request

Comments

@jglogan
Copy link

jglogan commented Feb 14, 2023

OS: Mac OS 13.1
VS Code: 1.71.1
Dev Containers extension: 0.279.0
Dev container CLI: 0.29.0

I submitted an issue on the VS Code remote extension repo but it wasn't getting any responses, so trying here...

I've created and published a feature to our internal Artifactory repo named docker.xyz.com/john/devcontainer-features/test-alpine:0, and confirmed that I can launch via the devcontainer CLI a devcontainer that refers to this feature.

When I try to open in a container using the same devcontainer.json with VS Code, it fails when trying to run devcontainer read-configuration.

When I try to run that failing command from a shell, it works fine. I'm going to have a close look at the code but was wondering if anyone else has encountered this and found a workaround?

FAILURE LOG: failure.log

SUCCESS LOG: success.log

@jglogan
Copy link
Author

jglogan commented Feb 15, 2023

I added a log message here and installed the modified devContainersSpecCLI.js, and I now see the self-signed cert error below.

My OS certificate configuration includes this trusted root, and when invoking the CLI from a shell, there's no problem picking it up. It seems that there is an issue with certificate handling in the Code Helper (Plugin) --ms-enable-electron-run-as-node invocation context. Could there be a problem with Electron not using the OS certificate config in this invocation?

[2023-02-15T17:57:47.021Z] [httpOci] ENTER requestEnsureAuthenticated
[2023-02-15T17:57:47.165Z] Exception message self signed certificate in certificate chain
[2023-02-15T17:57:47.165Z] Failed to parse JSON with mimeType 'application/vnd.oci.image.manifest.v1+json': 

@chrmarti
Copy link
Contributor

You are using Node 18 when it succeeds which might have updated root certificates built-in when compared to Node 16 as shipped with the Electron version you use when it fails. Could the root certificate have been added or is it private?

NodeJS ships with the Mozilla cert store built-in and does not read the OS cert store from what I know. (For VS Code extensions we found a way to add the OS certs to NodeJS, but these are not carried over to the CLI we spawn from the extension.)

@jglogan
Copy link
Author

jglogan commented Feb 20, 2023

You're right. node as invoked from my shell uses a modified cert store that includes the additional root.

Is there any workaround that would allow us to do the same for the CLI invoked from the remote containers? I'd love to show other developers in my org how to make good use of dev container features, but it's a bit difficult with this issue standing in the way.

@jglogan
Copy link
Author

jglogan commented Feb 20, 2023

@chrmarti Thanks for the suggestion...I've confirmed that this crude hack does allow open in container from VSCode when I replace .vscode/extensions/ms-vscode-remote.remote-containers-0.279.0/dist/spec-node/devContainersSpecCLI.js. Not really a good solution but it does demonstrate exactly what you asserted.

diff --git a/src/spec-utils/httpRequest.ts b/src/spec-utils/httpRequest.ts
index 9c5b30a..481e5b0 100644
--- a/src/spec-utils/httpRequest.ts
+++ b/src/spec-utils/httpRequest.ts
@@ -9,6 +9,9 @@ import ProxyAgent from 'proxy-agent';
 import * as url from 'url';
 import { Log, LogLevel } from './log';
 
+import fs from 'fs';
+const caCertPem = fs.readFileSync('/etc/ssl/cert.pem');
+
 export function request(options: { type: string; url: string; headers: Record<string, string>; data?: Buffer }, output?: Log) {
        return new Promise<Buffer>((resolve, reject) => {
                const parsed = new url.URL(options.url);
@@ -19,6 +22,7 @@ export function request(options: { type: string; url: string; headers: Record<st
                        method: options.type,
                        headers: options.headers,
                        agent: new ProxyAgent(),
+                       ca: caCertPem,
                };
 
                const plainHTTP = parsed.protocol === 'http:' || parsed.hostname === 'localhost';
@@ -58,6 +62,7 @@ export function headRequest(options: { url: string; headers: Record<string, stri
                        method: 'HEAD',
                        headers: options.headers,
                        agent: new ProxyAgent(),
+                       ca: caCertPem,
                };
 
                const plainHTTP = parsed.protocol === 'http:' || parsed.hostname === 'localhost';
@@ -89,6 +94,7 @@ export function requestResolveHeaders(options: { type: string; url: string; head
                        method: options.type,
                        headers: options.headers,
                        agent: new ProxyAgent(),
+                       ca: caCertPem,
                };
 
                const plainHTTP = parsed.protocol === 'http:' || parsed.hostname === 'localhost';

@chrmarti chrmarti changed the title read-configuration works from command line but fails in VS Code "open in container" Read root certificates from OS cert stores Feb 21, 2023
@chrmarti chrmarti added feature-request New feature or request and removed info-needed Issue requires more information from poster labels Feb 21, 2023
@chrmarti
Copy link
Contributor

We might be able to reuse part of what we do for VS Code in https://github.com/microsoft/vscode-proxy-agent.

@jglogan
Copy link
Author

jglogan commented Feb 24, 2023

@chrmarti Thank you for your help! I'm not familiar with how new features get taken in, is there any way for me to know how soon this feature could be added? Is this a VS Code Remote Dev change, or a dev container CLI change?

@chrmarti
Copy link
Contributor

This would be only a dev container CLI change. (The repo referenced above might help us get there.)

We will comment here once we make progress. (Not scheduled at the moment.)

@erichaydel
Copy link

Is there any idea of when this will make the schedule? We're also having issues at my org with this since we have a proxy.

@swirle13
Copy link

@chrmarti

PRs merged into node added support for macOS Keychain certs via nodejs/node#56599 and support for windows via nodejs/node#56833

These changes are included in Node v23.8.0 for mac and windows and v23.9.0 for other platforms and backported to v22.15.0 LTS as of 7 hours ago (complete coincidence). Latest as of this writing is v23.11.0 and v22.15.0.

Currently, my devcontainers/cli is v0.75.0 which uses Node.js v20.18.3. Is there any way we could get a bump to LTS v22.15.0 to fix this issue upstream? I'd be interested in attempting a PR, but I haven't contributed to this project before and worry I'd be much slower than others may be.

@chrmarti
Copy link
Contributor

@swirle13 You can run the CLI standalone with the newer Node.js version. For VS Code we will have to wait for this change to make it into a new Electron version which we can then pick up. (I don't have an ETA for that at the moment.)

@chrmarti
Copy link
Contributor

These changes are being backported to Node.js 22.x: nodejs/node#57840. That should also speed up the arrival in Electron.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants