[PATCH 3/4] sftp-server: initial experimental SFTPv4 support

Mike Frysinger vapier at gentoo.org
Fri Oct 1 06:49:21 AEST 2021


This updates the server to handle SFTPv4.  It is not enabled by default
though -- SFTPv3 is still the advertised version.  Users have to pass
an explicit -V4 to use it for now.

Here's a summary of protocol differences from v3 to v4 and how they're
handled.

* symlink argument order
OpenSSH has a long standing known incompatibility with the SFTPv3 spec
where it accidentally swapped symlink's path arguments.  We can take
the opportunity to fix that for SFTPv4+.

* 64-bit time & nanoseconds
The main driver here is the expanded stat structure: we get 64-bit time
(important for y2k38 compatibility), and we get nanosecond resolution.

SFTPv4 also includes support for creation time (not to be confused with
last status change time aka "ctime"), but we don't currently provide it.
OS support for accessing the field is still pretty new and has not been
standardized (e.g. Linux has statx() vs st_birthtime in *BSD).  This
could be added in the future, but let's just skip it for now.

* User & group names
We also get account names (user & group) instead of numeric ids which
helps with system portability.  Numeric ids still work, they just get
serialized into a string first (for client->server requests).

* UTF-8 paths
The specification clarifies that paths are to be treated as UTF-8.  We
don't make any changes to the server side here -- paths are just bytes.
As long as the server filesystems are all UTF-8 encoded, everything will
work fine.  If the client attempts to use non-UTF-8 encoded paths, that
is an error on the client side, and the server is allowed to do whatever
it wants (we just opt for SFTPv3 behavior).  The specification also talks
about the client having to accept normalized forms (NFC) that the server
may provide, but it does not require that the server always use NFC.  So
from a protocol point-of-view, we don't have to do anything server-side,
and it's left to the person running the server to be compliant.

* Text mode
Attempts to use the new "text" mode when opening files is not supported.
This is not trivial to implement, and doesn't seem like it's worth the
effort to support.  We'll return an explicit "not supported" error, and
clients will have to continue treating everything as binary (like SFTPv3
and older do).

Along those lines, the "newline" extension is not implemented.  Since it
isn't advertised, clients shouldn't try to use it.

* ACLs
The other big concern people have is for the new ACL format.  This field
in the file attributes is simply ignored.  It's an opaque string, so it's
trivial to consume & skip.  We don't have to define or process any of the
fields it provides.  The specification is unclear where the new structure
and its fields even show up.

The server will never emit ACL information in the file attributes.  If a
client includes ACL info while trying to set file attributes, or open a
file, the server will warn & ignore rather than abort.
---
 PROTOCOL      |   6 +-
 sftp-common.c | 178 +++++++++++++++++++++++++++++++++++++++++---------
 sftp-common.h |  12 +++-
 sftp-server.c | 160 +++++++++++++++++++++++++++++++++++----------
 4 files changed, 286 insertions(+), 70 deletions(-)

diff --git a/PROTOCOL b/PROTOCOL
index 42c040d65a04..9a66f25a9d41 100644
--- a/PROTOCOL
+++ b/PROTOCOL
@@ -344,19 +344,21 @@ BSD-derived systems.
 
 3. SFTP protocol changes
 
-3.1. sftp: Reversal of arguments to SSH_FXP_SYMLINK
+3.1. sftp: Reversal of arguments to SSH_FXP_SYMLINK in SFTPv3
 
 When OpenSSH's sftp-server was implemented, the order of the arguments
 to the SSH_FXP_SYMLINK method was inadvertently reversed. Unfortunately,
 the reversal was not noticed until the server was widely deployed. Since
 fixing this to follow the specification would cause incompatibility, the
 current order was retained. For correct operation, clients should send
-SSH_FXP_SYMLINK as follows:
+SSH_FXP_SYMLINK as follows when using SFTPv3 or older:
 
 	uint32		id
 	string		targetpath
 	string		linkpath
 
+This has been fixed for SFTPv4 and newer.
+
 3.2. sftp: Server extension announcement in SSH_FXP_VERSION
 
 OpenSSH's sftp-server lists the extensions it supports using the
diff --git a/sftp-common.c b/sftp-common.c
index 7681206728a7..b81c12b90e2d 100644
--- a/sftp-common.c
+++ b/sftp-common.c
@@ -57,13 +57,7 @@ unsigned int sftp_version = 3;
 void
 attrib_clear(Attrib *a)
 {
-	a->flags = 0;
-	a->size = 0;
-	a->uid = 0;
-	a->gid = 0;
-	a->perm = 0;
-	a->atime = 0;
-	a->mtime = 0;
+	memset(a, 0, sizeof(*a));
 }
 
 /* Convert from struct stat to filexfer attribs */
@@ -74,14 +68,39 @@ stat_to_attrib(const struct stat *st, Attrib *a)
 	a->flags = 0;
 	a->flags |= SSH2_FILEXFER_ATTR_SIZE;
 	a->size = st->st_size;
-	a->flags |= SSH2_FILEXFER_ATTR_UIDGID;
-	a->uid = st->st_uid;
-	a->gid = st->st_gid;
 	a->flags |= SSH2_FILEXFER_ATTR_PERMISSIONS;
 	a->perm = st->st_mode;
-	a->flags |= SSH2_FILEXFER_ATTR_ACMODTIME;
-	a->atime = st->st_atime;
-	a->mtime = st->st_mtime;
+
+	switch (sftp_version) {
+	case 0 ... 3:
+		a->flags |= SSH2_FILEXFER_ATTR_UIDGID;
+		a->uid = st->st_uid;
+		a->gid = st->st_gid;
+		a->flags |= SSH2_FILEXFER_ATTR_ACMODTIME;
+		a->atime = st->st_atime;
+		a->mtime = st->st_mtime;
+		break;
+	case 4 ... 6:
+		if (S_ISREG(st->st_mode))
+			a->type = SSH2_FILEXFER_TYPE_REGULAR;
+		else if (S_ISDIR(st->st_mode))
+			a->type = SSH2_FILEXFER_TYPE_DIRECTORY;
+		else if (S_ISLNK(st->st_mode))
+			a->type = SSH2_FILEXFER_TYPE_SYMLINK;
+		else
+			a->type = SSH2_FILEXFER_TYPE_UNKNOWN;
+		a->flags |= SSH2_FILEXFER_ATTR_OWNERGROUP;
+		a->owner = user_from_uid(st->st_uid, 0);
+		a->group = group_from_gid(st->st_gid, 0);
+		a->flags |= SSH2_FILEXFER_ATTR_ACCESSTIME;
+		a->atime = st->st_atime;
+		a->flags |= SSH2_FILEXFER_ATTR_MODIFYTIME;
+		a->mtime = st->st_mtime;
+		a->flags |= SSH2_FILEXFER_ATTR_SUBSECOND_TIMES;
+		a->atime_nsec = st->st_atim.tv_nsec;
+		a->mtime_nsec = st->st_mtim.tv_nsec;
+		break;
+	}
 }
 
 /* Convert from filexfer attribs to struct stat */
@@ -92,15 +111,32 @@ attrib_to_stat(const Attrib *a, struct stat *st)
 
 	if (a->flags & SSH2_FILEXFER_ATTR_SIZE)
 		st->st_size = a->size;
-	if (a->flags & SSH2_FILEXFER_ATTR_UIDGID) {
-		st->st_uid = a->uid;
-		st->st_gid = a->gid;
-	}
 	if (a->flags & SSH2_FILEXFER_ATTR_PERMISSIONS)
 		st->st_mode = a->perm;
-	if (a->flags & SSH2_FILEXFER_ATTR_ACMODTIME) {
-		st->st_atime = a->atime;
-		st->st_mtime = a->mtime;
+	switch (sftp_version) {
+	case 0 ... 3:
+		if (a->flags & SSH2_FILEXFER_ATTR_UIDGID) {
+			st->st_uid = a->uid;
+			st->st_gid = a->gid;
+		}
+		if (a->flags & SSH2_FILEXFER_ATTR_ACMODTIME) {
+			st->st_atime = a->atime;
+			st->st_mtime = a->mtime;
+		}
+		break;
+	case 4 ... 6:
+		/* NB: Owner & group handling omitted for now. */
+		if (a->flags & SSH2_FILEXFER_ATTR_ACCESSTIME) {
+			st->st_atime = a->atime;
+			if (a->flags & SSH2_FILEXFER_ATTR_SUBSECOND_TIMES)
+				st->st_atim.tv_nsec = a->atime_nsec;
+		}
+		if (a->flags & SSH2_FILEXFER_ATTR_MODIFYTIME) {
+			st->st_mtime = a->mtime;
+			if (a->flags & SSH2_FILEXFER_ATTR_SUBSECOND_TIMES)
+				st->st_mtim.tv_nsec = a->mtime_nsec;
+		}
+		break;
 	}
 }
 
