Re: ftp(1): new -s flag to specify source IP address
I'm ok with the diff, can I get another ok before commiting ?
On Tue, Apr 24, 2012 at 10:53:34PM -0400, Lawrence Teo wrote:
> On Wed, Apr 18, 2012 at 11:58:26PM -0400, Lawrence Teo wrote:
> > This diff adds a -s flag to ftp(1) to let the user specify the
> > source IP address of the connection. This is useful when using
> > ftp(1) over VPN tunnels or when an alternate source IP is required
> > to fetch a file from a FTP/HTTP/HTTPS server due to access control
> > policies.
>
> I have revised this diff based on feedback from haesbaert@. The primary
> change is to use AI_NUMERICHOST in the hints addrinfo struct that's
> passed to getaddrinfo().
>
> The resulting ftp(1) binary still passes all 48 tests from my test
> script at http://lteo.net/stuff/ftp-test
>
> Comments and more testing are welcome!
>
> Thanks,
> Lawrence
>
>
> Index: fetch.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
> retrieving revision 1.104
> diff -u -p -r1.104 fetch.c
> --- fetch.c 23 Apr 2012 21:22:02 - 1.104
> +++ fetch.c 25 Apr 2012 02:12:13 -
> @@ -179,7 +179,7 @@ url_get(const char *origline, const char
> char *hosttail, *cause = "unknown", *newline, *host, *port, *buf = NULL;
> char *epath, *redirurl, *loctail;
> int error, i, isftpurl = 0, isfileurl = 0, isredirect = 0, rval = -1;
> - struct addrinfo hints, *res0, *res;
> + struct addrinfo hints, *res0, *res, *ares = NULL;
> const char * volatile savefile;
> char * volatile proxyurl = NULL;
> char *cookie = NULL;
> @@ -198,6 +198,7 @@ url_get(const char *origline, const char
> #endif /* !SMALL */
> SSL *ssl = NULL;
> int status;
> + int save_errno;
>
> direction = "received";
>
> @@ -490,6 +491,17 @@ noslash:
> goto cleanup_url_get;
> }
>
> +#ifndef SMALL
> + if (srcaddr) {
> + hints.ai_flags |= AI_NUMERICHOST;
> + error = getaddrinfo(srcaddr, NULL, &hints, &ares);
> + if (error) {
> + warnx("%s: %s", gai_strerror(error), srcaddr);
> + goto cleanup_url_get;
> + }
> + }
> +#endif /* !SMALL */
> +
> s = -1;
> for (res = res0; res; res = res->ai_next) {
> if (getnameinfo(res->ai_addr, res->ai_addrlen, hbuf,
> @@ -504,10 +516,28 @@ noslash:
> continue;
> }
>
> +#ifndef SMALL
> + if (srcaddr) {
> + if (ares->ai_family != res->ai_family) {
> + close(s);
> + s = -1;
> + errno = EINVAL;
> + cause = "bind";
> + continue;
> + }
> + if (bind(s, ares->ai_addr, ares->ai_addrlen) < 0) {
> + save_errno = errno;
> + close(s);
> + errno = save_errno;
> + s = -1;
> + cause = "bind";
> + continue;
> + }
> + }
> +#endif /* !SMALL */
> +
> again:
> if (connect(s, res->ai_addr, res->ai_addrlen) < 0) {
> - int save_errno;
> -
> if (errno == EINTR)
> goto again;
> save_errno = errno;
> @@ -532,6 +562,10 @@ again:
> break;
> }
> freeaddrinfo(res0);
> +#ifndef SMALL
> + if (srcaddr)
> + freeaddrinfo(ares);
> +#endif /* !SMALL */
> if (s < 0) {
> warn("%s", cause);
> goto cleanup_url_get;
> Index: ftp.1
> ===
> RCS file: /cvs/src/usr.bin/ftp/ftp.1,v
> retrieving revision 1.81
> diff -u -p -r1.81 ftp.1
> --- ftp.1 26 Jul 2010 21:31:34 - 1.81
> +++ ftp.1 25 Apr 2012 02:12:13 -
> @@ -42,10 +42,12 @@
> .Op Fl k Ar seconds
> .Op Fl P Ar port
> .Op Fl r Ar seconds
> +.Op Fl s Ar srcaddr
> .Op Ar host Op Ar port
> .Nm ftp
> .Op Fl C
> .Op Fl o Ar output
> +.Op Fl s Ar srcaddr
> .Sm off
> .No ftp:// Oo Ar user : password No @
> .Oc Ar host Oo : Ar port
> @@ -57,6 +59,7 @@
> .Op Fl C
> .Op Fl c Ar cookie
> .Op Fl o Ar output
> +.Op Fl s Ar srcaddr
> .Sm off
> .No http:// Ar host Oo : Ar port
> .Oc No / Ar file
> @@ -66,6 +69,7 @@
> .Op Fl C
> .Op Fl c Ar cookie
> .Op Fl o Ar output
> +.Op Fl s Ar srcaddr
> .Sm off
> .No https:// Ar host Oo : Ar port
> .Oc No / Ar file
> @@ -74,6 +78,7 @@
> .Nm ftp
> .Op Fl C
> .Op Fl o Ar output
> +.Op Fl s Ar srcaddr
> .Sm off
> .No file: Ar file
> .Sm on
> @@ -81,6 +86,7 @@
> .Nm ftp
> .Op Fl C
> .Op Fl o Ar output
> +.Op Fl s Ar srcaddr
> .Sm off
> .Ar host : No / Ar file Oo /
> .Oc
> @@ -220,6 +226,12 @@ if the server does not suppor
Re: ftp(1): new -s flag to specify source IP address
On Wed, Apr 18, 2012 at 11:58:26PM -0400, Lawrence Teo wrote:
> This diff adds a -s flag to ftp(1) to let the user specify the
> source IP address of the connection. This is useful when using
> ftp(1) over VPN tunnels or when an alternate source IP is required
> to fetch a file from a FTP/HTTP/HTTPS server due to access control
> policies.
I have revised this diff based on feedback from haesbaert@. The primary
change is to use AI_NUMERICHOST in the hints addrinfo struct that's
passed to getaddrinfo().
The resulting ftp(1) binary still passes all 48 tests from my test
script at http://lteo.net/stuff/ftp-test
Comments and more testing are welcome!
Thanks,
Lawrence
Index: fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.104
diff -u -p -r1.104 fetch.c
--- fetch.c 23 Apr 2012 21:22:02 - 1.104
+++ fetch.c 25 Apr 2012 02:12:13 -
@@ -179,7 +179,7 @@ url_get(const char *origline, const char
char *hosttail, *cause = "unknown", *newline, *host, *port, *buf = NULL;
char *epath, *redirurl, *loctail;
int error, i, isftpurl = 0, isfileurl = 0, isredirect = 0, rval = -1;
- struct addrinfo hints, *res0, *res;
+ struct addrinfo hints, *res0, *res, *ares = NULL;
const char * volatile savefile;
char * volatile proxyurl = NULL;
char *cookie = NULL;
@@ -198,6 +198,7 @@ url_get(const char *origline, const char
#endif /* !SMALL */
SSL *ssl = NULL;
int status;
+ int save_errno;
direction = "received";
@@ -490,6 +491,17 @@ noslash:
goto cleanup_url_get;
}
+#ifndef SMALL
+ if (srcaddr) {
+ hints.ai_flags |= AI_NUMERICHOST;
+ error = getaddrinfo(srcaddr, NULL, &hints, &ares);
+ if (error) {
+ warnx("%s: %s", gai_strerror(error), srcaddr);
+ goto cleanup_url_get;
+ }
+ }
+#endif /* !SMALL */
+
s = -1;
for (res = res0; res; res = res->ai_next) {
if (getnameinfo(res->ai_addr, res->ai_addrlen, hbuf,
@@ -504,10 +516,28 @@ noslash:
continue;
}
+#ifndef SMALL
+ if (srcaddr) {
+ if (ares->ai_family != res->ai_family) {
+ close(s);
+ s = -1;
+ errno = EINVAL;
+ cause = "bind";
+ continue;
+ }
+ if (bind(s, ares->ai_addr, ares->ai_addrlen) < 0) {
+ save_errno = errno;
+ close(s);
+ errno = save_errno;
+ s = -1;
+ cause = "bind";
+ continue;
+ }
+ }
+#endif /* !SMALL */
+
again:
if (connect(s, res->ai_addr, res->ai_addrlen) < 0) {
- int save_errno;
-
if (errno == EINTR)
goto again;
save_errno = errno;
@@ -532,6 +562,10 @@ again:
break;
}
freeaddrinfo(res0);
+#ifndef SMALL
+ if (srcaddr)
+ freeaddrinfo(ares);
+#endif /* !SMALL */
if (s < 0) {
warn("%s", cause);
goto cleanup_url_get;
Index: ftp.1
===
RCS file: /cvs/src/usr.bin/ftp/ftp.1,v
retrieving revision 1.81
diff -u -p -r1.81 ftp.1
--- ftp.1 26 Jul 2010 21:31:34 - 1.81
+++ ftp.1 25 Apr 2012 02:12:13 -
@@ -42,10 +42,12 @@
.Op Fl k Ar seconds
.Op Fl P Ar port
.Op Fl r Ar seconds
+.Op Fl s Ar srcaddr
.Op Ar host Op Ar port
.Nm ftp
.Op Fl C
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.No ftp:// Oo Ar user : password No @
.Oc Ar host Oo : Ar port
@@ -57,6 +59,7 @@
.Op Fl C
.Op Fl c Ar cookie
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.No http:// Ar host Oo : Ar port
.Oc No / Ar file
@@ -66,6 +69,7 @@
.Op Fl C
.Op Fl c Ar cookie
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.No https:// Ar host Oo : Ar port
.Oc No / Ar file
@@ -74,6 +78,7 @@
.Nm ftp
.Op Fl C
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.No file: Ar file
.Sm on
@@ -81,6 +86,7 @@
.Nm ftp
.Op Fl C
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.Ar host : No / Ar file Oo /
.Oc
@@ -220,6 +226,12 @@ if the server does not support passive c
.It Fl r Ar seconds
Retry to connect if failed, pausing for number of
.Ar seconds .
+.It Fl s Ar srcaddr
+Use
+.Ar srcaddr
+on the local machine as the source address
+of the connection.
+Only useful on systems with more than one address.
.It Fl t
Enables packet tracing.
.It Fl V
Index: ftp.c
=
ftp(1): new -s flag to specify source IP address
This diff adds a -s flag to ftp(1) to let the user specify the
source IP address of the connection. This is useful when using
ftp(1) over VPN tunnels or when an alternate source IP is required
to fetch a file from a FTP/HTTP/HTTPS server due to access control
policies.
The -s flag is present in ftp(1) on FreeBSD, NetBSD, DragonFly BSD,
and even OS X so it is very portable in shell scripts. :)
I have tested -s with both IPv4 and IPv6 with a variety of different
flags. The following is the test script that I used during
development to confirm that this diff does not break existing
behavior and to ensure that -s works with related flags:
http://lteo.net/stuff/ftp-test
These are the test results:
http://lteo.net/stuff/ftp-test-results.txt
(with my IPv6 address masked out)
Note: If you would like to replicate the test with my test script,
please assign two IPv4 addresses and two IPv6 addresses to your
egress interface. If you don't have IPv6 connectivity, you can run
the script in IPv4-only mode with "ftp-test -4"
Comments and feedback would be appreciated.
Thanks,
Lawrence
Index: fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.103
diff -u -p -r1.103 fetch.c
--- fetch.c 25 Aug 2010 20:32:37 - 1.103
+++ fetch.c 15 Apr 2012 02:08:43 -
@@ -179,7 +179,7 @@ url_get(const char *origline, const char
char *hosttail, *cause = "unknown", *newline, *host, *port, *buf = NULL;
char *epath, *redirurl, *loctail;
int error, i, isftpurl = 0, isfileurl = 0, isredirect = 0, rval = -1;
- struct addrinfo hints, *res0, *res;
+ struct addrinfo hints, *res0, *res, *ares = NULL;
const char * volatile savefile;
char * volatile proxyurl = NULL;
char *cookie = NULL;
@@ -198,6 +198,7 @@ url_get(const char *origline, const char
#endif /* !SMALL */
SSL *ssl = NULL;
int status;
+ int save_errno;
direction = "received";
@@ -490,6 +491,16 @@ noslash:
goto cleanup_url_get;
}
+#ifndef SMALL
+ if (srcaddr) {
+ error = getaddrinfo(srcaddr, NULL, &hints, &ares);
+ if (error) {
+ warnx("%s: %s", gai_strerror(error), srcaddr);
+ goto cleanup_url_get;
+ }
+ }
+#endif /* !SMALL */
+
s = -1;
for (res = res0; res; res = res->ai_next) {
if (getnameinfo(res->ai_addr, res->ai_addrlen, hbuf,
@@ -504,6 +515,26 @@ noslash:
continue;
}
+#ifndef SMALL
+ if (srcaddr) {
+ if (ares->ai_family != res->ai_family) {
+ close(s);
+ s = -1;
+ errno = EINVAL;
+ cause = "bind";
+ continue;
+ }
+ if (bind(s, ares->ai_addr, ares->ai_addrlen) < 0) {
+ save_errno = errno;
+ close(s);
+ errno = save_errno;
+ s = -1;
+ cause = "bind";
+ continue;
+ }
+ }
+#endif /* !SMALL */
+
again:
if (connect(s, res->ai_addr, res->ai_addrlen) < 0) {
int save_errno;
@@ -532,6 +563,10 @@ again:
break;
}
freeaddrinfo(res0);
+#ifndef SMALL
+ if (srcaddr)
+ freeaddrinfo(ares);
+#endif /* !SMALL */
if (s < 0) {
warn("%s", cause);
goto cleanup_url_get;
Index: ftp.1
===
RCS file: /cvs/src/usr.bin/ftp/ftp.1,v
retrieving revision 1.81
diff -u -p -r1.81 ftp.1
--- ftp.1 26 Jul 2010 21:31:34 - 1.81
+++ ftp.1 15 Apr 2012 02:08:43 -
@@ -42,10 +42,12 @@
.Op Fl k Ar seconds
.Op Fl P Ar port
.Op Fl r Ar seconds
+.Op Fl s Ar srcaddr
.Op Ar host Op Ar port
.Nm ftp
.Op Fl C
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.No ftp:// Oo Ar user : password No @
.Oc Ar host Oo : Ar port
@@ -57,6 +59,7 @@
.Op Fl C
.Op Fl c Ar cookie
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.No http:// Ar host Oo : Ar port
.Oc No / Ar file
@@ -66,6 +69,7 @@
.Op Fl C
.Op Fl c Ar cookie
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.No https:// Ar host Oo : Ar port
.Oc No / Ar file
@@ -74,6 +78,7 @@
.Nm ftp
.Op Fl C
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.No file: Ar file
.Sm on
@@ -81,6 +86,7 @@
.Nm ftp
.Op Fl C
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.Ar host : No / Ar file Oo /
.Oc
@@ -220,6 +226,12 @@ if the server does not support passive c
.It Fl r Ar seconds
Retry to connect if failed, pausing for number o
