Open Tokarak opened 1 week ago
This is technically ready to be merged, but I would like to add a command line toggle with this, to advertise this feature, and to emphasasize that it is dangerous.
Planned functionality:
--no-https-verification
explaining that it is behind a feature, and that it should only be enabled for debug purposes as it is a security concern.no-https-verification
feature disabled:
--no-https-verification
is passedno-https-verification
feature enabled:
--no-https-verification
--no-https-verification
is passed--no-https-verification
is not passedno-https-verification
feature in debug modeI'd also like to have a banner message displayed if HTTPS verification is disabled, so 1. we don't accidentally enable it 2. users are warned that the traffic could be logged. I know it can just be patched out but nonetheless seems prudent. (Also ignore failing checks)
I'd also like to have a banner message displayed if HTTPS verification is disabled Suggestion: put this in the instance info page, so that it is always available without breaking the aesthetic. This is also extendable to other debug features.
I'm putting this in "help wanted". This PR is accepting PRs.
Progress update: I have added the command line flag, but it has no functionality.
This pr should probably be rebased into at least two commits, since https://github.com/redlib-org/redlib/pull/254/commits/edb16f29ce5d9b79f3683d2264260eb191893ffe should be a seperate commit.
I'm stuck on passing the flag from the main thread to the instantiation of the hyper_rustls client in client.rs. I can probably create a global 'static optional bool variable, and save the outcome there in the main thread; client.rs will then unwrap that variable. But that feels hacky — any better suggestions? I think refactoring into using a Config struct and a clap::Parser pattern is what is needed. I suppose this pr can do the messy hack with a //TODO comment, and then a seperate pr can fix this.
One option would be replacing the Lazy with a OnceCell and instantiating it manually in main.rs
.
I implemented the functionality and edited your checklist - last is the instance info page. IMO though the feature/flag shouldn't be enabled in dev mode - some people might accidentally leave it in debug mode permanently when hosting an instance.
Anyway about the instance info: That might necessitate a OnceLock bool anyway for the actual CLI flag (so the template code can access it). In which case, we don't need the code I made about choosing it at runtime, we could simply initialize the OnceLock bool first, then lazily initialize the client, unwrapping that bool. Up to you if you want to go with that method, or just leave it as-is and bolt on the boolean.
Rationale: disabling https verification allows debugging the https connection by proxying Redlib through an HTTPS sniffer. This has been discussed in #249.
Closes #249