ulygit / asus_rt_ac68u

Configuration and script for Cloudflare DDNS on Asuswrt-Merlin
MIT License
58 stars 24 forks source link

HEREDOC Format appears to not function on some older Routers/Firmware #23

Closed bengalih closed 1 year ago

bengalih commented 1 year ago

Hey - So I know I mentioned that I had moved off the script to the inadyn services, but it turns out I actually needed to use the script again for another purpose. I had an older Asus RT-N66 router that only supports up to FW 380.70 and therefore doesn't have inadyn support. I was setting this device up as a VPN Server box for my folks and needed the ability to keep the ddns functionality.

Anyway, I could not get the script to function at all on the box, even after a fresh install or porting it directly over from my other config. After messing around for a while, it seems to me that something about this RT-N66/FW 380.70 combination does not properly support the HEREDOC format used in the script.

Specifically, I'm talking about these lines: https://github.com/ulygit/asus_rt_ac68u/blob/37c6e6cb644c9303db9b874c0536becee4c7f8aa/cloudflare_ddns#L92

update_record()
{
    debug "update_record"
    curl --silent --show-error --request PUT "https://api.cloudflare.com/client/v4/zones/$DNS_ZONE_ID/dns_records/$DNS_RECORD_ID" \
        --data "{\"type\":\"$DNS_RECORD_TYPE\",\"name\":\"$DNS_RECORD_NAME\",\"content\":\"$NEW_IP\",\"ttl\":1,\"proxied\":$DNS_RECORD_PROXIED}" \
        -H @- <<- HEADERS
            $CLOUDFLARE_AUTH_HEADERS
            Content-Type: application/json
        HEADERS
}

I attempted to execute similar command entirely outside of the script using the following:

admin@RT-N66W-19D8:/jffs/scripts/cloudflare-ddns# CLOUDFLARE_AUTH_HEADERS="Authorization: Bearer XXXXXXXXXXXXXXXXXXXXX"
admin@RT-N66W-19D8:/jffs/scripts/cloudflare-ddns# curl --show-error --request PUT https://api.cloudflare.com/client/v4/zones/XXX/dns_records/XXX --data {\"type\":\"A\",\"name\":\"access.mydomain.com\",\"content\":\"75.X.X.X\",\"ttl\":1,\"proxied\":false} -H @- <<- HEADERS
> $CLOUDFLARE_AUTH_HEADERS
> Content-Type: application/json
> HEADERS
{"success":false,"errors":[{"code":9106,"message":"Missing X-Auth-Key, X-Auth-Email or Authorization headers"}]}

You can see this command fails saying that there are no Authorization headers because somehow the HEREDOC syntax isn't working on this box. These exact same commands function properly on my AC68U which is where I have previously used the script for years.

If instead of using the HEREDOC format I simply append the headers as normal variables as such, the operation succeeds:

