forwardemail / forwardemail/superagent

Allowing insecure HTTPS requests in limited cases

Open
#1,338 4 comments 1 reaction 0 assignees View on GitHub
Dominant language
JavaScript
Stars
16.6k
Forks
1.3k
PR merge metrics
No merged PRs in 30d

Description

There have been many requests to allow disabling HTTPS certificate checks. I pushed back on it, because strict checks are absolutely essential to HTTPS security (without them active MITM attackers can bypass all encryption, eavesdrop and modify traffic). I'm convinced that this such feature that could be enabled easily and globally is more dangerous than it sounds, and I wanted to provide a proper solution to this.

I think a proper solution would be a form of certificate pinning. Instead of disabling all security everywhere, you could say "Trust this particular certificate, even if it's mismatched or self-signed". However, I don't have time to implement it properly (and an implementation mistake here could be even worse than a global "fuck all security" setting, because it would give false impression of security), so I'm looking into simpler alternatives that still have some safety limits.

Here are some ideas (specific method names may change):

1. `.alwaysTrustLocalhost()` method, which disables HTTPS security on `https://localhost`, `https://127.0.0.1`, `https://[::1]`. This is expected to be safe, because loopback network is as safe as any of the code running on the machine.

2. `.ip("127.0.0.1")` function that overrides connection IP address regardless of hostname in the URL (e.g. `superagent.get("https://api.example.com").ip("127.0.0.1")` would make localhost request with `Host: api.example.com` header), which makes `alwaysTrustLocalhost()` work for it too.

I think these two would be helpful in development on a single machine, and perhaps some microservices on the same machine.

Question: should there be a way to trust a LAN as well?

3. `superagent.disableAllSecurityInDevelopmentForDomains(["dev.example.com", "internal.example.com"])`. A global switch to make HTTPS useless, but only for given domains, and only if `NODE_ENV=development`. This way:
* Less likely to create a vulnerability if it's accidentally forgotten and deployed to production.
* In the worst case it will only compromise data of the listed domains (hopefully dev/test ones), and not others (e.g. if your server also makes requests to Google's APIs, it won't expose Google's credentials even if you chose to disable HTTPS security for your internal APIs).

4. A method that disables HTTPS checks for all requests, including in production. I'd like to make it intentionally long and scary, so that it won't go unnoticed in code reviews of a short "fix" that made HTTPS errors go away:

```js
superagent.disableAllHTTPSecurity({
allowAttackersToRemoveEncryption: true,
allowMaliciousModificationsOfTraffic: true,
})
```

Contributor guide

Open the contributing guide

Research direction

The issue names no implementation files, tests, or entry points beyond proposed methods such as alwaysTrustLocalhost(), ip(), and disableAllSecurityInDevelopmentForDomains(). Start by resolving which HTTPS-trust behavior is intended and reviewing the existing request and TLS handling. Done requires an agreed security model, implementation scope, and tests, none of which are specified here.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
security
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.