@@ -113,23 +149,66 @@ decode_attrib(struct sshbuf *b, Attrib *a)
 	attrib_clear(a);
 	if ((r = sshbuf_get_u32(b, &a->flags)) != 0)
 		return r;
+	if (sftp_version >= 4) {
+		if ((r = sshbuf_get_u8(b, &a->type)) != 0)
+			return r;
+	}
 	if (a->flags & SSH2_FILEXFER_ATTR_SIZE) {
 		if ((r = sshbuf_get_u64(b, &a->size)) != 0)
 			return r;
 	}
-	if (a->flags & SSH2_FILEXFER_ATTR_UIDGID) {
+	if (sftp_version <= 3 && a->flags & SSH2_FILEXFER_ATTR_UIDGID) {
 		if ((r = sshbuf_get_u32(b, &a->uid)) != 0 ||
 		    (r = sshbuf_get_u32(b, &a->gid)) != 0)
 			return r;
 	}
+	if (sftp_version >= 4 && a->flags & SSH2_FILEXFER_ATTR_OWNERGROUP) {
+		if ((r = sshbuf_get_cstring(b, &a->owner, NULL)) != 0 ||
+		    (r = sshbuf_get_cstring(b, &a->group, NULL)) != 0)
+			return r;
+	}
 	if (a->flags & SSH2_FILEXFER_ATTR_PERMISSIONS) {
 		if ((r = sshbuf_get_u32(b, &a->perm)) != 0)
 			return r;
 	}
-	if (a->flags & SSH2_FILEXFER_ATTR_ACMODTIME) {
-		if ((r = sshbuf_get_u32(b, &a->atime)) != 0 ||
-		    (r = sshbuf_get_u32(b, &a->mtime)) != 0)
-			return r;
+	if (sftp_version <= 3) {
+		if (a->flags & SSH2_FILEXFER_ATTR_ACMODTIME) {
+			u_int32_t atime, mtime;
+			if ((r = sshbuf_get_u32(b, &atime)) != 0 ||
+			    (r = sshbuf_get_u32(b, &mtime)) != 0)
+				return r;
+			a->atime = atime;
+			a->mtime = mtime;
+		}
+	} else if (sftp_version >= 4) {
+		if (a->flags & SSH2_FILEXFER_ATTR_ACCESSTIME) {
+			if ((r = sshbuf_get_u64(b, &a->atime)) != 0)
+				return r;
+			if (a->flags & SSH2_FILEXFER_ATTR_SUBSECOND_TIMES &&
+			    (r = sshbuf_get_u32(b, &a->atime_nsec)) != 0)
+				return r;
+		}
+		if (a->flags & SSH2_FILEXFER_ATTR_CREATETIME) {
+			if ((r = sshbuf_get_u64(b, &a->createtime)) != 0)
+				return r;
+			if (a->flags & SSH2_FILEXFER_ATTR_SUBSECOND_TIMES &&
+			    (r = sshbuf_get_u32(b, &a->createtime_nsec)) != 0)
+				return r;
+		}
+		if (a->flags & SSH2_FILEXFER_ATTR_MODIFYTIME) {
+			if ((r = sshbuf_get_u64(b, &a->mtime)) != 0)
+				return r;
+			if (a->flags & SSH2_FILEXFER_ATTR_SUBSECOND_TIMES &&
+			    (r = sshbuf_get_u32(b, &a->mtime_nsec)) != 0)
+				return r;
+		}
+		if (a->flags & SSH2_FILEXFER_ATTR_ACL) {
+			char *acl;
+			debug("Ignoring ACL attributes");
+			if ((r = sshbuf_get_cstring(b, &acl, NULL)) != 0)
+				return r;
+			free(acl);
+		}
 	}
 	/* vendor-specific extensions */
 	if (a->flags & SSH2_FILEXFER_ATTR_EXTENDED) {
@@ -161,23 +240,62 @@ encode_attrib(struct sshbuf *b, const Attrib *a)
 
 	if ((r = sshbuf_put_u32(b, a->flags)) != 0)
 		return r;
+	if (sftp_version >= 4) {
+		if ((r = sshbuf_put_u8(b, a->type)) != 0)
+			return r;
+	}
 	if (a->flags & SSH2_FILEXFER_ATTR_SIZE) {
 		if ((r = sshbuf_put_u64(b, a->size)) != 0)
 			return r;
 	}
-	if (a->flags & SSH2_FILEXFER_ATTR_UIDGID) {
+	if (sftp_version <= 3 && a->flags & SSH2_FILEXFER_ATTR_UIDGID) {
 		if ((r = sshbuf_put_u32(b, a->uid)) != 0 ||
 		    (r = sshbuf_put_u32(b, a->gid)) != 0)
 			return r;
 	}
+	if (sftp_version >= 4 && a->flags & SSH2_FILEXFER_ATTR_OWNERGROUP) {
+		if ((r = sshbuf_put_cstring(b, a->owner)) != 0 ||
+		    (r = sshbuf_put_cstring(b, a->group)) != 0)
+			return r;
+	}
 	if (a->flags & SSH2_FILEXFER_ATTR_PERMISSIONS) {
 		if ((r = sshbuf_put_u32(b, a->perm)) != 0)
 			return r;
 	}
-	if (a->flags & SSH2_FILEXFER_ATTR_ACMODTIME) {
-		if ((r = sshbuf_put_u32(b, a->atime)) != 0 ||
-		    (r = sshbuf_put_u32(b, a->mtime)) != 0)
-			return r;
+	switch (sftp_version) {
+	case 0 ... 3:
+		if (a->flags & SSH2_FILEXFER_ATTR_ACMODTIME) {
+			if ((r = sshbuf_put_u32(b, a->atime)) != 0 ||
+			    (r = sshbuf_put_u32(b, a->mtime)) != 0)
+				return r;
+		}
+		break;
+	case 4 ... 6:
+		if (a->flags & SSH2_FILEXFER_ATTR_ACCESSTIME) {
+			if ((r = sshbuf_put_u64(b, a->atime)) != 0)
+				return r;
+			if (a->flags & SSH2_FILEXFER_ATTR_SUBSECOND_TIMES &&
+			    (r = sshbuf_put_u32(b, a->atime_nsec)) != 0)
+				return r;
+		}
+		if (a->flags & SSH2_FILEXFER_ATTR_CREATETIME) {
+			if ((r = sshbuf_put_u64(b, a->atime)) != 0)
+				return r;
+			if (a->flags & SSH2_FILEXFER_ATTR_SUBSECOND_TIMES &&
+			    (r = sshbuf_put_u32(b, a->atime_nsec)) != 0)
+				return r;
+		}
+		if (a->flags & SSH2_FILEXFER_ATTR_MODIFYTIME) {
+			if ((r = sshbuf_put_u64(b, a->atime)) != 0)
+				return r;
+			if (a->flags & SSH2_FILEXFER_ATTR_SUBSECOND_TIMES &&
+			    (r = sshbuf_put_u32(b, a->atime_nsec)) != 0)
+				return r;
+		}
+		if (a->flags & SSH2_FILEXFER_ATTR_ACL) {
+			debug("Ignoring ACL attributes");
+		}
+		break;
 	}
 	return 0;
 }
diff --git a/sftp-common.h b/sftp-common.h
index 5fc8b45da39a..0dc37dc364a9 100644
--- a/sftp-common.h
+++ b/sftp-common.h
@@ -36,12 +36,20 @@ extern unsigned int sftp_version;
 /* File attributes */
 struct Attrib {
 	u_int32_t	flags;
+	u_int8_t	type;
 	u_int64_t	size;
 	u_int32_t	uid;
 	u_int32_t	gid;
+	/* NB: These are not allocated and thus never freed. */
+	char		*owner;
+	char		*group;
 	u_int32_t	perm;
-	u_int32_t	atime;
-	u_int32_t	mtime;
+	int64_t		atime;
+	u_int32_t	atime_nsec;
+	int64_t		createtime;
+	u_int32_t	createtime_nsec;
+	int64_t		mtime;
+	u_int32_t	mtime_nsec;
 };
 
 void	 attrib_clear(Attrib *);
diff --git a/sftp-server.c b/sftp-server.c
index bef7b6c18aa7..cb5577ec5d7d 100644
--- a/sftp-server.c
+++ b/sftp-server.c
@@ -33,6 +33,7 @@
 #include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <grp.h>
 #include <pwd.h>
 #include <stdlib.h>
 #include <stdio.h>
@@ -54,7 +55,7 @@
 #include "sftp-common.h"
 
 /* The max SFTP version we support */
-#define SSH2_FILEXFER_VERSION_MAX	3
+#define SSH2_FILEXFER_VERSION_MAX	4
 
 char *sftp_realpath(const char *, char *); /* sftp-realpath.c */
 
@@ -314,6 +315,27 @@ string_from_portable(int pflags)
 	return ret;
 }
 
+/* Convert string account names to local system ids. */
+static void
+portable_accts_to_id(const char *owner, const char *group, uid_t *uid,
+    gid_t *gid)
+{
+	struct passwd *owner_pw;
+	struct group *group_grp;
+
+	owner_pw = getpwnam(owner);
+	if (owner_pw)
+		*uid = owner_pw->pw_uid;
+	else
+		*uid = strtoul(owner, NULL, 10);
+
+	group_grp = getgrnam(group);
+	if (group_grp)
+		*gid = group_grp->gr_gid;
+	else
+		*gid = strtoul(group, NULL, 10);
+}
+
 /* handle handles */
 
 typedef struct Handle Handle;
@@ -634,10 +656,13 @@ send_names(u_int32_t id, int count, const Stat *stats)
 		fatal_fr(r, "compose");
 	debug("request %u: sent names count %d", id, count);
 	for (i = 0; i < count; i++) {
-		if ((r = sshbuf_put_cstring(msg, stats[i].name)) != 0 ||
-		    (r = sshbuf_put_cstring(msg, stats[i].long_name)) != 0 ||
-		    (r = encode_attrib(msg, &stats[i].attrib)) != 0)
-			fatal_fr(r, "compose filenames/attrib");
+		if ((r = sshbuf_put_cstring(msg, stats[i].name)) != 0)
+			fatal_fr(r, "compose filename");
+		if (sftp_version <= 3 &&
+		    (r = sshbuf_put_cstring(msg, stats[i].long_name)) != 0)
+			fatal_fr(r, "compose longname");
+		if ((r = encode_attrib(msg, &stats[i].attrib)) != 0)
+			fatal_fr(r, "compose attrs");
 	}
 	send_msg(msg);
 	sshbuf_free(msg);
@@ -763,6 +788,12 @@ process_open(u_int32_t id)
 		fatal_fr(r, "parse");
 
 	debug3("request %u: open flags %d", id, pflags);
+	if (sftp_version >= 4 && a.flags & SSH2_FXF_TEXT) {
+		verbose("Text mode not supported");
+		status = SSH2_FX_OP_UNSUPPORTED;
+		goto out;
+	}
+
 	flags = flags_from_portable(pflags);
 	mode = (a.flags & SSH2_FILEXFER_ATTR_PERMISSIONS) ? a.perm : 0666;
 	logit("open \"%s\" flags %s mode 0%o",
@@ -786,6 +817,8 @@ process_open(u_int32_t id)
 			}
 		}
 	}
+
+ out:
 	if (status != SSH2_FX_OK)
 		send_status(id, status);
 	free(name);
@@ -976,9 +1009,9 @@ attrib_to_tv(const Attrib *a)
 	static struct timeval tv[2];
 
 	tv[0].tv_sec = a->atime;
-	tv[0].tv_usec = 0;
+	tv[0].tv_usec = a->atime_nsec / 1000;
 	tv[1].tv_sec = a->mtime;
-	tv[1].tv_usec = 0;
+	tv[1].tv_usec = a->mtime_nsec / 1000;
 	return tv;
 }
 
