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 not recognized #39

Closed
rEnr3n opened this issue Sep 28, 2021 · 6 comments
Closed

Self-signed certificate not recognized #39

rEnr3n opened this issue Sep 28, 2021 · 6 comments
Labels
bug Something isn't working pinned Prevent from automatically closing due to inactivity

Comments

@rEnr3n
Copy link

rEnr3n commented Sep 28, 2021

Describe the bug
I'm testing website-stalker against my website on my local network. It has a certificate trusted by my device. Running website-stalker gives me an error saying it's an invalid certificate.

Versions

  • Version of website-stalker: 0.13.0
  • Version of rustc: n/a
  • Version of cargo: n/a

To Reproduce
Steps to reproduce the behavior:

  1. Setup test website (e.g. https://server.lan/) with self-signed certificate
  2. Trust certificate on all participating devices system-wide
  3. Use the config below
  4. Run website-stalker run --all --commit
  5. See error below

Expected behavior
The error should not be there. The contents of the webpage should have been committed.

Additional information

$ curl -Is https://server.lan | head -n1
HTTP/2 200

website-stalker.yaml

---
from: admin@server.lan
sites:
  - url: https://server.lan/
    extension: html
    editors:
      - html_prettify

Error:

$ website-stalker run --all --commit
Begin stalking of 1 sites on 1 domains...
ERROR: https://server.lan/ error sending request for url (https://server.lan/): error trying to connect: invalid certificate: UnknownIssuer
ERROR: All done but some site failed.
Thanks for using website-stalker!
@rEnr3n rEnr3n added the bug Something isn't working label Sep 28, 2021
@EdJoPaTo
Copy link
Owner

EdJoPaTo commented Sep 28, 2021

I looked into it and dump some links for future me (or someone else) to look into it at a later time.

Something useful might be a way to ignore certificate errors (via ClientBuilder::danger_accept_invalid_certs).

Currently rustls is used instead of reqwests default OpenSSL. Maybe enabling rustls-native-certs might help in this regards. reqwest has the feature flag rustls-tls-native-roots for this.
The crate lists some pros and cons if it should be used or not. Any thoughts on them?

@rEnr3n
Copy link
Author

rEnr3n commented Sep 28, 2021

This is just my opinion but from what I can tell this is the major con of rustls-native-certs:

  • The OS update system may, in fact, be quite poor at keeping the root certificates up-to-date

This is not really an issue for me as I use a rolling-release distro but it's definitely an issue for those using "stable" distros.

Is it possible to install both rustls-native-certs and webpki-roots at the same time? The idea is to choose between the two depending on which OS it is installed. If that's not possible I would lean on ignoring certificate errors. I don't expect most people to use this application against non-ICANN domains. I can settle for this as a temporary workaround.

@EdJoPaTo
Copy link
Owner

yeah, I think optional accept_invalid_certs with the built in certificates are a good way to work around it here.
Its probably a good idea to configure this per entry and not globally for all entries? More explicit ignore this. Also it takes more effort to do so → its easier to not just ignore everything.

When thinking about it, as the certificates are bundled the binary has to be fairly up to date in order to ensure up to date certificates. So its either the OS or the binary itself in this case.

@rEnr3n
Copy link
Author

rEnr3n commented Sep 29, 2021

Its probably a good idea to configure this per entry and not globally for all entries?

Better if both options are available.

When thinking about it, as the certificates are bundled the binary has to be fairly up to date in order to ensure up to date certificates.

That's a problem if you, the developer, becomes inactive for a long time. Sites could become inaccessible just by using an outdated set of certificates.

EdJoPaTo added a commit that referenced this issue Oct 6, 2021
@EdJoPaTo
Copy link
Owner

EdJoPaTo commented Oct 6, 2021

I think staying with rustls seems like the best choice. Building with updated dependencies will be possible even if become inactive as this is an open-source tool which can be self compiled. Also rustls isnt using external non-rust-stuff which is probably a safer thing to do in general. We just need to be aware that regular updates are a must when rustls is included in the binary and not on system level.

I added accept_invalid_certs only as site config in order to limit the usage of it. Its easier to not use it which should be the default.

Regarding the self signed certificates using the system store might be a good solution but as long as there isnt bigger interest I think using accept_invalid_certs is just the way with less added complexity to go.

@EdJoPaTo EdJoPaTo added the pinned Prevent from automatically closing due to inactivity label Oct 6, 2021
@EdJoPaTo
Copy link
Owner

EdJoPaTo commented Mar 3, 2023

As this feature seems to be working generally I will close this issue. If there are other feature requests or ideas feel free to comment or open a new issue.

@EdJoPaTo EdJoPaTo closed this as completed Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pinned Prevent from automatically closing due to inactivity
Projects
None yet
Development

No branches or pull requests

2 participants