gmpassos / shelf_letsencrypt

Let's Encrypt support for the shelf package (free and automatic HTTPS certificate support).
Apache License 2.0
8 stars 3 forks source link

Changed to passing a Domain object, hardened the lints and update the readme.md #6

Closed bsutton closed 8 months ago

bsutton commented 10 months ago

This is what I've done. See what you think.

codecov[bot] commented 10 months ago

Codecov Report

Attention: 196 lines in your changes are missing coverage. Please review.

Comparison is base (39d0dc2) 35.19% compared to head (60b43ed) 35.00%.

Files Patch % Lines
lib/src/letsencrypt.dart 22.50% 93 Missing :warning:
lib/src/certificates_handler_io.dart 63.55% 43 Missing :warning:
lib/src/certs_handler.dart 19.35% 25 Missing :warning:
lib/src/logging.dart 57.14% 9 Missing :warning:
lib/src/domain_certificate_file_path.dart 0.00% 8 Missing :warning:
lib/src/security_context_builder.dart 0.00% 7 Missing :warning:
lib/src/domain.dart 33.33% 6 Missing :warning:
lib/src/check_certificate_status.dart 0.00% 5 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #6 +/- ## ========================================== - Coverage 35.19% 35.00% -0.20% ========================================== Files 2 9 +7 Lines 429 440 +11 ========================================== + Hits 151 154 +3 - Misses 278 286 +8 ``` | [Flag](https://app.codecov.io/gh/gmpassos/shelf_letsencrypt/pull/6/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Graciliano+Monteiro+Passos) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/gmpassos/shelf_letsencrypt/pull/6/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Graciliano+Monteiro+Passos) | `35.00% <39.50%> (-0.20%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Graciliano+Monteiro+Passos#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

gmpassos commented 10 months ago

Let's upgrade to:

lints: ^3.0.0

bsutton commented 10 months ago

As requested I've move to lints 3.x

gmpassos commented 10 months ago

Let's bump the version to 1.3.0

bsutton commented 10 months ago

I now have a secure server up and running using the new code :)

bsutton commented 10 months ago

I think this is now ready to be merged unless you have any specific issues.

The key point is the breaking change to startServer

gmpassos commented 10 months ago

Please, check if any dependency need to be updated:

acme_client: ^1.3.0

gmpassos commented 8 months ago

Hi, I will check this PR this week. Do you have any update to it?

bsutton commented 8 months ago

No I'm done.

On Tue, 23 Jan 2024, 9:01 am Graciliano Monteiro Passos, < @.***> wrote:

Hi, I will check this PR this week. Do you have any update to it?

— Reply to this email directly, view it on GitHub https://github.com/gmpassos/shelf_letsencrypt/pull/6#issuecomment-1904900004, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG32OEHWY5CYLFMX7QJZNDYP3OT7AVCNFSM6AAAAABAMNOSE2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBUHEYDAMBQGQ . You are receiving this because you authored the thread.Message ID: @.***>

bsutton commented 8 months ago

FWIW: I use the dcli package to do this (I'm the author).

import 'package:dcli/dcli.dart';

void main() {
final shell = Shell.current;

if (!shell.isPriviliegedProcess()) {
print('you must run as root)';
exit(1);
}
/// stop executing as sudo
shell.releasePrivileges();

shell.withPrivilige(() {
// execute code that requires root
});

On Wed, Jan 24, 2024 at 8:37 AM Graciliano Monteiro Passos < @.***> wrote:

@.**** commented on this pull request.

In README.md https://github.com/gmpassos/shelf_letsencrypt/pull/6#discussion_r1464019814 :

+certificate. + + +You do this by passing in 'production: false' (the default) when creating +the LetsEncrypt certificate. +Staging certificates still have rate limits but they are much more generours + +dart +final LetsEncrypt letsEncrypt = LetsEncrypt(certificatesHandler, production: false); + + + +## Permissions +On Linux you need to be root (sudo) to open a port below 1024. If you try +to start your server with the default ports (80, 443) you will fail. +

Also add a documentation for setcap to avoid execution as root:

Enabling the dart VM:

sudo setcap 'cap_net_bind_service=+ep' /usr/lib/dart/bin/dart

Enabling a self-executable (from dart compile exe your_tool.dart):

sudo setcap 'cap_net_bind_service=+ep' your_tool.exe

— Reply to this email directly, view it on GitHub https://github.com/gmpassos/shelf_letsencrypt/pull/6#pullrequestreview-1839974229, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG32ODLOFBPMV27STFJCJLYQAUSHAVCNFSM6AAAAABAMNOSE2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQMZZHE3TIMRSHE . You are receiving this because you authored the thread.Message ID: @.***>

gmpassos commented 8 months ago

Nice package!

I usually avoid running server processes as root due to security risks. I recommend using setcap on the binary to enable the server to listen on low ports. I don't think we should advise executing server processes as root.

bsutton commented 8 months ago

Fair enough.

On Thu, 25 Jan 2024, 6:58 am Graciliano Monteiro Passos, < @.***> wrote:

Nice package!

I usually avoid running server processes as root due to security risks. I recommend using setcap on the binary to enable the server to listen on low ports. I don't think we should advise executing server processes as root.

— Reply to this email directly, view it on GitHub https://github.com/gmpassos/shelf_letsencrypt/pull/6#issuecomment-1908824341, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAG32OB4762HTIUVYX2Y62TYQFRXTAVCNFSM6AAAAABAMNOSE2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBYHAZDIMZUGE . You are receiving this because you authored the thread.Message ID: @.***>

gmpassos commented 8 months ago

FYI:

Preparing release and testing it in real projects: https://github.com/gmpassos/shelf_letsencrypt/pull/12