@@ -988,9 +1021,9 @@ attrib_to_ts(const Attrib *a)
 	static struct timespec ts[2];
 
 	ts[0].tv_sec = a->atime;
-	ts[0].tv_nsec = 0;
+	ts[0].tv_nsec = a->atime_nsec;
 	ts[1].tv_sec = a->mtime;
-	ts[1].tv_nsec = 0;
+	ts[1].tv_nsec = a->mtime_nsec;
 	return ts;
 }
 
@@ -1019,7 +1052,9 @@ process_setstat(u_int32_t id)
 		if (r == -1)
 			status = errno_to_portable(errno);
 	}
-	if (a.flags & SSH2_FILEXFER_ATTR_ACMODTIME) {
+
+	if ((sftp_version <= 3 && a.flags & SSH2_FILEXFER_ATTR_ACMODTIME) ||
+	    (sftp_version >= 4 && a.flags & SSH2_FILEXFER_ATTR_ACCESSTIME)) {
 		char buf[64];
 		time_t t = a.mtime;
 
@@ -1030,13 +1065,25 @@ process_setstat(u_int32_t id)
 		if (r == -1)
 			status = errno_to_portable(errno);
 	}
-	if (a.flags & SSH2_FILEXFER_ATTR_UIDGID) {
-		logit("set \"%s\" owner %lu group %lu", name,
-		    (u_long)a.uid, (u_long)a.gid);
-		r = chown(name, a.uid, a.gid);
-		if (r == -1)
-			status = errno_to_portable(errno);
+
+	if (sftp_version <= 3) {
+		if (a.flags & SSH2_FILEXFER_ATTR_UIDGID) {
+ attr_uidgid:
+			logit("set \"%s\" owner %lu group %lu", name,
+			    (u_long)a.uid, (u_long)a.gid);
+			r = chown(name, a.uid, a.gid);
+			if (r == -1)
+				status = errno_to_portable(errno);
+		}
+	} else {
+		if (a.flags & SSH2_FILEXFER_ATTR_OWNERGROUP) {
+			logit("set \"%s\" owner %s group %s", name,
+			    a.owner, a.group);
+			portable_accts_to_id(a.owner, a.group, &a.uid, &a.gid);
+			goto attr_uidgid;
+		}
 	}
+
 	send_status(id, status);
 	free(name);
 }
@@ -1076,7 +1123,10 @@ process_fsetstat(u_int32_t id)
 			if (r == -1)
 				status = errno_to_portable(errno);
 		}
