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

Self-signed certificate generation #532

Merged
merged 3 commits into from
May 27, 2022

Conversation

FileGo
Copy link
Collaborator

@FileGo FileGo commented May 23, 2022

This PR adds generation of a self-signed certficate, as proposed in #343.

It changes the functionality quite a lot - server start will no longer fail if HTTPS or TLS port is defined, but certificate/key files aren't. Blocky will instead create its own CA, which will in turn sign the newly created certificate.

As using self-signed certificates is insecure by the nature of it (compared to using a trusted CA), I didn't see the point of adding an option for configuring the DNS name for the certificate, as any software using blocky's DoT/DoH features in this configuration would have to use something similar to InsecureSkipVerify anyway.

I'd appreciate any feedback on this.

@codecov
Copy link

codecov bot commented May 23, 2022

Codecov Report

Merging #532 (1e794c0) into development (a8ab753) will decrease coverage by 0.51%.
The diff coverage is 70.12%.

❗ Current head 1e794c0 differs from pull request most recent head e2a2b45. Consider uploading reports for the commit e2a2b45 to get more accurate results

@@               Coverage Diff               @@
##           development     #532      +/-   ##
===============================================
- Coverage        93.73%   93.21%   -0.52%     
===============================================
  Files               36       36              
  Lines             3112     3171      +59     
===============================================
+ Hits              2917     2956      +39     
- Misses             145      154       +9     
- Partials            50       61      +11     
Impacted Files Coverage Δ
server/server.go 82.69% <69.33%> (-3.13%) ⬇️
config/config.go 98.07% <100.00%> (-0.07%) ⬇️
cmd/serve.go 60.41% <0.00%> (-4.17%) ⬇️
redis/redis.go 67.76% <0.00%> (+1.31%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a8ab753...e2a2b45. Read the comment docs.

@0xERR0R 0xERR0R added the 🔨 enhancement New feature or request label May 27, 2022
@0xERR0R
Copy link
Owner

0xERR0R commented May 27, 2022

Thanks for your work!

As using self-signed certificates is insecure by the nature of it (compared to using a trusted CA), I didn't see the point of adding an option for configuring the DNS name for the certificate, as any software using blocky's DoT/DoH features in this configuration would have to use something similar to InsecureSkipVerify anyway.

Absolutely agree, this should be used only for testing purposes or in trusted local networks. I also think, it is ok, that the certificate will be regenerated on each start -> it doesn't make sense to import this generated cert in any trust store. The only pain point is the start time, on my laptop it needs about 8 sec to generate the certificate, I think on a raspberry pi 1/2 it would take a noticeably amount of time, but in this case it is always possible to generate the certificate externally.

@0xERR0R 0xERR0R merged commit 92fd623 into 0xERR0R:development May 27, 2022
@0xERR0R 0xERR0R added this to the 0.19 milestone May 27, 2022
@FileGo
Copy link
Collaborator Author

FileGo commented May 28, 2022

The only pain point is the start time, on my laptop it needs about 8 sec to generate the certificate, I think on a raspberry pi 1/2 it would take a noticeably amount of time, but in this case it is always possible to generate the certificate externally.

I have noticed this during development, although my dev VM is fortunately slightly faster (about 4-5 seconds for start).

I have thought about using ECDSA keys, and blocky with slightly modified code launches nearly instantaneously on Pi Zero W 2, but I decided against it in favour of greater compatibility that use of standard RSA keys provide. I think this feature has a quite niche use, and is likely not going to used by many, and certainly not in production.

If you think the use of ECDSA doesn't pose any significant compatibility issues, let me know, and I can do a PR.

@FileGo FileGo deleted the self-signed-cert branch May 29, 2022 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FR: Generate self signed certificate if cert key and file are not specified
2 participants