admin@RT-N66W-19D8:/jffs/scripts/cloudflare-ddns# curl --show-error --request PUT https://api.cloudflare.com/client/v4/zones/XXX/dns_records/XXX
73460b95896bf --data {\"type\":\"A\",\"name\":\"access.mydomain.com\",\"content\":\"75.X.X.X\",\"ttl\":1,\"proxied\":false} -H "$CLOUDFLARE_AUTH_HEADERS" -H "Content-Type: application/js
on"
{"result":{"id":"XXXXXX","zone_id":"XXXXXXX","zone_name":"mydomain.com","name":"access.mydomain.com","type":"A","content":"75.X.X.X","proxiable":true,"proxied":false,"ttl":1,"locked":false,"meta":{"auto_added":false,"managed_by_apps":false,"managed_by_argo_tunnel":false},"comment":null,"tags":[],"created_on":"2020-10-11T22:35:14.994266Z","modified_on":"2023-04-25T16:22:25.756244Z"},"success":true,"errors":[],"messages":[]}

I can't explain this. The "sh" shell on the router doesn't support a --version command, but using the following script I determined that both systems report as:

SVR4 Bourne shell (SunOS 5 schily variant, since 2016-08-08, in posix mode)

So the shells report exactly the same and as such I would expect them to support the same syntax. The version of curl on my N66 is a bit older, but I don't think that would account for the issue using this syntax.

Finally, my AC68U does have entware installed whereas the N66 did not. But, just like the curl version above, I don't think updated versions of grep, etc would make the difference here for this syntax.

I don't recall (if I knew) why the original script preferred the HEREDOC format here instead of simply appending the variables as per my workaround. If anything, the HEREDOC format would probably have made the --data string more readable instead of using for headers.

In any event, I don't know if the script needs updating due to its deprecation, but I did want to log this issue here in case anyone else might happen to come across it and face a similar issue. Again, this seems to be an inability of some shell versions to not support HEREDOC.

EDIT: Upon some more testing I'm not sure if HEREDOC is the issue or the way that the shell is accepting standard in (@-)

weird.

bengalih commented 1 year ago

One other possible reason is it might be curl. The problematic version is : curl 7.54.1 (mipsel-unknown-linux-gnu) libcurl/7.54.1 OpenSSL/1.0.2n works fine on the other box with: curl 7.76.1 (arm-unknown-linux-gnu) libcurl/7.76.1 OpenSSL/1.1.1k

I'm curious about the text from the curl manpage for the -H command: https://curl.se/docs/manpage.html#-d

curl will make sure that each header you add/replace is sent with the proper end-of-line marker, you should thus not add that as a part of the header content: do not add newlines or carriage returns, they will only mess things up for you.

It might be possible that using the HEREDOC format is placing end of line markers (CR/LF) and the older version of curl does not deal with this properly and the newer version is more forgiving?

I don't really want to go into installing entware on the N66 just to try another version of curl, but this could be the culprit.

bengalih commented 1 year ago

OK - so I couldn't let it rest. I went ahead and installed Entware and updated curl and I was correct in my amended conclusion that curl is indeed the culprit. you can see here the exact same commands first succeeding with the newer version of curl and then failing with the older version built into the firmware:

admin@RT-N66W-19D8:/tmp/home/root# which curl
/opt/bin/curl
admin@RT-N66W-19D8:/tmp/home/root# curl --version
curl 7.57.0 (mipsel-openwrt-linux-gnu) libcurl/7.57.0 OpenSSL/1.0.2n zlib/1.2.11
Release-Date: 2017-11-29
Protocols: file ftp ftps http https imap imaps pop3 pop3s rtsp smtp smtps tftp
Features: IPv6 Largefile SSL libz HTTPS-proxy
admin@RT-N66W-19D8:/tmp/home/root# curl --show-error --request PUT https://api.cloudflare.com/client/v4/zones/XXXX/dns_records/XXXX -
-data {\"type\":\"A\",\"name\":\"access.mydomain.com\",\"content\":\"75.X.X.X\",\"ttl\":1,\"proxied\":false} -H @- <<- HEADERS
> $CLOUDFLARE_AUTH_HEADERS
> HEADERS
{"result":{"id":"XXXX","zone_id":"XXXX","zone_name":"mydomain.com","name":"access.mydomain.com","type":"A","content":"75.X.X.X","proxiable":true,"proxied":false,"ttl":1,"locked":false,"meta":{"auto_added":false,"managed_by_apps":false,"managed_by_argo_tunnel":false},"comment":null,"tags":[],"created_on":"2020-10-11T22:35:14.994266Z","modified_on":"2023-04-25T16:22:25.756244Z"},"success":true,"errors":[],"messages":[]}
admin@RT-N66W-19D8:/tmp/home/root#
admin@RT-N66W-19D8:/tmp/home/root#
admin@RT-N66W-19D8:/tmp/home/root# /usr/sbin/curl --version
curl 7.54.1 (mipsel-unknown-linux-gnu) libcurl/7.54.1 OpenSSL/1.0.2n
Release-Date: 2017-06-14
Protocols: file ftp ftps http https imap imaps pop3 pop3s rtsp smb smbs smtp smtps tftp
Features: IPv6 Largefile NTLM NTLM_WB SSL TLS-SRP UnixSockets HTTPS-proxy
admin@RT-N66W-19D8:/tmp/home/root# /usr/sbin/curl --show-error --request PUT https://api.cloudflare.com/client/v4/zones/XXXX/dns_records/a386c5fb83b97664c0c73460
b95896bf --data {\"type\":\"A\",\"name\":\"access.mydomain.com\",\"content\":\"75.X.X.X\",\"ttl\":1,\"proxied\":false} -H @- <<- HEADERS
> $CLOUDFLARE_AUTH_HEADERS
> HEADERS
{"success":false,"errors":[{"code":9106,"message":"Missing X-Auth-Key, X-Auth-Email or Authorization headers"}]}
admin@RT-N66W-19D8:/tmp/home/root#

I don't know at what version market these changes go into effect, but I do believe based on the quote from the curl page above that the new behavior is simply being more gracious, and the way the script is coded is probably not ideal for how curl expects headers (with no new line characters). At the very least you may want to place a note in the main documentation stating that older versions of curl may misbehave and that utilizing an updated version from entware or another repository is recommended.

thanks!

ulygit commented 1 year ago

Thanks for digging into this @bengalih. I'm in the middle of a rough week, but I'll make a note of the issue in the main page next week. I might tweak the script to address this, but it's unlikely. I have to value code stability at this stage of the project lifecycle. I seem to recall the here-doc helped with code maintenance, but it's been a few years now.

Again, appreciate your enthusiasm for figuring things out! :D

bengalih commented 1 year ago

No problem - again I defer to you both as it won't make much difference to me (based on my current usage and my ability to fix my own issues if required). I would however say this for your consideration:

As we've discovered this script is basically deprecated with newer versions of Merlin in favor of inadyn. So for anyone who has a FW that supports inadyn should probably be using that instead as it is better integrated. This means that this script's primary usage is not for those who have older FW that do not support inadyn. And, in those cases, as I have shown this script likely won't run in its current configuration due to the version of curl on those devices.

Of course, any bug is "as-designed" with appropriate documentation. So as long as you reference the issues here and propose that in order for the script to function they might need a working entware install with updated curl, then most people can resolve this.

At this point there is really no reason for any existing users to upgrade from a functioning version of this script and likely there never will be unless Cloudflare changes its API.

ulygit commented 1 year ago

Closing as WONTFIX in deference to code stability and due to available workaround. Bug and workaround are noted in docs.