-		if (a.flags & SSH2_FILEXFER_ATTR_ACMODTIME) {
+		if ((sftp_version <= 3 &&
+		     a.flags & SSH2_FILEXFER_ATTR_ACMODTIME) ||
+		    (sftp_version >= 4 &&
+		     a.flags & SSH2_FILEXFER_ATTR_ACCESSTIME)) {
 			char buf[64];
 			time_t t = a.mtime;
 
@@ -1091,16 +1141,27 @@ process_fsetstat(u_int32_t id)
 			if (r == -1)
 				status = errno_to_portable(errno);
 		}
-		if (a.flags & SSH2_FILEXFER_ATTR_UIDGID) {
-			logit("set \"%s\" owner %lu group %lu", name,
-			    (u_long)a.uid, (u_long)a.gid);
+		if (sftp_version <= 3) {
+			if (a.flags & SSH2_FILEXFER_ATTR_UIDGID) {
+ attr_uidgid:
+				logit("set \"%s\" owner %lu group %lu", name,
+				    (u_long)a.uid, (u_long)a.gid);
 #ifdef HAVE_FCHOWN
-			r = fchown(fd, a.uid, a.gid);
+				r = fchown(fd, a.uid, a.gid);
 #else
-			r = chown(name, a.uid, a.gid);
+				r = chown(name, a.uid, a.gid);
 #endif
-			if (r == -1)
-				status = errno_to_portable(errno);
+				if (r == -1)
+					status = errno_to_portable(errno);
+			}
+		} else {
+			if (a.flags & SSH2_FILEXFER_ATTR_OWNERGROUP) {
+				logit("set \"%s\" owner %s group %s", name,
+				    a.owner, a.group);
+				portable_accts_to_id(a.owner, a.group, &a.uid,
+				    &a.gid);
+				goto attr_uidgid;
+			}
 		}
 	}
 	send_status(id, status);
@@ -1172,7 +1233,11 @@ process_readdir(u_int32_t id)
 				continue;
 			stat_to_attrib(&st, &(stats[count].attrib));
 			stats[count].name = xstrdup(dp->d_name);
-			stats[count].long_name = ls_file(dp->d_name, &st, 0, 0);
+			if (sftp_version <= 3)
+				stats[count].long_name = ls_file(dp->d_name,
+				    &st, 0, 0);
+			else
+				stats[count].long_name = NULL;
 			count++;
 			/* send up to 100 entries in one message */
 			/* XXX check packet size instead */
@@ -1365,9 +1430,19 @@ process_symlink(u_int32_t id)
 	char *oldpath, *newpath;
 	int r, status;
 
-	if ((r = sshbuf_get_cstring(iqueue, &oldpath, NULL)) != 0 ||
-	    (r = sshbuf_get_cstring(iqueue, &newpath, NULL)) != 0)
-		fatal_fr(r, "parse");
+	/*
+	 * For SFTPv3 and older, the arguments were implemented in the incorrect
+	 * order.  It's fixed for SFTPv4+.  See PROTOCOL for more info.
+	 */
+	if (sftp_version <= 3) {
+		if ((r = sshbuf_get_cstring(iqueue, &oldpath, NULL)) != 0 ||
+		    (r = sshbuf_get_cstring(iqueue, &newpath, NULL)) != 0)
+			fatal_fr(r, "parse");
+	} else {
+		if ((r = sshbuf_get_cstring(iqueue, &newpath, NULL)) != 0 ||
+		    (r = sshbuf_get_cstring(iqueue, &oldpath, NULL)) != 0)
+			fatal_fr(r, "parse");
+	}
 
 	debug3("request %u: symlink", id);
 	logit("symlink old \"%s\" new \"%s\"", oldpath, newpath);
@@ -1498,7 +1573,9 @@ process_extended_lsetstat(u_int32_t id)
 		if (r == -1)
 			status = errno_to_portable(errno);
 	}
