-
-
Notifications
You must be signed in to change notification settings - Fork 97
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
Virtualmin 7.20.0 pre-release possible bugs #841
Comments
|
I cleaned up your commit in f54f4b7 |
|
You're welcome. However, your clean-up isn't correct from my perspective. The whole point of my fix was to handle different types of data, because
Alright!
If you're sure about it, then we're safe in this case!
Yes, though I don't think we can address this easily. In my example, I think, in the case of unattended requests, we shouldn't check or validate IPv6 at all, as it has been causing a lot of issues
Yes, I understand that. However, my concern is that some users might have already saved a template. |
How can @err contain anything other than a list of hash refs? That's all that
I mean we could turn that off for this case, but in other cases it will lead to failed LE requests that are hard for users to understand.
Maybe we need to check if the first version installed was below 7.10.0, and if so set |
Note - previously the error was returned as: Setting up initial SSL certificate .. .. domain validation failed : HASH(0x55e6034cb970)
Hmm, I think I incorrectly assumed that the
I agree. But how do we deal with this then? Perhaps we don't check for IPv6 if the domain's
Cool, that could work! Clever idea!
Well, it may break existing installs. I think we can just handle it the way you just suggested. |
How can Virtualmin tell if the domain isn't hosted locally though? It does skip that IPv6 check already if |
It's not easy to do exact, I don't think.
Yeah, I know. But couldn't we add more checks to avoid false-positive results for such case scenarios? |
Not sure how we can do that without failing to detect true positives! |
I think we should handle this for specific DNS providers, particularly when Virtualmin doesn't manage the DNS. For example, with Cloudflare, if the Virtualmin system doesn't have control over DNS (i.e. We could use a dedicated function for this and add other DNS services later if needed. |
Maaaybe .. but that's going to be hard to do reliability, because it depends on knowing that DNS is not just hosted elsewhere, but also at a provider like Cloudflare that does proxying. |
Hey Jamie!
I have made a real life tests, starting with a clean install with Virtualmin 7.20.0 tagged code and discovered a few possible issues.
I usually don't use Contabo, but a few users reported some issues with it, so I decided to give it a try myself.
❗ Keep in mind that the DNS for the domain is hosted on Cloudflare, but Virtualmin is not configured to handle it. I did this intentionally to create a test case where Virtualmin doesn't have control over DNS!
The issue that I have fixed was related to how errors were incorrectly printed when validation on domain creation time is enabled (default) but errors are enabled to be shown (non default). The patch for this is here d673898.
This issue hasn't been addressed on my side. The problem is that when we create the initial SSL certificate, Virtualmin discards domains that cannot be resolved using the new feature. However, when visiting the SSL Certificate page afterwards, Virtualmin still shows the full set of SANs, for example:
Domain creation time:
Later on, when visiting the SSL Certificate page, Virtualmin still shows the full set of associated domains:
🪲 I believe it shouldn't and should display only the domains from the last successful request.
Question: Will this new feature work for the renewal requests made in the background? What I mean is, will Virtualmin remove SANs that aren't resolvable when renewing a domain in the background, during a cron job?
If domain validation isn't disabled during SSL certificate creation at domain creation time in the Virtualmin config (default), the Virtualmin validation process still fails with this error:
The problem traces back to this initial patch d7306b9 .. however, I don't believe that this code is the issue on its own.
🪲 I don't think we can reliably validate IPv6 addresses in this configuration scenario with CF proxy enabled. My suggestion is to disable IPv6 validation completely when an SSL certificate is requested during the initial domain creation, where the user doesn't have the option to disable validation.
The text was updated successfully, but these errors were encountered: