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

DietPi-Banner | add letsencrypt certificate details #6314

Merged
merged 10 commits into from
Apr 21, 2023

Conversation

gary2002
Copy link
Contributor

Addition to check if a letsencrypt certificate exists and if so, print the certificate expiration details.

This can act as a simple check to see if letsencrypt certificates are updating over time.

@Joulinar Joulinar mentioned this pull request Apr 15, 2023
@Joulinar Joulinar requested review from MichaIng and Joulinar April 15, 2023 08:09
@Joulinar Joulinar added this to the v8.17 milestone Apr 15, 2023
@Joulinar Joulinar changed the title banner with letsencrypt certificate details DietPi-Banner | add letsencrypt certificate details Apr 15, 2023
@gary2002
Copy link
Contributor Author

Many thanks. I finally got over the line ;-)

But I have learnt some useful GitHub tips and techniques.

Many thanks for your patience!!!

dietpi/func/dietpi-banner Outdated Show resolved Hide resolved
dietpi/func/dietpi-banner Outdated Show resolved Hide resolved
dietpi/func/dietpi-banner Outdated Show resolved Hide resolved
dietpi/func/dietpi-banner Outdated Show resolved Hide resolved
gary2002 and others added 5 commits April 20, 2023 19:14
Addition to check if a letsencrypt certificate exists and if so, print the certificate expiration details.

This can act as a simple check to see if letsencrypt certificates are updating over time.
Happy with that.

Co-authored-by: MichaIng <[email protected]>
DietPi-Banner | add letsencrypt certificate details, do not enable function by default

Co-authored-by: MichaIng <[email protected]>
DietPi-Banner | add letsencrypt certificate details, change menu name

Co-authored-by: MichaIng <[email protected]>
- CHANGELOG | DietPi-Banner: Added a new option to show the Let's Encrypt certificate status (expiry date), when installed via dietpi-letsencrypt or Certbot. Many thanks to @gary2002 for implementing this option.
- DietPi-Banner | Simplify awk for obtaining the certificate expiry date
@MichaIng
Copy link
Owner

Looks good to me now. Only thing is that I'm not too happy with the date format:

Valid until 10-Jul-2023 @ 23:25:20

I'd prefer the common standard international format

Valid until 2023-07-10 23:25:20

But there is no way to change this in openssl, is it? At least I couldn't find an option. So closest we could do:

Valid until 2023-Jul-10 23:25:20

What do you think?

- DietPi-Banner | Do not obtain IP info if it is not printed anyway
- DietPi-Banner | Let's Encrypt cert status: Print in format "YYYY-MMM-DD HH:MM:SS"
MichaIng
MichaIng previously approved these changes Apr 21, 2023
- CHANGELOG | DietPi-Banner: Add link to PR
@MichaIng
Copy link
Owner

We talked on our meeting yesterday about the time format and agreed to prefer 2023-Jul-10 23:25:20 as being closest to the otherwise common international standard 2023-07-10 23:25:20. I adjusted the PR accordingly and added a little other minor coding updates, I hope you agree. Merging this.

@MichaIng MichaIng merged commit 5f327ef into MichaIng:dev Apr 21, 2023
@gary2002
Copy link
Contributor Author

Very happy with that. Apologies for not responding sooner - busy day at work.

@MichaIng
Copy link
Owner

Jep no problem, I apologies for being a little impatient 😄. Want to get the open PRs all merged this weekend (aside of the two older banner related ones, which require significantly more work).

@gary2002
Copy link
Contributor Author

I always use MMM in the date format to avoid ambiguity between days and months. I've found it necessary in projects I've worked on across Australia, the USA, France, etc. so everyone clearly knows delivery and milestone dates. More than happy with the solution that is being implemented. Thanks again for your support. It's helped me learn a lot.

@MichaIng
Copy link
Owner

I think with the year first it is pretty clear that the next number is the month and last the day. But with year last I agree that one can be nether sure whether the first or the second number is the month, so MMM is probably nicer then.

@Joulinar
Copy link
Collaborator

guys, did we ever tested this as non-root user? It seems it's working for root only.

https://dietpi.com/forum/t/dietpi-banner-no-certificate-found/16826

@MichaIng
Copy link
Owner

We didn't 🙈. The certificate, respectively the whole directory is probably readable for the root user only, I guess 🤔. Would make somehow sense as this is quite sensible data.

If this expectation is true, there is nothing we can or should do about it. Then it would be easiest to just skip the banner entry for non-root users and print a related info when it is enabled.

@gary2002
Copy link
Contributor Author

gary2002 commented May 11, 2023 via email

@MichaIng
Copy link
Owner

Since I use acme.sh, can you two verify that in your case /etc/letsencrypt/live/your.domain and/or /etc/letsencrypt/archive/your.domain (the first contains symlinks to the second) are not world-readable? I mean actually only the private key is sensitive while the certificate is public anyway, but probably just as an additional measure, the whole directory is non-executable other non-root users, so they cannot even see the content.

@Joulinar
Copy link
Collaborator

ping @JappeHallunken as I'm using NPM for proxy and certificates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants