scp: now using SFTP protocol by default
Damien Miller
djm at mindrot.org
Mon Sep 20 11:42:36 AEST 2021
On Thu, 9 Sep 2021, Damien Miller wrote:
> FYI: the next release will have scp using the SFTP protocol by
> default.
>
> There are two known incompatibilities:
>
> Use of the SFTP protocol avoids interpretation of remote paths by
> the shell. We consider this a feature, but it does change (simplify
> really) necessary quoting of shell characters.
>
> Remote paths with a ~user/ prefix require a SFTP protocol extension
> that was included in OpenSSH 8.7's sftp-server.
>
> The original scp/rcp protocol remains available via "scp -O ..."
>
> If you're in a position to test snapshots/git prior to release
> (ETA October), then it would be appreciated.
FYI, I'm rolling this back for the release that will happen in a few
days. We want to give people a bit more time to pick up the sftp-server
"expand-path at openssh.com" extension to support ~user paths.
If you're an OpenSSH maintainer for an operating system distribution
consider either updating your stable OpenSSH to the 8.8 release when
it ships or backporting the "expand-path at openssh.com" extension to
your stable OpenSSH sftp-server. Attached are some patches to do this
for OpenSSH 8.2 and should be fairly easily adaptable to other
versions.
Removing this backwards-compatibility problem in popular distributions
will hasten the time when we can turn scp protocol off by default.
-d
-------------- next part --------------
From 8b85f54a2fdce9b749ea69ffe4f69afe559be591 Mon Sep 17 00:00:00 2001
From: Damien Miller <djm at google.com>
Date: Mon, 20 Sep 2021 11:30:50 +1000
Subject: [PATCH 1/2] Cherrypick extension advertisment code (6653c6120)
---
regress/sftp-perm.sh | 8 -----
sftp-server.c | 82 ++++++++++++++++++++++++++++----------------
2 files changed, 53 insertions(+), 37 deletions(-)
diff --git a/regress/sftp-perm.sh b/regress/sftp-perm.sh
index 304ca0ac..798966d1 100644
--- a/regress/sftp-perm.sh
+++ b/regress/sftp-perm.sh
@@ -220,14 +220,6 @@ perm_test \
"test ! -d ${COPY}.dd" \
"test -d ${COPY}.dd"
-perm_test \
- "posix-rename" \
- "realpath,stat,lstat" \
- "rename $COPY ${COPY}.1" \
- "touch $COPY" \
- "test -f ${COPY}.1 -a ! -f $COPY" \
- "test -f $COPY -a ! -f ${COPY}.1"
-
perm_test \
"rename" \
"realpath,stat,lstat" \
diff --git a/sftp-server.c b/sftp-server.c
index 359204fa..b7b54d7c 100644
--- a/sftp-server.c
+++ b/sftp-server.c
@@ -155,6 +155,18 @@ static const struct sftp_handler extended_handlers[] = {
{ NULL, NULL, 0, NULL, 0 }
};
+static const struct sftp_handler *
+extended_handler_byname(const char *name)
+{
+ int i;
+
+ for (i = 0; extended_handlers[i].handler != NULL; i++) {
+ if (strcmp(name, extended_handlers[i].ext_name) == 0)
+ return &extended_handlers[i];
+ }
+ return NULL;
+}
+
static int
request_permitted(const struct sftp_handler *h)
{
@@ -641,6 +653,29 @@ send_statvfs(u_int32_t id, struct statvfs *st)
sshbuf_free(msg);
}
+/*
+ * Prepare SSH2_FXP_VERSION extension advertisement for a single extension.
+ * The extension is checked for permission prior to advertisment.
+ */
+static int
+compose_extension(struct sshbuf *msg, const char *name, const char *ver)
+{
+ int r;
+ const struct sftp_handler *exthnd;
+
+ if ((exthnd = extended_handler_byname(name)) == NULL)
+ fatal("%s: internal error: no handler for %s", __func__, name);
+ if (!request_permitted(exthnd)) {
+ debug2("%s: refusing to advertise disallowed extension %s",
+ __func__, name);
+ return 0;
+ }
+ if ((r = sshbuf_put_cstring(msg, name)) != 0 ||
+ (r = sshbuf_put_cstring(msg, ver)) != 0)
+ fatal("%s: buffer error: %s", __func__, ssh_err(r));
+ return 0;
+}
+
/* parse incoming */
static void
@@ -655,25 +690,17 @@ process_init(void)
if ((msg = sshbuf_new()) == NULL)
fatal("%s: sshbuf_new failed", __func__);
if ((r = sshbuf_put_u8(msg, SSH2_FXP_VERSION)) != 0 ||
- (r = sshbuf_put_u32(msg, SSH2_FILEXFER_VERSION)) != 0 ||
- /* POSIX rename extension */
- (r = sshbuf_put_cstring(msg, "posix-rename at openssh.com")) != 0 ||
- (r = sshbuf_put_cstring(msg, "1")) != 0 || /* version */
- /* statvfs extension */
- (r = sshbuf_put_cstring(msg, "statvfs at openssh.com")) != 0 ||
- (r = sshbuf_put_cstring(msg, "2")) != 0 || /* version */
- /* fstatvfs extension */
- (r = sshbuf_put_cstring(msg, "fstatvfs at openssh.com")) != 0 ||
- (r = sshbuf_put_cstring(msg, "2")) != 0 || /* version */
- /* hardlink extension */
- (r = sshbuf_put_cstring(msg, "hardlink at openssh.com")) != 0 ||
- (r = sshbuf_put_cstring(msg, "1")) != 0 || /* version */
- /* fsync extension */
- (r = sshbuf_put_cstring(msg, "fsync at openssh.com")) != 0 ||
- (r = sshbuf_put_cstring(msg, "1")) != 0 || /* version */
- (r = sshbuf_put_cstring(msg, "lsetstat at openssh.com")) != 0 ||
- (r = sshbuf_put_cstring(msg, "1")) != 0) /* version */
+ (r = sshbuf_put_u32(msg, SSH2_FILEXFER_VERSION)) != 0)
fatal("%s: buffer error: %s", __func__, ssh_err(r));
+
+ /* extension advertisments */
+ compose_extension(msg, "posix-rename at openssh.com", "1");
+ compose_extension(msg, "statvfs at openssh.com", "2");
+ compose_extension(msg, "fstatvfs at openssh.com", "2");
+ compose_extension(msg, "hardlink at openssh.com", "1");
+ compose_extension(msg, "fsync at openssh.com", "1");
+ compose_extension(msg, "lsetstat at openssh.com", "1");
+
send_msg(msg);
sshbuf_free(msg);
}
@@ -1440,22 +1467,19 @@ static void
process_extended(u_int32_t id)
{
char *request;
- int i, r;
+ int r;
+ const struct sftp_handler *exthand;
if ((r = sshbuf_get_cstring(iqueue, &request, NULL)) != 0)
fatal("%s: buffer error: %s", __func__, ssh_err(r));
- for (i = 0; extended_handlers[i].handler != NULL; i++) {
- if (strcmp(request, extended_handlers[i].ext_name) == 0) {
- if (!request_permitted(&extended_handlers[i]))
- send_status(id, SSH2_FX_PERMISSION_DENIED);
- else
- extended_handlers[i].handler(id);
- break;
- }
- }
- if (extended_handlers[i].handler == NULL) {
+ if ((exthand = extended_handler_byname(request)) == NULL) {
error("Unknown extended request \"%.100s\"", request);
send_status(id, SSH2_FX_OP_UNSUPPORTED); /* MUST */
+ } else {
+ if (!request_permitted(exthand))
+ send_status(id, SSH2_FX_PERMISSION_DENIED);
+ else
+ exthand->handler(id);
}
free(request);
}
--
2.33.0.464.g1972c5931b-goog
-------------- next part --------------
From 43dc7a3fa6d1661af675ddc8d8e2060eeb4cde9e Mon Sep 17 00:00:00 2001
From: Damien Miller <djm at google.com>
Date: Mon, 20 Sep 2021 11:38:38 +1000
Subject: [PATCH 2/2] Cherrypick expand-path at openssh.com ext (2ab864010)
---
misc.c | 47 ++++++++++++++++++++++++++++-----------
misc.h | 1 +
sftp-server.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 96 insertions(+), 13 deletions(-)
diff --git a/misc.c b/misc.c
index 3a31d5c1..bcfed9a0 100644
--- a/misc.c
+++ b/misc.c
@@ -1010,29 +1010,37 @@ freeargs(arglist *args)
* Expands tildes in the file name. Returns data allocated by xmalloc.
* Warning: this calls getpw*.
*/
-char *
-tilde_expand_filename(const char *filename, uid_t uid)
+int
+tilde_expand(const char *filename, uid_t uid, char **retp)
{
const char *path, *sep;
char user[128], *ret;
struct passwd *pw;
u_int len, slash;
- if (*filename != '~')
- return (xstrdup(filename));
+ if (*filename != '~') {
+ *retp = xstrdup(filename);
+ return 0;
+ }
filename++;
path = strchr(filename, '/');
if (path != NULL && path > filename) { /* ~user/path */
slash = path - filename;
- if (slash > sizeof(user) - 1)
- fatal("tilde_expand_filename: ~username too long");
+ if (slash > sizeof(user) - 1) {
+ error("%s ~username too long", __func__);
+ return -1;
+ }
memcpy(user, filename, slash);
user[slash] = '\0';
- if ((pw = getpwnam(user)) == NULL)
- fatal("tilde_expand_filename: No such user %s", user);
- } else if ((pw = getpwuid(uid)) == NULL) /* ~/path */
- fatal("tilde_expand_filename: No such uid %ld", (long)uid);
+ if ((pw = getpwnam(user)) == NULL) {
+ error("%s: No such user %s", __func__, user);
+ return -1;
+ }
+ } else if ((pw = getpwuid(uid)) == NULL) { /* ~/path */
+ error("%s: No such uid %ld", __func__, (long)uid);
+ return -1;
+ }
/* Make sure directory has a trailing '/' */
len = strlen(pw->pw_dir);
@@ -1045,10 +1053,23 @@ tilde_expand_filename(const char *filename, uid_t uid)
if (path != NULL)
filename = path + 1;
- if (xasprintf(&ret, "%s%s%s", pw->pw_dir, sep, filename) >= PATH_MAX)
- fatal("tilde_expand_filename: Path too long");
+ if (xasprintf(&ret, "%s%s%s", pw->pw_dir, sep, filename) >= PATH_MAX) {
+ error("%s: Path too long", __func__);
+ return -1;
+ }
- return (ret);
+ *retp = ret;
+ return 0;
+}
+
+char *
+tilde_expand_filename(const char *filename, uid_t uid)
+{
+ char *ret;
+
+ if (tilde_expand(filename, uid, &ret) != 0)
+ cleanup_exit(255);
+ return ret;
}
/*
diff --git a/misc.h b/misc.h
index 4a05db2d..a090028c 100644
--- a/misc.h
+++ b/misc.h
@@ -66,6 +66,7 @@ int parse_user_host_path(const char *, char **, char **, char **);
int parse_user_host_port(const char *, char **, char **, int *);
int parse_uri(const char *, const char *, char **, char **, int *, char **);
long convtime(const char *);
+int tilde_expand(const char *, uid_t, char **);
char *tilde_expand_filename(const char *, uid_t);
char *percent_expand(const char *, ...) __attribute__((__sentinel__));
char *tohex(const void *, size_t);
diff --git a/sftp-server.c b/sftp-server.c
index b7b54d7c..41bbe344 100644
--- a/sftp-server.c
+++ b/sftp-server.c
@@ -110,6 +110,7 @@ static void process_extended_fstatvfs(u_int32_t id);
static void process_extended_hardlink(u_int32_t id);
static void process_extended_fsync(u_int32_t id);
static void process_extended_lsetstat(u_int32_t id);
+static void process_extended_expand(u_int32_t id);
static void process_extended(u_int32_t id);
struct sftp_handler {
@@ -152,6 +153,8 @@ static const struct sftp_handler extended_handlers[] = {
{ "hardlink", "hardlink at openssh.com", 0, process_extended_hardlink, 1 },
{ "fsync", "fsync at openssh.com", 0, process_extended_fsync, 1 },
{ "lsetstat", "lsetstat at openssh.com", 0, process_extended_lsetstat, 1 },
+ { "expand-path", "expand-path at openssh.com", 0,
+ process_extended_expand, 0 },
{ NULL, NULL, 0, NULL, 0 }
};
@@ -700,6 +703,7 @@ process_init(void)
compose_extension(msg, "hardlink at openssh.com", "1");
compose_extension(msg, "fsync at openssh.com", "1");
compose_extension(msg, "lsetstat at openssh.com", "1");
+ compose_extension(msg, "expand-path at openssh.com", "1");
send_msg(msg);
sshbuf_free(msg);
@@ -1463,6 +1467,63 @@ process_extended_lsetstat(u_int32_t id)
free(name);
}
+static void
+process_extended_expand(u_int32_t id)
+{
+ char cwd[PATH_MAX], resolvedname[PATH_MAX];
+ char *path, *npath;
+ int r;
+ Stat s;
+
+ if ((r = sshbuf_get_cstring(iqueue, &path, NULL)) != 0)
+ fatal("%s: parse message: %s", __func__, ssh_err(r));
+ if (getcwd(cwd, sizeof(cwd)) == NULL) {
+ send_status(id, errno_to_portable(errno));
+ goto out;
+ }
+
+ debug3("request %u: expand, original \"%s\"", id, path);
+ if (path[0] == '\0') {
+ /* empty path */
+ free(path);
+ path = xstrdup(".");
+ } else if (*path == '~') {
+ /* ~ expand path */
+ /* Special-case for "~" and "~/" to respect homedir flag */
+ if (strcmp(path, "~") == 0) {
+ free(path);
+ path = xstrdup(cwd);
+ } else if (strncmp(path, "~/", 2) == 0) {
+ npath = xstrdup(path + 2);
+ free(path);
+ xasprintf(&path, "%s/%s", cwd, npath);
+ } else {
+ /* ~user expansions */
+ if (tilde_expand(path, pw->pw_uid, &npath) != 0) {
+ send_status(id, errno_to_portable(EINVAL));
+ goto out;
+ }
+ free(path);
+ path = npath;
+ }
+ } else if (*path != '/') {
+ /* relative path */
+ xasprintf(&npath, "%s/%s", cwd, path);
+ free(path);
+ path = npath;
+ }
+ verbose("expand \"%s\"", path);
+ if (sftp_realpath(path, resolvedname) == NULL) {
+ send_status(id, errno_to_portable(errno));
+ goto out;
+ }
+ attrib_clear(&s.attrib);
+ s.name = s.long_name = resolvedname;
+ send_names(id, 1, &s);
+ out:
+ free(path);
+}
+
static void
process_extended(u_int32_t id)
{
--
2.33.0.464.g1972c5931b-goog
More information about the openssh-unix-dev
mailing list