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

Add flag to ignore SSL certificate issues #1371

Closed
Maxim-Mazurok opened this issue Dec 18, 2018 · 32 comments
Closed

Add flag to ignore SSL certificate issues #1371

Maxim-Mazurok opened this issue Dec 18, 2018 · 32 comments
Labels
cli related to cli/ dir feat new feature (which has been agreed to/accepted)
Milestone

Comments

@Maxim-Mazurok
Copy link
Contributor

There should be a way to allow insecure https requests, using window.fetch() for example.
It should disable certificate validation for all requests made from the program.

As of implementation, there are two optoins that come to my mind: environment variable and flag. Below are some examples of how it's done in other web-related software, so we can come up with intuitive name and approach.

Examples:

  • In NodeJS, there is a NODE_TLS_REJECT_UNAUTHORIZED environment variable that can be set to 0 to disable TLS certificate validation. It takes name from tls.connect options.rejectUnauthorized
  • In curl, there's a -k or --insecure flag, which allows insecure server connections when using SSL
  • In Chrome, there's a --unsafely-treat-insecure-origin-as-secure="https://example.com" for the same purposes
  • For WebDrivers, there's acceptInsecureCerts capability. It can allow self-signed or otherwise invalid certificates to be implicitly trusted by the browser.

That said, I'd like to see --accept-insecure-certs flag option, because deno already uses flags for a lot of things, including permissions.


My use-case: I'm using a corporate Windows laptop with network monitoring. AFAIK, all https requests are also monitored, so there's a custom SSL-certificate installed system-wide. So, most of the software, that uses system CA storage, works just fine. But some uses custom or bundled CA, and it seems like it's a case with deno. Anyway, it downloads deps just fine, but fails to perfprm any https request.

Issue created as a follow up to gitter conversation: December 18, 2018 1:24 PM

@ry ry added this to the v0.3 milestone Dec 18, 2018
@eventualbuddha
Copy link

Seems like it'd be better to allow adding new CA entries that requests would trust, no? Having this flag is probably fine in exceptional circumstances, but for any production system leaving this flag on all the time is a huge red flag.

@hayd
Copy link
Contributor

hayd commented Dec 19, 2018

I wonder if this ought to share a prefix i.e. --allow? Would it be added to DenoPermissions, would it be "upgradeable" at runtime?

Related: #1064

@Maxim-Mazurok
Copy link
Contributor Author

@eventualbuddha yes, I'd like to have that option too. I mean, for some non-production cases, it might be better to completely disable any checks. And later, you might want to add new CA entries when preparing the code for production.

@hayd I made more research on that because I need to make it work already. So, I discovered, that these checks are performed in rustls. And as far as I can tell, we'll be unable to disable certificate checks, or add custom CA's unless this lib is built with "dangerous_configuration" feature turned on. After enabling it, we can create Connector with custom ClientConfig that has custom ServerCertVerifier. It should be done in //src/http_util.rs -> fn get_client.
That said, I'm not sure is it a good idea at all, to always build deno with dangerous_configuration feature. It sounds... "dangerous" :) But also I don't like locking CAs using webpki-roots with static CAs list, which is used by rustls. I'm more then sure, that if @ry wants deno to be adopted by corporations, it might be really vital to at least add an option to support system CAs, so developers on machines with monitored traffic by an employer, can also develop with deno.
So, even tho, adding option for custom CAs will require building rustls with dangerous_configuration feature, I think we should consider doing so.

Back to @hayd questions, I might be wrong, but I think that ideally, deno should pick up system CAs instead of bundled CAs without any additional flags. AFAIK, it will be a more browser-like approach.
If @ry decides to go with the flag instead, I don't think that it should be a permission, but rather a configuration. If we take a look at Chromes behavior, it will allow visiting the website with a bad certificate after we choose to proceed, but all insecure requests from that page to other sources with bad certificates will fail without any prompts and there's no way to allow them in "upgradable" manner.

Please, let me know what do you think about it?

@ry ry modified the milestones: v0.3, future Jan 4, 2019
@Maxim-Mazurok
Copy link
Contributor Author

Maxim-Mazurok commented Jan 16, 2019

