littlebizzy / slickstack

Lightning-fast WordPress on Nginx
https://slickstack.io
GNU General Public License v3.0
641 stars 112 forks source link

Ensure more privacy for openssl cert generation. #195

Closed NathanAdhitya closed 8 months ago

NathanAdhitya commented 1 year ago

Problem

OpenSSL cert generation includes domains and public IP of the server This presents a security risk of exposing the server's public IP (which is the entire point of using Cloudflare in the first place) if a misconfiguration happens. Such things are searchable in services like Censys and Shodan.

Suggestion

jessuppi commented 1 year ago

As always thanks for your constructive criticism @NathanAdhitya

It definitely seems like a good idea to conditionally sign the IPs only if the sysadmin wants that, which is probably a rare case, so SlickStack should probably default to not doing that. It would probably also be fairly simple to add support for other names like localhost and such too.

About the Issuer do you mean support for third party CA's like ZeroSSL etc?

jessuppi commented 1 year ago

Perhaps some default ss-config options like this:

OPENSSL_CERT_DOMAINS="true"
OPENSSL_CERT_IPS="false"
OPENSSL_CERT_LOCALHOST="false"
NathanAdhitya commented 1 year ago

I think a more appropriate / better variable naming for readability's sake would be:

OPENSSL_CERT_INCLUDE_DOMAINS="true"
OPENSSL_CERT_INCLUDE_IPS="false"
OPENSSL_CERT_INCLUDE_LOCALHOST="false"

Also, consider checking my message in Discord. Thanks!

jessuppi commented 1 year ago

Ref: https://github.com/littlebizzy/slickstack/commit/e98cb31562878d1eb7a18d3455cc85072f840b2c

jessuppi commented 9 months ago

Update:

It's been almost a year since we changed to use -subj "/CN=localhost" and there's been no issues or complaints. I think at this point we can move ahead with removing IP addresses from the OpenSSL certs.

And probably, something like this:

"subjectAltName=DNS:${SITE_DOMAIN_EXCLUDING_WWW},DNS:${SITE_DOMAIN_INCLUDING_WWW},DNS:staging.${SITE_DOMAIN_EXCLUDING_WWW},DNS:dev.${SITE_DOMAIN_EXCLUDING_WWW}"

...depending on whether staging/dev are enabled in ss-config. I don't think there's any privacy concerns with always including the domains that are activated on a given SlickStack server, unless in some bizarre case where the cert is being used for a new domain already, and the old domains still exist in the cert or something, but that's virtually impossible to achieve with SlickStack anyways and I think stripping out the domains would be extreme and/or pointless.

jessuppi commented 9 months ago

Ref: https://github.com/littlebizzy/slickstack/blob/master/bash/ss-encrypt-openssl.txt

jessuppi commented 9 months ago

Per Discord chat:

https://stackoverflow.com/questions/11539121/how-can-i-tell-nginx-to-silently-ignore-requests-that-dont-match-and-let-them-ti

jessuppi commented 8 months ago

Ref: https://github.com/littlebizzy/slickstack/commit/c4d936e412f9b3683ca57b7b40195b6d20a83b27

jessuppi commented 8 months ago

The big one...

Ref: https://github.com/littlebizzy/slickstack/commit/40cd39c00baf2b2d42f79c407502b3754b0526c0

jessuppi commented 8 months ago

This one is for localhost support...

Ref: https://github.com/littlebizzy/slickstack/commit/8a7f63de573177f19c35e8c5a9ef3468e31af8ba

...Note that, maybe we should also add support for arbitrary IP addresses?

Ref: https://stackoverflow.com/questions/60030906/self-signed-certificate-only-works-with-localhost-not-127-0-0-1

jessuppi commented 8 months ago

Here's the supported options now in ss-config with their default values:

OPENSSL_CERT_INCLUDE_DOMAINS="true"
OPENSSL_CERT_INCLUDE_IPS="false"
OPENSSL_CERT_INCLUDE_LOCALHOST="false"

I think this covers nearly all scenarios while defaulting to safe default values... as far as my last post about arbitrary IP addresses, that seems to be localhost related, so perhaps our options would be enough already. And if it's not enough, it's a very niche case and probably not important enough to worry about for now. Closing this, thanks again.