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

allow protocol to always override ssl in geturl() #482

Closed
wants to merge 1 commit into from

Conversation

mmogilvi
Copy link
Contributor

Signed-off-by: Matthew Ogilvie mmogilvi+ddc@zoho.com

Signed-off-by: Matthew Ogilvie <mmogilvi+ddc@zoho.com>
@mmogilvi
Copy link
Contributor Author

Background:

I have long used the "use=web" option to extract my IP address from my router with my own config (to avoid guessing which if any "fw" might work, and to avoid offloading anything to free services I don't absolutely have to), but a recent OS update (gentoo) to ddclient 3.10.0 seems to have changed things so the global ssl option is now honored when looking up IP address via web, and not only is it honored, it also takes precedence over an explicitly-specified "web=http://..." (non-https) prefix.

Perhaps I could try disabling the global ssl option instead of patching the code, but I suspect that isn't a good idea unless there is some other way to ensure dyndns2 uses ssl. Or is SSL unconditionally part of dyndns2? (I tried prefixing the dyndns2 part to "server=https://..." to override a lack of global "ssl=yes", but that didn't work, and I didn't find any documented technique at all.)

FUTURE: I haven't confirmed, but I would guess some of the "fw" options probably may have similar problems, although I haven't fully traced through the code to be sure.

FUTURE: It might be better to have a clear way to indicate SSL/non-SSL for every individual kind of internet interaction (including "protocol=dyndns2"), and deprecate the global "ssl" option.

@SuperSandro2000
Copy link
Member

SuperSandro2000 commented Jan 13, 2023

The idea is that forcessl enforces ssl everywhere and ssl should only load/use it by default.

FUTURE: It might be better to have a clear way to indicate SSL/non-SSL for every individual kind of internet interaction (including "protocol=dyndns2"), and deprecate the global "ssl" option.

There are no resources to implement this.

Comment on lines +2435 to +2444
## canonify use_ssl, proxy and url
if ($url =~ /^https:/) {
$use_ssl = 1;
} elsif ($url =~ /^http:/) {
$use_ssl = 0;
} elsif ($globals{'ssl'} && !($params{ignore_ssl_option} // 0)) {
$use_ssl = 1;
} else {
$use_ssl = 0;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep forcessl to always use ssl no matter what the URL says.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is that forcessl enforces ssl everywhere and ssl should only load/use it by default.

Please keep forcessl to always use ssl no matter what the URL says.

Huh? forcessl local variable was only ever set based on the URL in the first place! (Old lines 2436 and 2437). Were you maybe misreading the old code and thinking it was based on a global option or something? Or am I somehow blind to something else? (Or maybe you didn't notice the second "chunk" of the patch that simplifies the later part of the function to only read (not write) "use_ssl", since github seems to confusingly "collapse" that part of the patch by default for some reason?)

My replacement lines 2436 and 2437 get rid of the separate variable and is functionally identical to how forcessl was used, as the first/highest priority part of a consolidated chain of prioritized elseif's that ONLY decides whether to to use SSL or not, instead of scattering and intermingling the "use SSL" decision logic across multiple places in the function. Seems clearer than the old code to me, but I don't really care as long there is SOME way I can configure it the way I want (no SSL getting IP address from my router on my LAN, but use SSL for anything else ddclient does, such as the actual ddns registration). If you would prefer I code the same logic some other way, I would need to know more about what you are thinking.

(I haven't investigated the registration part carefully, but my impression is that the ddns registration can be SSL, but the only way to do it is with the global "ssl" option, leaving it necessary to use some other technique to disable SSL for IP lookup? Or is there something incorrect in my impressions as described?)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were you maybe misreading the old code and thinking it was based on a global option or something?

yeah, thought it was a global option.

Seems clearer than the old code to me, but I don't really care as long there is SOME way I can configure it the way I want (no SSL getting IP address from my router on my LAN, but use SSL for anything else ddclient does, such as the actual ddns registration).

That behaviors is abused in multiple places to enforce https. I don't want to change that and don't have the time to fix it properly.

If you would prefer I code the same logic some other way, I would need to know more about what you are thinking.

I didn't think to much about it and with the limited time I spend on ddclient.

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

Successfully merging this pull request may close these issues.

None yet

2 participants