As an update to this, I have a working branch, and it works fine, but I'm not ready to open a PR yet. Anyway, it's my first time coding in rust, so I expect a lot of help from code review. Currently, the branch is here: https://github.com/Maxim-Mazurok/deno/tree/accept-insecure-certs and it adds --accept-insecure-certs flag support. It's not considered permission, and thus, not upgradable during run-time. Someone might find it helpful. Luckily, I'm no longer an employee of company where I had to use Windows laptop with custom CA certs and I'm back to my lovely personal CentOS laptop :) So that issue is not critical for me anymore, but still I want to finish what I've started in hope that it'll help somebody else to adopt deno for their project. And in case if anyone is looking for an employee in SF Bay Area, I'm open for new opportunities :)

@anxiousmodernman
Copy link

anxiousmodernman commented Jun 8, 2019

Chiming in that the Go tls.Config kind of supports both strategies.

  1. An all-bets-off InsecureSkipVerify boolean that seems analogous to curl's -k
  2. More detailed configuration: A boatload of hooks and fields for trusting additional CAs, and actually providing callbacks for inspecting connections at runtime (e.g. GetConfigForClient). Lots more. Easily the nicest TLS experience I've ever worked with.

Docs here: https://golang.org/pkg/crypto/tls/#Config

We could start with something like number 1, but leave the door open for more granular configuration.

My biggest worry here is that TLS in rust is kind of rough as it stands. We will be bound to the underlying implementation chosen, but we should take care to avoid letting the underlying implementation leak into the TypeScript API.

My dream would be a Typescript interface that roughly maps onto what Go does, and each capability added would be a runtime escalation just like allow-network. But we can start simple, e.g.

export interface TLSConfig {
    // defaults to false
    insecureSkipVerify?: boolean;
}

@geoFlux
Copy link
Contributor

geoFlux commented Feb 3, 2020

It looks like deno is now using the reqwest library, which claims:

By default, a Client will make use of system-native transport layer security to connect to HTTPS destinations. This means schannel on Windows, Security-Framework on macOS, and OpenSSL on Linux.

But I'm still getting certificate errors when behind a corporate proxy. It looks like deno is configured to use rustls-tls instead of default-tls or native-tls. I think that's why it's still not working. Is there a reason to not use default-tls or native-tls?

I'm looking here. Specifically this line:

reqwest = { version = "0.10.1", default-features = false, features = ["rustls-tls", "stream", "gzip"] }

Seems like you could just change it to

reqwest = { version = "0.10.1", default-features = false, features = ["native-tls", "stream", "gzip"] }

And that would fix it.

@kushdilip
Copy link

Is there any update on this issue, because I am facing this error when calling internally hosted reviewboard api using fetch while on VPN.
The error I get is

error: Uncaught Http: error sending request for url (https://[CUSTOM_HOST]/api/users/): error trying to connect: tls handshake eof
    at unwrapResponse ($deno$/ops/dispatch_json.ts:43:11)
    at Object.sendAsync ($deno$/ops/dispatch_json.ts:98:10)
    at async fetch ($deno$/web/fetch.ts:591:27)

deno --version

deno 1.0.2
v8 8.4.300
typescript 3.9.2

@jakajancar
Copy link

I'm not sure about a flag, but at least ConnectOptions should support ignoring SSL problems. Unless I'm missing something, there is currently no way to connect to a server with a self-signed certificate.

crookse added a commit to drashland/wocket that referenced this issue Jun 21, 2020
See denoland/deno#1371 for issues with TLS and fetch. There are other issues on Deno about TLS and fetch as well.
@ebebbington
Copy link
Contributor

Would love to see something like this implemented, i'm trying to run integration tests for a web app to ensure serveTls works, but all fetch calls (to test the endpoint works) fail

@silverwind
Copy link

There needs to be both a environment variable and a fetch option for this. The former should be chosen over an cli argument for cases when one does not have control of the arguments passed to deno, which certainly will be the case sometimes.

@stale
Copy link

stale bot commented Jan 6, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 6, 2021
@Maxim-Mazurok
Copy link
Contributor Author

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Occuring an activity :)

@stale stale bot removed the stale label Jan 6, 2021
@kitsonk kitsonk added cli related to cli/ dir feat new feature (which has been agreed to/accepted) labels Jan 7, 2021
@luolong
Copy link

luolong commented Jan 20, 2021

There needs to be both a environment variable and a fetch option for this.

I really want this too! While being safe by default is nice and all, I wish there was a way I could override this on an individua fetch call basis.

@remdragon
Copy link

this would be helpful in multiple scenarios such as running unit tests against a development version of a local webserver. Additionally, interacting with an internal server where a real cert isn't needed.

@peakea
Copy link

peakea commented Mar 14, 2021

This is necessary in isolated development scenarios and causes a headache when trying to create quick demo systems post production

@artutra
Copy link

artutra commented Apr 8, 2021

This would be really useful for my crawler application. I need to download a lot of pages from a website that have an insecure certification, but I cant fix it. I am obligated to use this website even with a insecure flag on it (its literally a government site)

@roseMix
Copy link

roseMix commented Apr 25, 2021

  • Virtually every library (which doesn't use fetch) has a flag for ignoring SSL errors including: Axios and Curl, so obviously there is a need at least for development (and even for production) for such a functionality.

  • All the browsers have an option (interactively) to ignore ssl errors. It would just make sense and be amazing if Deno can do the same for fetch, since fetch is the real Deno way of accessing web resources.

As an example:
I have this usecase, I have a service running Deno which creates new Plesk servers in Vultr programmatically , I can start initial plesk "init" from REST API via a fetch to the server IP, and have Plesk install the ssl but since at init it gets installed with a default cert (which doesn't match IP) it is impossible to so now.

@roseMix
Copy link

roseMix commented Apr 25, 2021

Just found this:
#5931 (comment)

I believe this would solve this issue

@cryptogohan
Copy link

cryptogohan commented May 21, 2021

Went way down the rabbit hole on this one 😩 . Digital Ocean, but it seems Heroku, and Google Cloud too use self signed certs to secure a postgres DB connection. Sadly, passing that cert is not enough as something goes wrong. Two pg drivers available for Deno (postgres, pg) show the same error when trying to write to the TLS connection:

error: Uncaught (in promise) UnexpectedEof: tls handshake eof
    await Deno.writeAll(this.conn, writer.buffer);
    ^
    at deno:core/core.js:86:46
    at unwrapOpResult (deno:core/core.js:106:13)
    at async write (deno:runtime/js/12_io.js:107:12)
    at async Object.writeAll (deno:runtime/js/13_buffer.js:224:19)
    at async Connection.startup (https://deno.land/x/pg@v0.6.0/connection.ts:61:5)
    at async Connection.connect (https://deno.land/x/pg@v0.6.0/connection.ts:45:7)
    at async Client.connect (https://deno.land/x/pg@v0.6.0/client.ts:31:5)
    at async file:///Users/cryptogohan/code/twitter_scraper/connect.ts:13:1

Using an insecure connection with a DB is not an option for me. Deno has no way to bypass the cert check, many hours later I'm left with not using Deno and rewriting which is v. sad as I was having an amazing time with it!

Thanks for all the hard work so far, hopefully, this one makes it in. Or one of the suggestions in #5931 , or possibly #7660 .

@TheAifam5
Copy link
Contributor

So even I went into a rabbit hole and without this feature can't get out.

Basically I've spent days on developing a tool for parsing templates and expression compiler using Deno. Now having everything set up, wanted to write the part for handling those templates and sending data to the server in the VM with self-signed certificate - no there is no way to disable SSL on that service (basically a restriction of the software). That's my story, I've should research more about Deno. Since this feature is since 2018 open I don't feel that something will move right now so I am forced to rewrite everything (don't want to use Node).

Awesome tool tho, but misses some core functionality :/

@lucacasonato
Copy link
Member

lucacasonato commented Jun 14, 2021

@TheAifam5 The solution for using self signed certificates is not to disable verification, but instead to pass the self signed CA certificate to --cert so it can be included in the verification logic.

@Maxim-Mazurok
Copy link
Contributor Author

@lucacasonato are you describing a potential solution or something that deno already supports? It didn't have --cert flag last time I've checked...

$ deno --cert some.cert
error: Found argument '--cert' which wasn't expected, or isn't valid in this context

@lucacasonato
Copy link
Member

Deno has had a --cert flag since 1.0. You must specify it after the subcommand.

@cryptogohan
Copy link

For those ending up here trying to connect to databases, I started a discussion over here and identified some work to be done to make things work. I'll also update as things start working. Slow going for now, but may already contain hints for people with SSL issues.

For theaifam5 passing a cert should already do the trick! Either as a flag or when making the connection for whatever request you're making.

@TheAifam5
Copy link
Contributor

TheAifam5 commented Jun 16, 2021

The --cert does not work for whatever reason and I still get the same issue. Similar to the #8939 which got just closed without even waiting if the solution provided actually solves the problem.

Before anyone asks, yes I extracted the certificate though a browser and through a CertMgr and I have it installed on my machines with a private key.

Maybe because of "fetch"?

Same thing with "connectTls" but also throws when "127.0.0.1" set as the hostname, or when leaved to default - error "invalid hostname".
Back to the issue - "connectTls" does not solve the problem for me either.

I tried both, as a flag and in the code with the „connectTls“.

Tried the @cryptogohan scripts and says that server does not like ssl - also takes a long time to process.

The application where I try to connect is the Microsoft Azure CosmosDB Emulator using the SQL API (through HTTPS). Maybe someone have it running and can verify my issue.

I can not find better alternative for Deno :D Working on implementation for this feature.

UPDATE (18 Jun 2020 at 02:02 AM):
I've managed to connect into CosmosDB with connectTls API, ignoring the certificate:
image
image

now the fetch part.

I've added --no-check-certificate which can be empty or have a list of domains + as well in the code as you've seen. The integration part between --no-check-certificate and connectTls is not done yet - does not get those values from the CLI args, but the fetch does.

UPDATE (18 Jun 2020 at 02:42 AM):
Got fetch working, little bit hacky because of seanmonstar/reqwest#1210. I hope reqwest will release soon.

image

If someone can tell me how to access Flags from op_connect_tls to combine or set the default value for "noCheckCertificate" (in the connectTls) using the value from the CLI, would really help a lot. Maybe a better idea? Basically ELI5 because I have completely no knowledge about the code base.

Fork maybe on Friday or Saturday :) PR is slowly incoming.

@ry ry modified the milestones: future, 1.12.0 Jun 21, 2021
@bartlomieju bartlomieju mentioned this issue Jun 28, 2021
23 tasks
@bartlomieju
Copy link
Member

Hey @TheAifam5, I was supposed to look into implementing this flag for 1.12 but I see you got it more or less working. To answer your question:

If someone can tell me how to access Flags from op_connect_tls to combine or set the default value for "noCheckCertificate" (in the connectTls) using the value from the CLI, would really help a lot. Maybe a better idea? Basically ELI5 because I have completely no knowledge about the code base.

You can't access Flags in op_connect_tls. Instead you want to pass the option from Flags when ops are registered (inside runtime/worker.rs or runtime/web_worker.rs), ie. change deno_net::init::<Permissions>(options.unstable) to deno_net::init::<Permissions>(options.unstable, allow_unsafe_certs).

Please open a PR, even if it doesn't yet completely work, so we can discuss on code, seems like you are close to the solution.

@TheAifam5
Copy link
Contributor

TheAifam5 commented Jul 7, 2021

@bartlomieju that was it :D thank you for giving me a hint. I do not have tests written yet.
So basically, the MVP is finished, needs some tweaking and adjustments and cleanups.

I've created a draft PR as you requested, also a PR contains an important notice: #11324

@bartlomieju
Copy link
Member

With #11324 landed, which adds --unsafely-treat-insecure-origin-as-secure flag similar to Chrome we now have ability to ignore SSL certificate validation errors. This flag can be used to ignore all errors, or it can take list of domain names to ignore only specific errors.

I consider this issue resolved now.

@LeoDog896
Copy link
Contributor

By the way, the option is now --unsafely-ignore-certificate-errors.

@denizdogan
Copy link

Is this supposed to also accept weak TLS ciphers? No matter what I try, I can't make Deno speak to a server with a self-signed certificate which also uses weak TLS ciphers. Unfortunately I can't provide an example of this because it's a non-public service.

The only solution I have to this issue is by using an ad-hoc proxy, or not use Deno at all.

@bartlomieju
Copy link
Member

@denizdogan Deno uses rustls for the TLS handling, you can check the "Non-features" section here: https://crates.io/crates/rustls which explains which ciphers are not supported.

@denizdogan
Copy link

@bartlomieju Yeah, I know... 😞 I understand the reasoning 100%, it's just so unfortunate. Explaining this to non-Deno-converts is difficult.

I'm left wondering if the door is 100% definitively closed on this whole thing, or if it would be possible to e.g. sneak in an extra --unstable feature to use a different TLS library on a per-Deno.HttpClient basis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir feat new feature (which has been agreed to/accepted)
Projects
None yet
Development

No branches or pull requests