SSL error with Safari but not Chrome

ezrichards commented 1 year ago

Hi there, I'm writing a node.js site using the following helmet security policy:

app.use(helmet.contentSecurityPolicy({
  directives: {
    defaultSrc: ["'self'"],
    scriptSrc: ["'self'", "'unsafe-inline'", "'unsafe-eval'", "https://cdn.jsdelivr.net"],
    styleSrc: ["'self'", "'unsafe-inline'", "https://fonts.googleapis.com", "https://cdn.jsdelivr.net"],
    imgSrc: ["'self'"],
  },
}));

This works flawlessly with Google Chrome, but when testing on Safari, I get the following error (no local stylesheets/assets working):

I have not setup any SSL certificates on the site and am simply working in a development environment. Further, if I just comment out all helmet code, Safari seems to recognize the local stylesheets correctly.

Is this something going on with Safari or have I configured helmet.js wrong?

EvanHahn commented 1 year ago

That's surprising. I wouldn't expect Helmet to be the problem here, but it's possible.

It looks like Safari is making requests to https://localhost:3000. Is Chrome doing the same?

ezrichards commented 1 year ago

That's surprising. I wouldn't expect Helmet to be the problem here, but it's possible.

It looks like Safari is making requests to https://localhost:3000. Is Chrome doing the same?

Yes, it looks like Chrome is doing the same.

EvanHahn commented 1 year ago

For some reason, it looks like Safari is incorrectly using HTTPS and Chrome is correctly using HTTP. That might explain those SSL errors in Safari.

I don't know why Helmet would be causing that to happen, though. Can you create a sample app that reproduces this issue?

ezrichards commented 1 year ago

Sure thing. Here's a watered down version of my project (same issue still happens): https://github.com/ezrichards/helmet-test-app

EvanHahn commented 1 year ago

Thanks for sending. I'll take a look.

EvanHahn commented 1 year ago

Short answer:

Safari is inconsistent with Chrome and Firefox when it comes to the Content Security Policy upgrade-insecure-requests directive. Chrome and Firefox don't think that localhost needs "upgrading" from HTTP to HTTPS, but Safari does.

To get around your problem while testing, you can set the upgradeInsecureRequests directive to null. For example:

app.use(helmet.contentSecurityPolicy({
  directives: {
    // ...
    // Disable Helmet's default. Don't forget to
    // remove this when you're done testing!
    upgradeInsecureRequests: null,
  },
}));

There are other ways to solve this problem, too, but that's a longer answer...

Long answer

As best I can tell, this is an inconsistency (or bug) with Safari. To understand this, you need a little background.

Background on upgrade-insecure-requests

Content Security Policy has a directive called upgrade-insecure-requests. Effectively, it tells browsers to use HTTPS even if they're written with http: in the HTML. Helmet sets this directive by default.

For example, if you had the following HTML on your page:

<img src="http://example.com/image.png" alt="test">

The upgrade-insecure-requests directive tells the browser to load https://example.com/image.png instead. (Notice the change in protocol from http: to https:.)

I didn't test this, but I assume it also happens if your page is loaded over insecure HTTP with relative URLs. For example, imagine the following HTML:

<img src="/image.png" alt="test">

If that HTML were at http://example.net/home.html, the directive would cause the browser to load https://example.com/image.png. (Again, I didn't test this, but I assume that happens.)

If you were already using HTTPS, nothing would be rewritten.

The localhost wrinkle

There's one tricky piece here, which is what you're running into: localhost.

I'm not certain, but there seems to be debate over whether localhost should be considered insecure and worthy of an upgrade to HTTPS. Firefox and Chrome don't think so, but it seems that Safari does.

To be extra-sure this wasn't a Helmet bug, I reproduced the problem in an app written in a different programming language.

Personally, I consider this a bug in Safari, but I'm not familiar with the details of the spec.

So how do you solve your problem today?

Solutions

I can think of several ways around this problem.

The simplest option: just don't bother testing in Safari.

Another simple option: temporarily disable upgrade-insecure-requests while you're testing in Safari. This is what I recommended above.

app.use(helmet.contentSecurityPolicy({
  directives: {
    // ...
    // Disable Helmet's default. Don't forget to
    // remove this when you're done testing!
    upgradeInsecureRequests: null,
  },
}));

Similarly, disable the directive in development. (Just make sure you're not accidentally deploying to production with NODE_ENV=development!)

const cspDirectives = {
 defaultSrc: [/* ... */],
 // ...
};
if (app.get("env") === "development") {
 // This prevents Safari from trying to load localhost over HTTPS.
 cspDirectives.upgradeInsecureRequests = null;
}

app.use(helmet.contentSecurityPolicy({
 directives: cspDirectives,
}));

EDIT (forgot to include this): Use HTTPS in development.
Disable the directive altogether. This has security implications and I wouldn't recommend it, but it would avoid the problem.
```
app.use(helmet.contentSecurityPolicy({
 directives: {
   upgradeInsecureRequests: null,
 },
}));
```

There are some other options, like removing Helmet altogether or joining the Safari team just to fix this bug, but I suspect those are overkill.

An aside about Helmet

As an aside, I saw this in your sample code:

app.use(helmet());
app.use(helmet.contentSecurityPolicy({
  // ...
}));

This will work, but is incorrect. (A lot of people make this mistake, which means it's probably my fault for designing a confusing API.) The first call to helmet() will set the Content-Security-Policy header and the second will clobber it, which means the first is a waste.

If you're doing the same thing in your real app, I recommend doing something like this:

app.use(helmet({
  contentSecurityPolicy: {
    directives: {
      // ...
    },
  },
}));

Again, what you have will work, but there's a slightly better way.

I'm going to close this issue because I think I've determined this isn't an issue with Helmet, but I'll think about changing this in the future if other people report it.

ezrichards commented 1 year ago

Thanks for this in-depth analysis, I really appreciate it! I actually had not tested on a domain other than localhost, so this is my bad. I assume, like you say, with a proper certificate and HTTPS connection to the domain, this goes away. Thank you again for offering solutions nonetheless. :)

EvanHahn commented 1 year ago

Ah yes. I forgot to mention that you could use HTTPS in development which would also address the issue. I've updated my comment.

Please don't hesitate to reach out about anything else!

helmetjs / helmet