-	if (a.flags & SSH2_FILEXFER_ATTR_ACMODTIME) {
+
+	if ((sftp_version <= 3 && a.flags & SSH2_FILEXFER_ATTR_ACMODTIME) ||
+	    (sftp_version >= 4 && a.flags & SSH2_FILEXFER_ATTR_ACCESSTIME)) {
 		char buf[64];
 		time_t t = a.mtime;
 
@@ -1510,13 +1587,24 @@ process_extended_lsetstat(u_int32_t id)
 		if (r == -1)
 			status = errno_to_portable(errno);
 	}
-	if (a.flags & SSH2_FILEXFER_ATTR_UIDGID) {
-		logit("set \"%s\" owner %lu group %lu", name,
-		    (u_long)a.uid, (u_long)a.gid);
-		r = fchownat(AT_FDCWD, name, a.uid, a.gid,
-		    AT_SYMLINK_NOFOLLOW);
-		if (r == -1)
-			status = errno_to_portable(errno);
+
+	if (sftp_version <= 3) {
+		if (a.flags & SSH2_FILEXFER_ATTR_UIDGID) {
+ attr_uidgid:
+			logit("set \"%s\" owner %lu group %lu", name,
+			    (u_long)a.uid, (u_long)a.gid);
+			r = fchownat(AT_FDCWD, name, a.uid, a.gid,
+			    AT_SYMLINK_NOFOLLOW);
+			if (r == -1)
+				status = errno_to_portable(errno);
+		}
+	} else {
+		if (a.flags & SSH2_FILEXFER_ATTR_OWNERGROUP) {
+			logit("set \"%s\" owner %s group %s", name,
+			    a.owner, a.group);
+			portable_accts_to_id(a.owner, a.group, &a.uid, &a.gid);
+			goto attr_uidgid;
+		}
 	}
  out:
 	send_status(id, status);
-- 
2.33.0



More information about the openssh-unix-dev mailing list