-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
ddns-scripts_route53-v1: incorrectly complains about curl not being SSL-capable #6744
Comments
Similar variables like |
Found the problem. |
@chris5560 Is this 8bb49eb#diff-636b35c9cb832b1c8ca09254fbf759c9 the offending comimt? |
I updated my PR to undo the part of the commit that introduced this problem. This fixes the problem for me without adding any noticeable delay. It's unclear to me what the root cause of the 10 s (!) delay is that @Ansuel claimed to fix with #6103. I would expect this check to be very fast and in fact on my hardware it is:
Also note that @Ansuel did not move the |
fixed in master. |
@mark0n execute that command using console is ok Lua should never execute external command as it's slow in every way... |
The fix is wrong. The right fix is to disable the check, and here is why. Let's start with a fresh LEDE 17.01.4.
You see - there is no way to install curl without SSL support. Let's test it in the same way as ddns-scripts do:
So - fresh curl does claim to support https. Of course it does not work:
So, please: remove the pointless $CURL_SSL check, and add the dependency on ca-bundle. |
On 2018-08-24 10:45 PM, Alexander E. Patrakov wrote:
The fix is wrong. The right fix is to disable the check, and here is why.
Actually it's right because curl can be built without SSL support (it's
not the default build option, but it is a supported self-build option).
You see - there is no way to install curl without SSL support.
Let's test it in the same way as ddns-scripts do:
***@***.***:~# $(which curl) -V 2>/dev/null | grep "Protocols:" Protocols:
file ftp ftps http https |
So - fresh curl does claim to support https. Of course it does not work:
***@***.***:~# curl https://www.google.com/ curl: (77) Error reading ca
cert file /etc/ssl/certs/ca-certificates.crt - mbedTLS: (-0x3E00) PK -
Read/write of file failed |
So, please: remove the pointless $CURL_SSL check, and add the dependency
on ca-bundle.
We don't do that in ddns-script because SSL support is optional. For
curl the issue is that curl users may want to use only their own CA and
not ca-bundle. So NAK from me.
Regards,
Daniel
|
Are there any plans to push this out to the repository? Seem to still be encountering this in ddns-scripts 2.7.8-1. |
@ethanbergstrom please retest with current release level (2.7.8-5) and raise a new issue, if you encounter still an issue. Thanks! |
The latest version available out on the default repo shipped with OpenWRT (http://downloads.openwrt.org/releases/18.06.1/packages/mips_24kc/packages/) is 2.7.8-1. Manually splicing in the changes did fix it, but it doesnt look like revision 5 has been published. |
It's currently only available in snapshot repo, see here: |
Installed 2.7.8-1, got the Installed 2.7.8-6, can confirm it is fixed! |
This is still not in the release packages. Installing from the snapshot package repo fixed this. I am not familiar with the process, does someone need to make a pull-request for this? Another thing to watch out for, when I installed the snapshot ddns-scripts base package, it 'lost' the route53 option in the DDNS provider selection. So I installed the latetest snapshot ddns-scripts_route53-v1 package 2.7.8-9 but this now seems to be broken, as it's missing the username/password fields in the LUCI interface. Got it working eventually by keeping the base scripts package from the snapshot repo and removing then re-installing the release package of the route53 scripts. |
Maintainer
@chris5560, @maxberger
Environment
OpenWRT 18.06.0
Description
DDNS script for AWS Route 53 aborts with the following error:
Root cause: Variable $CURL_SSL is checked in
/usr/lib/ddns/update_route53_v1.sh
but not initialized anywhere. The script aborts if this variable is empty.Package versions
Suggested solution
Commenting out the check fixes the problem for me (note that the variable doesn't seem to be used). Shouldn't the package dependencies ensure that cURL is SSL-capable?
The text was updated successfully, but these errors were encountered: