sasa1977 / site_encrypt

Integrated certification via Let's encrypt for Elixir-powered sites
MIT License
462 stars 33 forks source link

Re-use existing certificate on application restart #27

Closed lawik closed 3 years ago

lawik commented 3 years ago

Hey

I've been trying site_encrypt out with my little wisps project on my site. Loving it overall. But encountered something that I'm unclear if it is working as intended or a misconfig on my part.

When restarting the application it doesn't pick up that I already had a valid cert from before the restart and thus I get a short period of snakeoil certification which seems like it could be avoided.

Running as an Elixir release. Nothing particularly fancy.

sasa1977 commented 3 years ago

Hey,

This definitely shouldn't happen. If the certificate was successfully obtained, it will be stored to disk, and it should be used on the next boot. Could you first try if this works with the included demo app (see readme instructions)?

lawik commented 3 years ago

I'll try to find a moment to run through that. Glad to know this is not intended.

lawik commented 3 years ago

Where are they stored and can it be configured? Before I dig in, maybe they end up overwritten as part of overwriting the previous release?

sasa1977 commented 3 years ago

It's determined by the db_folder setting. If you used the suggested config from the readme, then it's priv, so it could indeed happen that the next release overwrites it.

lawik commented 3 years ago

Yeah, that was it. Something to keep in mind when deploying :)

sasa1977 commented 3 years ago

I'll see about making it clearer in the docs.

sasa1977 commented 3 years ago

But anyway, the idea is to make this folder gitignored, and make sure that deploy doesn't overwrite it in prod. For example, I run the Erlangelist blog in the docker container, and this folder is mounted from the host, so it is preserved on subsequent deploys.

lawik commented 3 years ago

It makes sense. I figured it should be doing something like this but wasn't clear on what was going on. I just build a release on the final server environment for this particular service and it doesn't have anything else that needs to be kept. So it would just overwrite the old release with the new. Configured it to persist to a specific dir outside the release and everything works just the way I want.

ericmj commented 3 years ago

IIRC priv/ should be used for data included in the release and should not be written to by the application after the release is created. Maybe a required configuration for the certification location would be better?

sasa1977 commented 3 years ago

The location configuration is required, the problem is that project doc (e.g. readme & moduledoc) recommends priv. I agree though that this is not the best choice for storage. I've used it because it makes things simpler in local dev&test, where you probably want something under the project folder (though not necessarily under priv), while on staging/prod machines you probably want an absolute path.

Perhaps docs could recommend something like System.get_env("SITE_ENCRYPT", "site_encrypt")? This would still lead to the original problem in this issue, but it wouldn't incorrectly promote priv for storage, and making it work in production would boil down to choosing some absolute path outside of the release folder. WDYT?

ericmj commented 3 years ago

Perhaps docs could recommend something like System.get_env("SITE_ENCRYPT", "site_encrypt")? This would still lead to the original problem in this issue, but it wouldn't incorrectly promote priv for storage, and making it work in production would boil down to choosing some absolute path outside of the release folder. WDYT?

I think that's a great solution.

lawik commented 3 years ago

Sounds good to me, or || it to blow up with an error if not configured.

sasa1977 commented 3 years ago

I ultimately went for System.get_env("SITE_ENCRYPT_DB", Path.join("tmp", "site_encrypt_db")), because tmp is auto gitignored in newer Elixirs. In case you're interested, the relevant commit is 1dad766a71617f7f0af42bd006f117de10a40998 (not merged yet).

lawik commented 3 years ago

Sounds good