Skip to content

[v2.1638-vsc1.39.2] Unable to start latest CS with HTTPS #1109

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

Closed
antofthy opened this issue Oct 25, 2019 · 10 comments
Closed

[v2.1638-vsc1.39.2] Unable to start latest CS with HTTPS #1109

antofthy opened this issue Oct 25, 2019 · 10 comments
Labels
bug Something isn't working

Comments

@antofthy
Copy link

antofthy commented Oct 25, 2019

  • code-server version: 2.1638-vsc1.39.2
  • OS Version: Debian Buster Container (php-apache), running on a RHEL7 docker swarm

Description

Release notes state...

If you aren't doing SSL termination elsewhere you can directly give code-server a certificate with code-server --cert followed by the path to your certificate. Additionally, you can use certificate keys with --cert-key followed by the path to your key. If you pass --cert without any path code-server will generate a self-signed certificate.

code-server --help says...

--cert Path to certificate. If the path is omitted,
both this and --cert-key will be generated.

Neither of which has changed from the previous beta release 2.1523-vsc1.38.1

So I run with the same arguments that was working before...

PASSWORD=xxxx code-server .  --port 8443 --auth password --cert '' --disable-telemetry

But it fails to set up a HTTPS with self-signed certificate

info  Server listening on http://localhost:8443
info    - Using custom password for authentication
info    - Not serving HTTPS

Which brings us back to a point I made before in #1052 , now closed.
How can you specify no path for --cert option without that option being the last option in the command line?

Assuming that is the problem, I made the --cert option the last option with nothing following...

PASSWORD=xxxxx code-server .  --port 8443 --auth password --disable-telemetry --cert

produces the same result

ASIDE: I have noted before that I no longer not need to specify --auth password as it is again the default, as it was for CS v1

@antofthy antofthy added the bug Something isn't working label Oct 25, 2019
@featherbear
Copy link

This behaviour is caused by this condition on vscode/src/vs/platform/environment/node/argv.ts#L184

String properties require an argument, so the condition fails and the key is not added.

Perhaps we could modify the process.argv in [node/cli.ts#L86] and prefix a dummy character so that the argument will be parsed - but that's probably error prone

@featherbear
Copy link

PR merged into October 2019 milestone

@code-asher
Copy link
Member

Yeah it broke when we updated to VS Code 1.39.2. I ended up just manually checking the arguments for the existence of --cert. #1101

Will have a new release out before the end of today.

@code-asher
Copy link
Member

Although the "proper" solution would probably be to add a new type that can be either boolean or string and parse it accordingly.

@featherbear
Copy link

Might run in to lexical parsing issues though

@code-asher
Copy link
Member

I did some testing and I think the only issue we have is if something like this is done:

code-server -- --cert

Where we're trying to open a directory called --cert. So we should probably only check up to a --.

I was also concerned about something like:

code-server --base-path "--cert"

Where the base path is literally --cert but it seems the parser in this case still parses these as two options so it seems we're OK here.

There might be other scenarios I'm missing; do you have any thoughts?

@featherbear
Copy link

featherbear commented Oct 25, 2019

I did some testing and I think the only issue we have is if something like this is done:

code-server -- --cert

Where we're trying to open a directory called --cert. So we should probably only check up to a --.

I was also concerned about something like:

code-server --base-path "--cert"

Where the base path is literally --cert but it seems the parser in this case still parses these as two options so it seems we're OK here.

There might be other scenarios I'm missing; do you have any thoughts?

Yup, as long as it's enclosed it won't be matched.

But yeah, I think it's better to address the parsing than to monkey patch a fix


I think b8e6369 introduces some unwanted rigidity into the software (Checking on command line args). Perhaps it should be extensible to allow all of the option flags to be set via environment variables

PR has been merged into vscode microsoft/vscode@f2c1232 and I believe that will break your check.
Any explicitly defined argument (cert) will now be passed into the returned arguments.

@code-asher
Copy link
Member

Ahhh I see what you're saying. Thanks for the heads up on that PR!!

So we could just revert the aforementioned monkey-patch and then patch in the fix from microsoft/vscode@f2c1232 until it makes its way into code-server properly.

code-asher added a commit that referenced this issue Oct 25, 2019
@antofthy
Copy link
Author

antofthy commented Oct 28, 2019

So after all this...

How to you specify --cert with no argument? Especially if other options follow?

Give it a blank argument as I have done in the past? Or just a bad filename?
Or its a filename without a full path (starting with '/')?
Or does it know that if the next argument starts with a '--' is it an option and their is no argument?

It isn't just that it failed, it is also documenting how to specify no argument!

In my thinking adding an option of -cert-self-signed would make it clear,
without inconsistencies, or problems in trying to document it.

Tesing new release....
--cert '' is working as it did before in code-server2.1650-vsc1.39.2

@featherbear
Copy link

featherbear commented Oct 28, 2019

@antofthy You should be able to do either --cert (path omitted) or --cert '' (empty path).
The handling of arguments are done by vscode's argument parser

$> ./code-server --cert --port 1111
info  Server listening on https://localhost:1111
info    - Password is b6d827dd8816cea54a9169a9
info      - To use your own password, set the PASSWORD environment variable
info      - To disable use `--auth none`
info    - Using generated certificate and key for HTTPS
$> ./code-server --cert "" --port 2222
info  Server listening on https://localhost:2222
info    - Password is 1608e07b2adac35565001d46
info      - To use your own password, set the PASSWORD environment variable
info      - To disable use `--auth none`
info    - Using generated certificate and key for HTTPS

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants