Read file for SSL (certs and keys) and add pfx support

[JBusillo]

Fixes #1659.

This replaces PR #1662 due to my inexperience with Github.

The dev server passes the vite options (config.kit.vite) when creating the http(s) server. This is functioning correctly.
The preview server passes the kit options (config.kit) when creating the http(s) server. This is not functioning correctly.

In preview mode, when the user specifies a cert/key pair, the cert/key pair is ignored, since the logic is looking for the option in the wrong place. Instead, Kit will generate its own cert/key pair. You can observe this by examining the certificate details within the browser.

This change aligns where the dev and preview modes look for a user-specified cert/key pair.

For reference, here is an example of a user-specified cert/key pair in svelte.config.js

/** @type {import('@sveltejs/kit').Config} */
import fs from "fs";
const config = {
  kit: {
    // hydrate the <div id="svelte"> element in src/app.html
    target: "#svelte",
    vite: {
      server: {
        https: {
          key: fs.readFileSync("example.key"),
          cert: fs.readFileSync("example.crt"),
        },
      },
    },
  },
};
export default config;

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts

[changeset-bot]

🦋 Changeset detected

Latest commit: b667f81

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[JBusillo]

To assist in the review, the following document describes the files modified, changes made, strategy, etc.

Also, additional comments concerning this PR are contained in the original PR, #1662

Change Summary

SvelteKit PR 2027

dev/index.js

• renamed user_config to vite_config
• added type checking for vite_config
• added a @ts-ignore for vite.ssr -- Hopefully Vite will add SSR typedefs in the near future, which will resolve this.

server/index.js

The goal of the get_server and underling functions is to:

• If the -H (or --https) option is specified as a command line parameter and a cert/key pair (or pfx) is not specified, a self-signed certificate is created.
• If the -H (or --https) option is specified as a command line parameter and a cert/key pair (or pfx) is specified, use the specified cert/key pair or pfx.
• Create the http(s) server, honoring SvelteKit CLI options and Vite options.

Note: Specifying custom self-signed certificates never worked in preview mode. Now it does.

The Vite option vite.server.https allows any child option that is defined in node:https.ServerOptions. 'ca' and 'pfx' are options that are processed similarly to the 'cert' and 'key' options. 'ca' overrides the trusted CA certificates, and 'pfx' is a binary certificate format commonly used in Windows (which requires an addition 'passphrase' option). Correctly handling these two options might prevent future issues/enhancements to be raised.

Vite allows the key, cert, ca, and pfx options to be specified as a filename, string, or buffer[]. For example, the "cert" can be specified as any of the following:

    cert: "./my-certificate.crt"   // filename          
    cert: fs.readFileSync("my-certificate.crt")   // returns a buffer[]
    cert: fs.readFileSync("my-certificate.crt", {encoding: 'utf-8'} )   // returns a string
    cert: "-----BEGIN CERTIFICATE-----\nMIIGEzCCA/ugAwIB ... -----END CERTIFICATE-----"  // string

I copied/adapted Vite's code in function readFileIfExists (see below).

function get_server(...)

user_config was renamed to vite_config. Type checking was added.

If the -H cli flag is specified

• load http_options from vite.server.https
• resolve the ca/cert/key/pfx options -- i.e., if a filename was specified, set the option value to the file's content as a buffer[]. Otherwise, just return the passed value, which can be a string or a buffer[].
• if the 'pfx' option was specified, ignore the cert and key options. Pfx certificates already contain both the key and cert.
• if pfx/key/cert options aren't present, create a self-signed certificate.

If the -H cli flag is NOT specified, and if vite.server.https options are specified, Vite's HMR server gets confused, causing constant reloads from the client. To resolve this, I just delete the vite.server.https leaf.

function readFileIfExists(...)

This is Vite's code --- It's kludgy, but it works:

If a string is passed as a ca/cert/key/pfx option, it can either be a filename, or a string that holds the certificate contents. A readFileSync is issued, and if successful, the resulting buffer[] is returned. If it fails, the string is assumed to be the actual certificate contents, and is returned as-is.

If a non-string is passed, it's assumed that it's a buffer[] containing the actual certificate contents, and is returned as-is.

Yes, the @param type specification has an any[]. The types for 'ca' and 'pfx' have different signatures, and want to include the crypto module, among other things. The native Vite code has it specified as this.

preview/index.js

• pass the vite configuration into get_server. It was (incorrectly) passing kit configuration. This is the 'crux' of the original issue Expose config.kit.server.https so that svelte-kit preview --https can use user-defined cert #1659.

types/config.d.ts

• re-export the imported ViteConfig type to support type checking in the above modules.

Testing

My testing strategy included:

HTTPS testing: All three variations of HTTPS options for cert/key/pfx -- filename, string and buffer[]. For "ca", I ensured (via debugger) that all three variations were formatted correctly when passed to https.createServer.

