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

Support for DNS and HTTPS (and self-signed certificates) #106

Open
Abrynos opened this issue Jun 3, 2024 · 6 comments
Open

Support for DNS and HTTPS (and self-signed certificates) #106

Abrynos opened this issue Jun 3, 2024 · 6 comments

Comments

@Abrynos
Copy link
Contributor

Abrynos commented Jun 3, 2024

As the title suggests, I'd like to see support for encrypted connections via HTTPS.

My use-case seems quite simple at a glance, but has multiple caveats if you think it through:

My home-network is set up with

  • a local DNS server (the screen will receive that via DHCP from the router; lookup didn't work when I tested) and
  • encrypted connections for everything. (means nginx on my raspberry pi having default configuration shown in the spoiler below redirecting any and all traffic from port 80 to 443 immediately)

Furthermore, there is more than one service available on the device that runs klipper. Those services are all available via different names (imagine something like my3dprinter.local and someothersoftware.local as domain names). This means, that - in order to get to the correct endpoints - the Host header must be set correctly. This is complicated even more by the fact that the root-certificate that signed the printers certificate was self-signed (due to the nature of local-network-only availability).

For now I've worked around this by allowing access to klipper via plain HTTP on the IP address in nginx, but I'd really prefer a better solution. I'm willing to help however I can and if need be contribute to the project myself, but I'm not familiar with the code so I'll at least need a hint on where to start for that. 😉

nginx default configuration
server {
  listen 80 default_server;
  listen [::]:80 default_server;

  location / {
    return 301 https://$host$request_uri;
  }
}
@suchmememanyskill
Copy link
Owner

I consider this issue out of scope, it's adding unnecessary complexity to the entire stack.

But even then, i suspect this isn't possible at all to implement, as you have no real way to load the self signed certificate to communicate.

Also, multiple .local addresses per ip sounds like spec abuse to me(?)

@Abrynos
Copy link
Contributor Author

Abrynos commented Jun 4, 2024

I consider this issue out of scope, it's adding unnecessary complexity to the entire stack.

While I agree that HTTPS would add qutie a bit of complexity, I do believe that neither host-resolving via DNS nor adding the Host header to the HTTP requests fit that description. After all the input already says "Enter Klipper IP/Hostname and Port".

But even then, i suspect this isn't possible at all to implement, as you have no real way to load the self signed certificate to communicate.

Technically it's not necessary to load certificates in advance. This part is only relevant if the implementation had a bunch of "trusted root certificate authorities" like your normal computer/browser (This would be the most complicated option to implement and only resolve my use-case if it has a check-box named "ignore certificate"). A primitive implementation could just accept whatever it gets from the server during the TLS handshake handshake without question.

Also, multiple .local addresses per ip sounds like spec abuse to me(?)

As far as I understood the spec (don't take my word for it), it's perfectly fine. But for the sake of the argument you can assume any TLD such as .intranet, .personal, .internal or .home.

@suchmememanyskill
Copy link
Owner

Host-resolving via DNS should already be functional, as i connect to github pages to fetch the latest firmware for OTA updates.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Host according to mozilla a Host header should be sent, so i suppose fair enough.

I'm currently using the esp32's standard library to make http requests. If it's configurable to ignore ssl errors, i'm fine with implementing it

@Abrynos
Copy link
Contributor Author

Abrynos commented Jun 5, 2024

Host-resolving via DNS should already be functional

After checking again it indeed seems to work; This seems to be an issue with some TLDs and is/was probably me doing something wrong.

according to mozilla a Host header should be sent, so i suppose fair enough.

Thanks! That's what I like to hear 😄 I've just checked out the code and saw it's probably a one-liner in src/core/http_client.cpp. Let me know if you prefer me sending a merge request or if you like to do it yourself.

If it's configurable to ignore ssl errors, i'm fine with implementing it

According to this page (beware of the ads) it should just be a call to client.setInsecure(); (possibly in the same source file mentioend above). The (I assume) more complex part is determining whether it's possible to use HTTPS instead of plain HTTP. I saw the hardcoded string-concattenation with "http://" and I'm not sure whether it's worth the effort of automatically determining the protocol. We can't hardcode it based on the port either because according to the spec it's perfectly fine to provide HTTPS on port 80 and plain HTTP on port 443. Maybe it's even possible leaving the protocol part out of the URI, but I don't know the library. So depending on the effort of automatically determining the protocol, it might be simplest to just add a checkbox for the user.

@suchmememanyskill
Copy link
Owner

Sorry for the delayed response;

I don't use SecureHttpClient, i experienced a few issues with it.
image

HttpClient also seems to already add the host header: https://github.com/espressif/arduino-esp32/blob/master/libraries/HTTPClient/src/HTTPClient.cpp#L1118

@Abrynos
Copy link
Contributor Author

Abrynos commented Jun 10, 2024

Sorry for the delayed response

No worries. Soon I'll go on holiday and then I'll be the one responding late.

I don't use SecureHttpClient

In that case we'd need to change the call to the begin method. Ideally, we'd just use the call to bool HTTPClient::begin(String host, uint16_t port, String uri, const char* CAcert) with a nullptr as the cert, but it seems parameter validation is inconsistent between methods in the current implementation (see this issue) and we'd need to use the overload supplying NetworkClient& instead. (In general, there are overloads already taking host, port and path as separate arguments so in terms of runtime it would probably be an advantage to use one of those instead of packaging the parameters up into a string and the library parsing them out again)

HttpClient also seems to already add the host header

My bad. I just double-checked and apparently I was looking at the wrong column in my logs. Not sure what went wrong there with my brain-dead self.

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

No branches or pull requests

2 participants