Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix certbot staging certificate generation #236

Merged
merged 2 commits into from Sep 22, 2021

Conversation

frafra
Copy link
Collaborator

@frafra frafra commented Sep 16, 2021

Fix #203.

@lpasquali
Copy link
Contributor

@frafra, many thanks for this PR

I think that a better foreseeing fix might be try to remove --staging in staging and the whole --server option for both prod and staging and use --dry-run option in certbot for staging certificates. This way we will not depend anymore on ACME host names changes (see https://letsencrypt.org/docs/staging-environment/)

@frafra
Copy link
Collaborator Author

frafra commented Sep 17, 2021

@lpasquali Ok! I would then suggest renaming "STAGING" to "DRYRUN" to avoid confusion (as different certificates will be served over HTTPS).

@lpasquali
Copy link
Contributor

@lpasquali Ok! I would then suggest renaming "STAGING" to "DRYRUN" to avoid confusion (as different certificates will be served over HTTPS).

well it is still staging stating at the current doc I pasted, the option --dry-run uses staging ACME environment, we'd just need to update to a simpler configuration the certbot commands and since --dryrun does stating certs, I'm unsure this would be a real benefit in avoiding confusion

@frafra
Copy link
Collaborator Author

frafra commented Sep 17, 2021

@lpasquali Ok! I would then suggest renaming "STAGING" to "DRYRUN" to avoid confusion (as different certificates will be served over HTTPS).

well it is still staging stating at the current doc I pasted, the option --dry-run uses staging ACME environment, we'd just need to update to a simpler configuration the certbot commands and since --dryrun does stating certs, I'm unsure this would be a real benefit in avoiding confusion

--dry-run Perform a test run of the client, obtaining test
(invalid) certificates but not saving them to disk.

-- https://certbot.eff.org/docs/using.html?highlight=--dry-run#certbot-command-line-options

It is my understanding that a GeoNode instance with set to "STAGING" will not get SSL certificates at all then, so it would rely on local self-signed certificates if available. That would break many browsers, probably.

@lpasquali
Copy link
Contributor

@lpasquali Ok! I would then suggest renaming "STAGING" to "DRYRUN" to avoid confusion (as different certificates will be served over HTTPS).

well it is still staging stating at the current doc I pasted, the option --dry-run uses staging ACME environment, we'd just need to update to a simpler configuration the certbot commands and since --dryrun does stating certs, I'm unsure this would be a real benefit in avoiding confusion

--dry-run Perform a test run of the client, obtaining test
(invalid) certificates but not saving them to disk.

-- https://certbot.eff.org/docs/using.html?highlight=--dry-run#certbot-command-line-options

It is my understanding that a GeoNode instance with set to "STAGING" will not get SSL certificates at all then, so it would rely on local self-signed certificates if available. That would break many browsers, probably.

@frafra I beg your pardon, I wanted to mean "--test-cert" (--staging is left for backward compatibility) , "--dry-run" is not to be used in our case.

@frafra
Copy link
Collaborator Author

frafra commented Sep 21, 2021

@frafra I beg your pardon, I wanted to mean "--test-cert" (--staging is left for backward compatibility) , "--dry-run" is not to be used in our case.

Ok! Great then :)
I updated my branch.

@afabiani
Copy link
Member

@lpasquali if it's ok with you we can merge then

@lpasquali
Copy link
Contributor

@lpasquali if it's ok with you we can merge then

changes approved @afabiani

@frafra frafra merged commit 64d403c into GeoNode:master Sep 22, 2021
@frafra frafra deleted the fix-certbot-letsencrypt-staging branch September 22, 2021 07:50
afabiani pushed a commit that referenced this pull request Sep 22, 2021
Use --test-cert instead of --staging and --server

(cherry picked from commit 64d403c)
afabiani pushed a commit that referenced this pull request Sep 22, 2021
Use --test-cert instead of --staging and --server

(cherry picked from commit 64d403c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

certbot fails when using LETSENCRYPT_MODE=staging
3 participants