Skip to content

rsync-ssl: -verify_ip when connecting to an IP/IPv6 address #1036

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.

Sign up for GitHub

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

Open
UTsweetyfish wants to merge 1 commit into
base: master
Choose a base branch
from
Open

rsync-ssl: -verify_ip when connecting to an IP/IPv6 address #1036

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion rsync-ssl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,19 @@ function rsync_ssl_helper {
fi

if [[ $RSYNC_SSL_TYPE == openssl ]]; then
exec $RSYNC_SSL_OPENSSL s_client $caopt $certopt $keyopt -quiet -verify_quiet -servername $hostname -verify_hostname $hostname -connect $hostname:$port
if echo "$hostname" | grep -qE '^[0-9]+\.[0-9]+\.[0-9]+\.[0-9]+$|^[0-9a-fA-F:]+$'; then

@steadytao steadytao Jul 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This pattern accepts invalid address-like values such as 999.999.999.999 and does not cover all valid IPv6 forms. Would be better to use strict IP parsing rather than classifying addresses with regex.

@UTsweetyfish UTsweetyfish Jul 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

rsync-ssl is just a simple wrapper for rsync, it will fail in rsync anyway.

btw handling addresses could be tricky, this regex seems can't handle some IPv6 addresses like link-local ones: fe80::1234%eth0. idk how to handle this, or whether we should handle this. Any advice?

@UTsweetyfish UTsweetyfish Jul 31, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

$ rsync -V
rsync  version 3.4.0  protocol version 32
...
$ rsync -vvv -e echo 'rsync://[fe80::1%eno1]:874/module'
opening connection using: echo "fe80::1%eno1" rsync --server --daemon .  (6 args)
...

@steadytao steadytao Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

hmmmmmm

@steadytao steadytao Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I feel that since rsync accepts scoped IPv6, I think the wrapper should handle it. There is no need to implement a complete IPv6 regex though. A colon unambiguously selects the IPv6 path then the zone can be removed only from the certificate identity while retained for the connection:

verify_ip=${hostname%%%*}
connect="[$hostname]:$port"

Use -verify_ip "$verify_ip" without -servername; an IP SAN cannot contain the interface zone. OpenSSL can reject malformed IPv6 itself. IPv4 is the only case that needs a small strict four-octet/range check rather than the current shape-only regex.

verify_opt="-verify_ip $hostname"
if echo "$hostname" | grep -q ':'; then
# IPv6
connect="[$hostname]:$port"
else
connect="$hostname:$port"
fi
else
verify_opt="-verify_hostname $hostname"
connect="$hostname:$port"
fi
exec $RSYNC_SSL_OPENSSL s_client $caopt $certopt $keyopt -quiet -verify_quiet -servername $hostname $verify_opt -connect "$connect"

@steadytao steadytao Jul 30, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This still omits -verify_return_error so s_client may continue after a certificate verification error. IP literals should also use -verify_ip without sending an IP address through -servername

Please add hostname, IPv4, IPv6, malformed-address and certificate-mismatch coverage.

@UTsweetyfish UTsweetyfish Jul 31, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

-verify_return_error is already in $caopt.

@steadytao steadytao Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

oh yeah it is, thank you 🤣

elif [[ $RSYNC_SSL_TYPE == gnutls ]]; then
exec $RSYNC_SSL_GNUTLS --logfile=/dev/null $gnutls_cert_opt $gnutls_key_opt $gnutls_opts $hostname:$port
else
Expand Down