HTTP testing: With and without vite.server.https options (ensures that HMR doesn't reload constantly)

Verify the above in both dev and preview modes. For dev mode, ensure that HMR continues to work properly. Also ensure that the proper certificate is used in all test cases (i.e., Kit's generated certificate or the user-specified certificate).

Cursory testing to ensure that --host and --port options are honored.

And, of course, pnpm format, pnpm lint, pnpm build and pnpm test.

[benmccann]

Thanks! There's kind of three different things being done here:

• Making sure the options get passed in preview
• Reading the file
• Adding support for pfx

To help separate the discussion, I cleaned up and committed the first part for you in #2036

Regarding the second part, Rich had suggested maybe it'd be better to leave it the way it is? #1798 (comment)

I haven't looked at the pfx part yet, but we could do just a smaller PR for that

[JBusillo]

If I understand correctly, you opened PR #2036 and only included code for the original issue -- preview using Kit options instead of (correctly) using Vite options. These changes have been merged into master.

As far as 'reading the file: In non middleware mode, Vite allows the cert and key (as well as pfx and ca) to be specified using the filename, a string or a buffer. I wanted to duplicate the functionality that Vite had so that middleware mode could use this same flexible (but somewhat ambiguous) specification. However, going forward, it seems that Rich wants the cert and key values to be strings (such as the string returned by fs.readFileSync(). That's OK by me.

For the 'pfx' option: If the key/cert options are not specified, Kit will populate the key/cert options with a self-signed certificate. I'm not sure what node will do, e.g., throw an error or let one option take precedence over the other. I'll test this out later today. If the 'pfx' option takes precedence, no changes may be needed. Otherwise I'll open an issue for that.

So, is this PR 'dead'? Should I close it? Thanks.

[benmccann]

If I understand correctly, you opened PR #2036 and only included code for the original issue -- preview using Kit options instead of (correctly) using Vite options. These changes have been merged into master.

Yes

As far as 'reading the file: In non middleware mode, Vite allows the cert and key (as well as pfx and ca) to be specified using the filename, a string or a buffer.

How does Vite differentiate between a filename and a string? I wonder if we should try to steal their logic for this. As far as I can see they only take a filename, but I could be missing something: https://github.com/vitejs/vite/blob/514e124b29597203cb90aac9d838cd9322f24451/packages/vite/src/node/server/http.ts#L43

So, is this PR 'dead'? Should I close it?

I think we got the most important part in, but I'm happy to discuss if you want to pursue the other parts of this such as pfx support

[JBusillo]

How does Vite differentiate between a filename and a string? I wonder if we should try to steal their logic for this. As far as I can see they only take a filename, but I could be missing something: https://github.com/vitejs/vite/blob/514e124b29597203cb90aac9d838cd9322f24451/packages/vite/src/node/server/http.ts#L43

Vite's differentiation logic is contained within this function:

If the option ('value' parameter) is not a string, it assumes that it's a buffer. It returns "value" as is.
If the option is a string:

• Vite tries to treat it as a filename, and reads it via fs.readFileSync(). If it's successful, the value returned is the file contents as a buffer[], since there's no {encoding: 'utf-8') specified.
• If the read was not successful, it assumes that the string value specifies the actual contents of the cert/key/ca/pfx. It returns the string as is.

The Node options cert, key, ca and pfx can be passed as a string or a buffer[] -- Create Secure Context Options

Using Vite's strategy might be beneficial. The user need not specify the {encoding: 'utf-8') option for fs.readFileSync, or can eliminate the fs.readFileSync call by merely specifying the filename.

So, is this PR 'dead'? Should I close it?

I believe that you prefer implementing PR's that resolve current open issues -- which is good for tracking/documentation/search purposes. Depending on what you'd like me to implement, I'd be happy to open the related issue(s) and open a separate PR to implement them. For me, it would be safest to open a new PR, avoiding the debacle of a couple days ago :)

BTW, I tested what happens when only the pfx option is specified -- Kit creates a self-signed certificate, and Node uses this certificate and ignores the pfx certificate. Therefore, I think we can classify this as a issue (bug).

[benmccann]

Using Vite's strategy sounds like a good plan to me

No need to file an issue. We've already discussed it here. You can just go ahead and create a PR if you'd like

[benmccann]

Btw, I see there's a PR pending to fix pfx support in Vite now: vitejs/vite#4496

[benmccann]

For me, it would be safest to open a new PR, avoiding the debacle of a couple days ago

Since you said you'd prefer opening a new PR to updating this one, I'll go ahead and close it. If you change your mind about that we can go ahead and reopen this one

[JBusillo]

Thanks for closing this. I've been a bit busy with personal items, but I'll open up the new PR and make the changes soon.
Also, you had mentioned the Vite PR for pfx support -- You might already be aware that the PR doesn't affect us, as SvelteKit is using 'middlewares' mode, and is responsible for setting up its own HTTP/S server.