From 0ef28075d98a43308bf6e9b5159c800beeeb2c2b Mon Sep 17 00:00:00 2001 From: Cam Cope Date: Thu, 2 Sep 2021 13:59:00 -0700 Subject: [PATCH 01/14] fix buffer leak --- sshfs.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/sshfs.c b/sshfs.c index 8addecbb..819cefa8 100644 --- a/sshfs.c +++ b/sshfs.c @@ -1384,17 +1384,20 @@ static int sftp_read(struct conn *conn, uint8_t *type, struct buffer *buf) buf_init(&buf2, 5); res = do_read(conn, &buf2); if (res != -1) { - if (buf_get_uint32(&buf2, &len) == -1) - return -1; + if ((res = buf_get_uint32(&buf2, &len)) == -1) + goto out; if (len > MAX_REPLY_LEN) { fprintf(stderr, "reply len too large: %u\n", len); - return -1; + res = -1; + goto out; + } + if ((res = buf_get_uint8(&buf2, type)) == -1) { + goto out; } - if (buf_get_uint8(&buf2, type) == -1) - return -1; buf_init(buf, len - 1); res = do_read(conn, buf); } +out: buf_free(&buf2); return res; } From 006ffb53196ef0cfd532e0673169a427e6f08177 Mon Sep 17 00:00:00 2001 From: Cam Cope Date: Mon, 26 Jul 2021 18:21:51 -0700 Subject: [PATCH 02/14] use new constants for protocol based message size limits --- sshfs.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/sshfs.c b/sshfs.c index 819cefa8..08a9a3fc 100644 --- a/sshfs.c +++ b/sshfs.c @@ -126,8 +126,6 @@ #define MY_EOF 1 -#define MAX_REPLY_LEN (1 << 17) - #define RENAME_TEMP_CHARS 8 #define SFTP_SERVER_PATH "/usr/lib/sftp-server" @@ -137,6 +135,8 @@ #define READDIR_MAX 32 #define MAX_PASSWORD 1024 +#define DEFAULT_MAX_SFTP_MSG_LIMIT 65536 +#define DEFAULT_MIN_SFTP_MSG_LIMIT 32768 /* Handling of multiple SFTP connections @@ -1386,7 +1386,7 @@ static int sftp_read(struct conn *conn, uint8_t *type, struct buffer *buf) if (res != -1) { if ((res = buf_get_uint32(&buf2, &len)) == -1) goto out; - if (len > MAX_REPLY_LEN) { + if (len > sshfs.max_read) { fprintf(stderr, "reply len too large: %u\n", len); res = -1; goto out; @@ -1580,8 +1580,10 @@ static int sftp_init_reply_ok(struct conn *conn, struct buffer *buf, if (buf_get_uint32(buf, &len) == -1) return -1; - if (len < 5 || len > MAX_REPLY_LEN) + if (len < 5 || len > DEFAULT_MAX_SFTP_MSG_LIMIT) { + DEBUG("Invalid init packet length: %u\n", len); return 1; + } if (buf_get_uint8(buf, &type) == -1) return -1; @@ -4173,9 +4175,6 @@ int main(int argc, char *argv[]) #else sshfs.blksize = 4096; #endif - /* SFTP spec says all servers should allow at least 32k I/O */ - sshfs.max_read = 32768; - sshfs.max_write = 32768; #ifdef __APPLE__ sshfs.rename_workaround = 1; #else @@ -4343,11 +4342,6 @@ int main(int argc, char *argv[]) sshfs.randseed = time(0); - if (sshfs.max_read > 65536) - sshfs.max_read = 65536; - if (sshfs.max_write > 65536) - sshfs.max_write = 65536; - fsname = fsname_escape_commas(fsname); tmp = g_strdup_printf("-osubtype=sshfs,fsname=%s", fsname); fuse_opt_insert_arg(&args, 1, tmp); From 70048b550a9690482976e2e750820b859b5911ad Mon Sep 17 00:00:00 2001 From: Cam Cope Date: Mon, 26 Jul 2021 18:24:36 -0700 Subject: [PATCH 03/14] make readahead tunable --- sshfs.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/sshfs.c b/sshfs.c index 08a9a3fc..615189f0 100644 --- a/sshfs.c +++ b/sshfs.c @@ -330,6 +330,7 @@ struct sshfs { GHashTable *r_uid_map; GHashTable *r_gid_map; unsigned max_read; + unsigned max_readahead; unsigned max_write; unsigned ssh_ver; int sync_write; @@ -474,6 +475,7 @@ static struct fuse_opt sshfs_opts[] = { SSHFS_OPT("directport=%s", directport, 0), SSHFS_OPT("ssh_command=%s", ssh_command, 0), SSHFS_OPT("sftp_server=%s", sftp_server, 0), + SSHFS_OPT("max_readahead=%u", max_readahead, 0), SSHFS_OPT("max_read=%u", max_read, 0), SSHFS_OPT("max_write=%u", max_write, 0), SSHFS_OPT("ssh_protocol=%u", ssh_ver, 0), @@ -1912,6 +1914,9 @@ static void *sshfs_init(struct fuse_conn_info *conn, // SFTP only supports 1-second time resolution conn->time_gran = 1000000000; + if (sshfs.max_readahead > 0) + conn->max_readahead = sshfs.max_readahead; + return NULL; } From cf79d017d5dc9153fc5424747bb56a54a1198de2 Mon Sep 17 00:00:00 2001 From: Cam Cope Date: Thu, 2 Sep 2021 18:37:04 -0700 Subject: [PATCH 04/14] [WIP] Create sync request functions --- sshfs.c | 135 +++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 133 insertions(+), 2 deletions(-) diff --git a/sshfs.c b/sshfs.c index 615189f0..d3e82e07 100644 --- a/sshfs.c +++ b/sshfs.c @@ -597,6 +597,9 @@ static const char *type_name(uint8_t type) } } +static int sftp_request_sync(struct conn *conn, uint8_t type, const struct buffer *buf, + uint8_t expect_type, struct buffer *outbuf); + #define container_of(ptr, type, member) ({ \ const typeof( ((type *)0)->member ) *__mptr = (ptr); \ (type *)( (char *)__mptr - offsetof(type,member) );}) @@ -1462,6 +1465,55 @@ static int clean_req(void *key, struct request *req, gpointer user_data) return TRUE; } +static int sftp_request_process_sync(struct conn *conn, struct request *req, uint8_t type) +{ + int res; + struct buffer buf; + uint32_t id; + + buf_init(&buf, 0); + res = sftp_read(conn, &type, &buf); + if (res == -1) + return -1; + if (buf_get_uint32(&buf, &id) == -1) + return -1; + + if (req != NULL) { + if (sshfs.debug) { + struct timeval now; + unsigned int difftime; + unsigned msgsize = buf.size + 5; + + gettimeofday(&now, NULL); + difftime = (now.tv_sec - req->start.tv_sec) * 1000; + difftime += (now.tv_usec - req->start.tv_usec) / 1000; + DEBUG(" [%05i] %14s %8ubytes (%ims)\n", id, + type_name(type), msgsize, difftime); + + if (difftime < sshfs.min_rtt || !sshfs.num_received) + sshfs.min_rtt = difftime; + if (difftime > sshfs.max_rtt) + sshfs.max_rtt = difftime; + sshfs.total_rtt += difftime; + sshfs.num_received++; + sshfs.bytes_received += msgsize; + } + req->reply = buf; + req->reply_type = type; + req->replied = 1; + if (req->want_reply) + sem_post(&req->ready); + else { + pthread_mutex_lock(&sshfs.lock); + request_free(req); + pthread_mutex_unlock(&sshfs.lock); + } + } else + buf_free(&buf); + + return 0; +} + static int process_one_request(struct conn *conn) { int res; @@ -1937,7 +1989,7 @@ static int sftp_request_wait(struct request *req, uint8_t type, err = -EIO; if (req->reply_type != expect_type && req->reply_type != SSH_FXP_STATUS) { - fprintf(stderr, "protocol error\n"); + fprintf(stderr, "request wait: protocol error\n"); goto out; } if (req->reply_type == SSH_FXP_STATUS) { @@ -1983,6 +2035,15 @@ static int sftp_request_wait(struct request *req, uint8_t type, return err; } +static int sftp_request_wait_sync(struct conn *conn, struct request *req, uint8_t type, + uint8_t expect_type, struct buffer *outbuf) +{ + int err; + if ((err = sftp_request_process_sync(conn, req, type)) != 0) + return err; + return sftp_request_wait(req, type, expect_type, outbuf); +} + static int sftp_request_send(struct conn *conn, uint8_t type, struct iovec *iov, size_t count, request_func begin_func, request_func end_func, int want_reply, void *data, struct request **reqp) @@ -2050,6 +2111,68 @@ static int sftp_request_send(struct conn *conn, uint8_t type, struct iovec *iov, return err; } +static int sftp_request_send_sync(struct conn *conn, uint8_t type, struct iovec *iov, + size_t count, request_func begin_func, request_func end_func, + int want_reply, void *data, struct request **reqp) +{ + int err; + uint32_t id; + struct request *req = g_new0(struct request, 1); + + req->want_reply = want_reply; + req->end_func = end_func; + req->data = data; + sem_init(&req->ready, 0, 0); + buf_init(&req->reply, 0); + if (begin_func) + begin_func(req); + id = sftp_get_id(); + req->id = id; + req->conn = conn; + req->conn->req_count++; + req->len = iov_length(iov, count) + 9; + if (sshfs.debug) { + gettimeofday(&req->start, NULL); + sshfs.num_sent++; + sshfs.bytes_sent += req->len; + } + DEBUG("[%05i] %s\n", id, type_name(type)); + err = -EIO; + if (sftp_send_iov(conn, type, id, iov, count) == -1) { + if (!want_reply) { + /* request already freed */ + return err; + } + goto out; + } + if (want_reply) + *reqp = req; + return 0; + +out: + req->error = err; + if (!want_reply) + sftp_request_wait_sync(conn, req, type, 0, NULL); + else + *reqp = req; + + return err; + +} + +static int sftp_request_iov_sync(struct conn *conn, uint8_t type, struct iovec *iov, + size_t count, uint8_t expect_type, struct buffer *outbuf) +{ + int err; + struct request *req; + + err = sftp_request_send_sync(conn, type, iov, count, NULL, NULL, + expect_type, NULL, &req); + if (expect_type == 0) + return err; + return sftp_request_wait_sync(conn, req, type, expect_type, outbuf); +} + static int sftp_request_iov(struct conn *conn, uint8_t type, struct iovec *iov, size_t count, uint8_t expect_type, struct buffer *outbuf) { @@ -2060,10 +2183,18 @@ static int sftp_request_iov(struct conn *conn, uint8_t type, struct iovec *iov, expect_type, NULL, &req); if (expect_type == 0) return err; - return sftp_request_wait(req, type, expect_type, outbuf); } +static int sftp_request_sync(struct conn *conn, uint8_t type, const struct buffer *buf, + uint8_t expect_type, struct buffer *outbuf) +{ + struct iovec iov; + + buf_to_iov(buf, &iov); + return sftp_request_iov_sync(conn, type, &iov, 1, expect_type, outbuf); +} + static int sftp_request(struct conn *conn, uint8_t type, const struct buffer *buf, uint8_t expect_type, struct buffer *outbuf) { From bb40e5b53d7fc2ca1619b6bdebcca10a0ee861b4 Mon Sep 17 00:00:00 2001 From: Cam Cope Date: Thu, 2 Sep 2021 16:51:06 -0700 Subject: [PATCH 05/14] limit negotiation extension --- sshfs.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/sshfs.c b/sshfs.c index d3e82e07..1b26bf78 100644 --- a/sshfs.c +++ b/sshfs.c @@ -121,6 +121,7 @@ #define SFTP_EXT_STATVFS "statvfs@openssh.com" #define SFTP_EXT_HARDLINK "hardlink@openssh.com" #define SFTP_EXT_FSYNC "fsync@openssh.com" +#define SFTP_EXT_LIMITS "limits@openssh.com" #define PROTO_VERSION 3 @@ -257,6 +258,13 @@ struct request { struct conn *conn; }; +struct sftp_limits { + uint64_t packet_length; + uint64_t read_length; + uint64_t write_length; + uint64_t open_handles; +}; + struct sshfs_io { int num_reqs; pthread_cond_t finished; @@ -1625,6 +1633,32 @@ static void *process_requests(void *data_) return NULL; } +static int sftp_init_limits(struct conn *conn) { + int res; + struct buffer buf, outbuf; + struct sftp_limits limits; + memset(&limits, 0, sizeof(limits)); + buf_init(&buf, 0); + buf_add_string(&buf, SFTP_EXT_LIMITS); + res = sftp_request(conn, SSH_FXP_EXTENDED, &buf, SSH_FXP_EXTENDED_REPLY, &outbuf); + buf_free(&buf); + if (res != 0) { + return res; + } + DEBUG("Received limits reply\n", NULL); + + if ((res = buf_get_uint64(&outbuf, &limits.packet_length)) != 0 || + (res = buf_get_uint64(&outbuf, &limits.read_length)) != 0 || + (res = buf_get_uint64(&outbuf, &limits.write_length)) != 0 || + (res = buf_get_uint64(&outbuf, &limits.open_handles)) != 0) { + buf_free(&outbuf); + return res; + } + DEBUG("Parsed limits reply:\nread: %u write: %u\n", NULL); + return 0; + +} + static int sftp_init_reply_ok(struct conn *conn, struct buffer *buf, uint32_t *version) { From c290b309d45b985df1a452dc7146356e2196bc2c Mon Sep 17 00:00:00 2001 From: Cam Cope Date: Thu, 2 Sep 2021 16:53:22 -0700 Subject: [PATCH 06/14] apply negotiated limits --- sshfs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sshfs.c b/sshfs.c index 1b26bf78..cd7b63e4 100644 --- a/sshfs.c +++ b/sshfs.c @@ -1655,6 +1655,7 @@ static int sftp_init_limits(struct conn *conn) { return res; } DEBUG("Parsed limits reply:\nread: %u write: %u\n", NULL); + apply_sftp_limits(&limits); return 0; } From 77373d8dc1c030760a7fafe6c1ea3ef812c259bf Mon Sep 17 00:00:00 2001 From: Cam Cope Date: Thu, 2 Sep 2021 18:13:01 -0700 Subject: [PATCH 07/14] limit apply funcs --- sshfs.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/sshfs.c b/sshfs.c index cd7b63e4..17d0ae41 100644 --- a/sshfs.c +++ b/sshfs.c @@ -1633,6 +1633,67 @@ static void *process_requests(void *data_) return NULL; } +static void apply_naive_sftp_limits() { + // OpenSSH SFTP server < v8.6p1 limit is 65536 bytes + // TODO: Allow override? + int warn = 0; + if (sshfs.max_read > DEFAULT_MAX_SFTP_MSG_LIMIT) { + sshfs.max_read = DEFAULT_MAX_SFTP_MSG_LIMIT; + warn = 1; + } else if (sshfs.max_read == 0) { + sshfs.max_read = DEFAULT_MIN_SFTP_MSG_LIMIT; + } + + if (sshfs.max_write > DEFAULT_MAX_SFTP_MSG_LIMIT) { + sshfs.max_write = DEFAULT_MAX_SFTP_MSG_LIMIT; + warn = 1; + } else if (sshfs.max_write == 0) { + sshfs.max_write = DEFAULT_MIN_SFTP_MSG_LIMIT; + } + + if (warn) { + fprintf(stderr, "OpenSSH SFTP server read/write limit is 64KB\n"); + } +} + +/* If server has limits feature: + * * if server limit is 0: + * [x] if user limit, set to user limit + * [x] otherwise default to 1MB + * * if server limit > 0: + * [x] if user limit less than server limit, use user limit + * [ ] warn? + * [x] if user limit > server limit, use server limit + * [ ] warn + * [x] else use server limit + * + */ +static void apply_sftp_limits(struct sftp_limits *limits) { + if (limits->read_length == 0) { + if (sshfs.max_read == 0) { + // TODO: Pick a bigger number, maybe dynamically somehow? + sshfs.max_read = 1048576; + } + // else sshfs.max_read already has a value + } else if (limits->read_length > 0) { + if (sshfs.max_read == 0 || sshfs.max_read > limits->read_length) { + sshfs.max_read = limits->read_length; + } + } + + if (limits->write_length == 0) { + if (sshfs.max_write == 0) { + // TODO: Pick a bigger number, maybe dynamically somehow? + sshfs.max_write = DEFAULT_MAX_SFTP_MSG_LIMIT * 8; + } + // else sshfs.max_write already has a value + } else if (limits->write_length > 0) { + if (sshfs.max_write == 0 || sshfs.max_write > limits->write_length) { + sshfs.max_write = limits->write_length; + } + } +} + static int sftp_init_limits(struct conn *conn) { int res; struct buffer buf, outbuf; From 0ec953291630c3e42de54fc2923889609ad7411d Mon Sep 17 00:00:00 2001 From: Cam Cope Date: Thu, 2 Sep 2021 18:13:32 -0700 Subject: [PATCH 08/14] [WIP] Update limit functions, use sync request function --- sshfs.c | 93 ++++++++++++++++++++++++++++++++++----------------------- 1 file changed, 56 insertions(+), 37 deletions(-) diff --git a/sshfs.c b/sshfs.c index 17d0ae41..bd83f2d9 100644 --- a/sshfs.c +++ b/sshfs.c @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #ifndef __APPLE__ @@ -380,6 +381,7 @@ struct sshfs { int ext_statvfs; int ext_hardlink; int ext_fsync; + int ext_limits; struct fuse_operations *op; /* statistics */ @@ -1637,18 +1639,18 @@ static void apply_naive_sftp_limits() { // OpenSSH SFTP server < v8.6p1 limit is 65536 bytes // TODO: Allow override? int warn = 0; - if (sshfs.max_read > DEFAULT_MAX_SFTP_MSG_LIMIT) { - sshfs.max_read = DEFAULT_MAX_SFTP_MSG_LIMIT; + if (sshfs.max_read > (DEFAULT_MAX_SFTP_MSG_LIMIT - 1024)) { + sshfs.max_read = DEFAULT_MAX_SFTP_MSG_LIMIT - 1024; warn = 1; } else if (sshfs.max_read == 0) { - sshfs.max_read = DEFAULT_MIN_SFTP_MSG_LIMIT; + sshfs.max_read = DEFAULT_MIN_SFTP_MSG_LIMIT - 1024; } if (sshfs.max_write > DEFAULT_MAX_SFTP_MSG_LIMIT) { - sshfs.max_write = DEFAULT_MAX_SFTP_MSG_LIMIT; + sshfs.max_write = DEFAULT_MAX_SFTP_MSG_LIMIT - 1024; warn = 1; } else if (sshfs.max_write == 0) { - sshfs.max_write = DEFAULT_MIN_SFTP_MSG_LIMIT; + sshfs.max_write = DEFAULT_MIN_SFTP_MSG_LIMIT - 1024; } if (warn) { @@ -1656,25 +1658,28 @@ static void apply_naive_sftp_limits() { } } -/* If server has limits feature: - * * if server limit is 0: - * [x] if user limit, set to user limit - * [x] otherwise default to 1MB - * * if server limit > 0: - * [x] if user limit less than server limit, use user limit - * [ ] warn? - * [x] if user limit > server limit, use server limit - * [ ] warn - * [x] else use server limit - * - */ static void apply_sftp_limits(struct sftp_limits *limits) { + // sshfs.max_read has a user-set value or a default + if (sshfs.max_read > (limits->packet_length - 1024)) { + fprintf(stderr, + "Read size too large. " + "OpenSSH SFTP server message limit is %" PRIu64 "\n", + (limits->packet_length - 1024)); + abort(); + } + // sshfs.max_read has a user-set value or a default + if (sshfs.max_write > (limits->packet_length - 1024)) { + fprintf(stderr, + "Write size too large. " + "OpenSSH SFTP server message limit is %" PRIu64 "\n", + (limits->packet_length - 1024)); + abort(); + } if (limits->read_length == 0) { if (sshfs.max_read == 0) { - // TODO: Pick a bigger number, maybe dynamically somehow? - sshfs.max_read = 1048576; + sshfs.max_read = MAX(DEFAULT_MAX_SFTP_MSG_LIMIT - 1024, + limits->packet_length - 1024); } - // else sshfs.max_read already has a value } else if (limits->read_length > 0) { if (sshfs.max_read == 0 || sshfs.max_read > limits->read_length) { sshfs.max_read = limits->read_length; @@ -1683,10 +1688,9 @@ static void apply_sftp_limits(struct sftp_limits *limits) { if (limits->write_length == 0) { if (sshfs.max_write == 0) { - // TODO: Pick a bigger number, maybe dynamically somehow? - sshfs.max_write = DEFAULT_MAX_SFTP_MSG_LIMIT * 8; + sshfs.max_read = MAX(DEFAULT_MAX_SFTP_MSG_LIMIT - 1024, + limits->packet_length - 1024); } - // else sshfs.max_write already has a value } else if (limits->write_length > 0) { if (sshfs.max_write == 0 || sshfs.max_write > limits->write_length) { sshfs.max_write = limits->write_length; @@ -1695,30 +1699,30 @@ static void apply_sftp_limits(struct sftp_limits *limits) { } static int sftp_init_limits(struct conn *conn) { - int res; + int err; struct buffer buf, outbuf; struct sftp_limits limits; memset(&limits, 0, sizeof(limits)); buf_init(&buf, 0); buf_add_string(&buf, SFTP_EXT_LIMITS); - res = sftp_request(conn, SSH_FXP_EXTENDED, &buf, SSH_FXP_EXTENDED_REPLY, &outbuf); - buf_free(&buf); - if (res != 0) { - return res; + err = sftp_request_sync(conn, SSH_FXP_EXTENDED, &buf, SSH_FXP_EXTENDED_REPLY, &outbuf); + if (err != 0) { + goto out; } - DEBUG("Received limits reply\n", NULL); - if ((res = buf_get_uint64(&outbuf, &limits.packet_length)) != 0 || - (res = buf_get_uint64(&outbuf, &limits.read_length)) != 0 || - (res = buf_get_uint64(&outbuf, &limits.write_length)) != 0 || - (res = buf_get_uint64(&outbuf, &limits.open_handles)) != 0) { - buf_free(&outbuf); - return res; + if ((err = buf_get_uint64(&outbuf, &limits.packet_length)) != 0 || + (err = buf_get_uint64(&outbuf, &limits.read_length)) != 0 || + (err = buf_get_uint64(&outbuf, &limits.write_length)) != 0 || + (err = buf_get_uint64(&outbuf, &limits.open_handles)) != 0) { + goto out; } - DEBUG("Parsed limits reply:\nread: %u write: %u\n", NULL); apply_sftp_limits(&limits); - return 0; + err = 0; +out: + buf_free(&buf); + buf_free(&outbuf); + return err; } static int sftp_init_reply_ok(struct conn *conn, struct buffer *buf, @@ -1784,6 +1788,11 @@ static int sftp_init_reply_ok(struct conn *conn, struct buffer *buf, strcmp(extdata, "1") == 0) sshfs.ext_fsync = 1; + // Query the server for its limits + if (strcmp(ext, SFTP_EXT_LIMITS) == 0 && + strcmp(extdata, "1") == 0) { + sshfs.ext_limits = 1; + } free(ext); free(extdata); } while (buf2.len < buf2.size); @@ -1839,6 +1848,16 @@ static int sftp_init(struct conn *conn) "Warning: server uses version: %i, we support: %i\n", version, PROTO_VERSION); } + + if (sshfs.ext_limits == 1) { + if (sftp_init_limits(conn) != 0) { + fprintf(stderr, "handling server-side limits failed\n"); + abort(); + } + } else { + apply_naive_sftp_limits(); + } + res = 0; out: From 0baa8487f16eab9b2cb2371d90d844a87a4a6e6a Mon Sep 17 00:00:00 2001 From: Cam Cope Date: Tue, 17 Aug 2021 13:59:27 -0700 Subject: [PATCH 09/14] port detect_uid and check_root to use sftp_request_sync --- sshfs.c | 83 ++++++++++----------------------------------------------- 1 file changed, 14 insertions(+), 69 deletions(-) diff --git a/sshfs.c b/sshfs.c index bd83f2d9..0c4e9a21 100644 --- a/sshfs.c +++ b/sshfs.c @@ -1883,48 +1883,21 @@ static int sftp_error_to_errno(uint32_t error) static void sftp_detect_uid(struct conn *conn) { int flags; - uint32_t id = sftp_get_id(); - uint32_t replid; - uint8_t type; - struct buffer buf; - struct stat stbuf; - struct iovec iov[1]; + struct buffer buf, outbuf; + struct stat st; buf_init(&buf, 5); buf_add_string(&buf, "."); - buf_to_iov(&buf, &iov[0]); - if (sftp_send_iov(conn, SSH_FXP_STAT, id, iov, 1) == -1) + if (sftp_request_sync(conn, SSH_FXP_STAT, &buf, SSH_FXP_ATTRS, &outbuf) == -1) goto out; - buf_clear(&buf); - if (sftp_read(conn, &type, &buf) == -1) + if (buf_get_attrs(&outbuf, &st, &flags) != 0) goto out; - if (type != SSH_FXP_ATTRS && type != SSH_FXP_STATUS) { - fprintf(stderr, "protocol error\n"); - goto out; - } - if (buf_get_uint32(&buf, &replid) == -1) - goto out; - if (replid != id) { - fprintf(stderr, "bad reply ID\n"); - goto out; - } - if (type == SSH_FXP_STATUS) { - uint32_t serr; - if (buf_get_uint32(&buf, &serr) == -1) - goto out; - - fprintf(stderr, "failed to stat home directory (%i)\n", serr); - goto out; - } - if (buf_get_attrs(&buf, &stbuf, &flags) != 0) - goto out; - if (!(flags & SSH_FILEXFER_ATTR_UIDGID)) goto out; - sshfs.remote_uid = stbuf.st_uid; + sshfs.remote_uid = st.st_uid; sshfs.local_uid = getuid(); - sshfs.remote_gid = stbuf.st_gid; + sshfs.remote_gid = st.st_gid; sshfs.local_gid = getgid(); sshfs.remote_uid_detected = 1; DEBUG("remote_uid = %i\n", sshfs.remote_uid); @@ -1934,59 +1907,30 @@ static void sftp_detect_uid(struct conn *conn) fprintf(stderr, "failed to detect remote user ID\n"); buf_free(&buf); + buf_free(&outbuf); } static int sftp_check_root(struct conn *conn, const char *base_path) { int flags; - uint32_t id = sftp_get_id(); - uint32_t replid; - uint8_t type; - struct buffer buf; - struct stat stbuf; - struct iovec iov[1]; int err = -1; + struct buffer buf, outbuf; + struct stat st; const char *remote_dir = base_path[0] ? base_path : "."; buf_init(&buf, 0); buf_add_string(&buf, remote_dir); - buf_to_iov(&buf, &iov[0]); - if (sftp_send_iov(conn, SSH_FXP_LSTAT, id, iov, 1) == -1) - goto out; - buf_clear(&buf); - if (sftp_read(conn, &type, &buf) == -1) - goto out; - if (type != SSH_FXP_ATTRS && type != SSH_FXP_STATUS) { - fprintf(stderr, "protocol error\n"); - goto out; - } - if (buf_get_uint32(&buf, &replid) == -1) - goto out; - if (replid != id) { - fprintf(stderr, "bad reply ID\n"); - goto out; - } - if (type == SSH_FXP_STATUS) { - uint32_t serr; - if (buf_get_uint32(&buf, &serr) == -1) - goto out; - - fprintf(stderr, "%s:%s: %s\n", sshfs.host, remote_dir, - strerror(sftp_error_to_errno(serr))); - + if (sftp_request_sync(conn, SSH_FXP_LSTAT, &buf, SSH_FXP_ATTRS, &outbuf) == -1) goto out; - } - - int err2 = buf_get_attrs(&buf, &stbuf, &flags); - if (err2) { - err = err2; + err = buf_get_attrs(&outbuf, &st, &flags); + if (err) { goto out; } if (!(flags & SSH_FILEXFER_ATTR_PERMISSIONS)) goto out; - if (!S_ISDIR(stbuf.st_mode)) { + if (!S_ISDIR(st.st_mode)) { fprintf(stderr, "%s:%s: Not a directory\n", sshfs.host, remote_dir); goto out; @@ -1996,6 +1940,7 @@ static int sftp_check_root(struct conn *conn, const char *base_path) out: buf_free(&buf); + buf_free(&outbuf); return err; } From f5f23ff62bf259167acedc57c133e84e50dab3d2 Mon Sep 17 00:00:00 2001 From: Cam Cope Date: Thu, 2 Sep 2021 21:26:35 -0700 Subject: [PATCH 10/14] move check root+detect uid --- sshfs.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/sshfs.c b/sshfs.c index 0c4e9a21..ce264579 100644 --- a/sshfs.c +++ b/sshfs.c @@ -1858,6 +1858,15 @@ static int sftp_init(struct conn *conn) apply_naive_sftp_limits(); } + if (sshfs.detect_uid) { + sftp_detect_uid(conn); + sshfs.detect_uid = 0; + } + + if (!sshfs.no_check_root && + (res = sftp_check_root(conn, sshfs.base_path)) != 0) + goto out; + res = 0; out: @@ -1981,11 +1990,6 @@ static int start_processing_thread(struct conn *conn) return -EIO; } - if (sshfs.detect_uid) { - sftp_detect_uid(conn); - sshfs.detect_uid = 0; - } - sigemptyset(&newset); sigaddset(&newset, SIGTERM); sigaddset(&newset, SIGINT); @@ -4162,10 +4166,6 @@ static int ssh_connect(void) if (connect_remote(&sshfs.conns[0]) == -1) return -1; - if (!sshfs.no_check_root && - sftp_check_root(&sshfs.conns[0], sshfs.base_path) != 0) - return -1; - } return 0; } From 66c07b0b001da87425805f29b6a74e4eebe3e2b2 Mon Sep 17 00:00:00 2001 From: Cam Cope Date: Thu, 2 Sep 2021 21:26:46 -0700 Subject: [PATCH 11/14] maxread comment --- sshfs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sshfs.c b/sshfs.c index ce264579..0d68d10e 100644 --- a/sshfs.c +++ b/sshfs.c @@ -2030,6 +2030,7 @@ static void *sshfs_init(struct fuse_conn_info *conn, // SFTP only supports 1-second time resolution conn->time_gran = 1000000000; + // TODO: Should this be based upon max_read? if (sshfs.max_readahead > 0) conn->max_readahead = sshfs.max_readahead; From 3a37678a0f8dd3f123bf834f8c6bc42f49386f41 Mon Sep 17 00:00:00 2001 From: Cam Cope Date: Tue, 17 Aug 2021 23:53:14 -0700 Subject: [PATCH 12/14] share more logic between request_send + request_send_sync --- sshfs.c | 44 +++++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/sshfs.c b/sshfs.c index 0d68d10e..4291c01b 100644 --- a/sshfs.c +++ b/sshfs.c @@ -2109,11 +2109,10 @@ static int sftp_request_wait_sync(struct conn *conn, struct request *req, uint8_ return sftp_request_wait(req, type, expect_type, outbuf); } -static int sftp_request_send(struct conn *conn, uint8_t type, struct iovec *iov, - size_t count, request_func begin_func, request_func end_func, - int want_reply, void *data, struct request **reqp) +static struct request *sftp_request_build(struct conn *conn, struct iovec *iov, size_t count, + request_func begin_func, request_func end_func, int want_reply, + void *data) { - int err; uint32_t id; struct request *req = g_new0(struct request, 1); @@ -2122,19 +2121,33 @@ static int sftp_request_send(struct conn *conn, uint8_t type, struct iovec *iov, req->data = data; sem_init(&req->ready, 0, 0); buf_init(&req->reply, 0); - pthread_mutex_lock(&sshfs.lock); if (begin_func) begin_func(req); id = sftp_get_id(); req->id = id; req->conn = conn; req->conn->req_count++; + req->len = iov_length(iov, count) + 9; + return req; +} + +static int sftp_request_send(struct conn *conn, uint8_t type, struct iovec *iov, + size_t count, request_func begin_func, request_func end_func, + int want_reply, void *data, struct request **reqp) +{ + int err; + struct request *req = sftp_request_build(conn, iov, count, begin_func, end_func, + want_reply, data); + uint32_t id = req->id; + + // in the case of delay_connect, the ssh connection isn't established until + // the first filesystem interaction err = start_processing_thread(conn); if (err) { - pthread_mutex_unlock(&sshfs.lock); goto out; } - req->len = iov_length(iov, count) + 9; + + pthread_mutex_lock(&sshfs.lock); sshfs.outstanding_len += req->len; while (sshfs.outstanding_len > sshfs.max_outstanding_len) pthread_cond_wait(&sshfs.outstanding_cond, &sshfs.lock); @@ -2181,21 +2194,10 @@ static int sftp_request_send_sync(struct conn *conn, uint8_t type, struct iovec int want_reply, void *data, struct request **reqp) { int err; - uint32_t id; - struct request *req = g_new0(struct request, 1); + struct request *req = sftp_request_build(conn, iov, count, begin_func, end_func, + want_reply, data); + uint32_t id = req->id; - req->want_reply = want_reply; - req->end_func = end_func; - req->data = data; - sem_init(&req->ready, 0, 0); - buf_init(&req->reply, 0); - if (begin_func) - begin_func(req); - id = sftp_get_id(); - req->id = id; - req->conn = conn; - req->conn->req_count++; - req->len = iov_length(iov, count) + 9; if (sshfs.debug) { gettimeofday(&req->start, NULL); sshfs.num_sent++; From fe13ae6369bffe3388e2884cc2613bbc9fb645be Mon Sep 17 00:00:00 2001 From: Cam Cope Date: Thu, 30 Sep 2021 18:29:48 -0700 Subject: [PATCH 13/14] temporary forward declarations --- sshfs.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sshfs.c b/sshfs.c index 4291c01b..5b961849 100644 --- a/sshfs.c +++ b/sshfs.c @@ -1827,6 +1827,8 @@ static int sftp_find_init_reply(struct conn *conn, uint32_t *version) return res; } +static int sftp_check_root(struct conn *conn, const char *base_path); +static void sftp_detect_uid(struct conn *conn); static int sftp_init(struct conn *conn) { int res = -1; From 9e5f99a6ff6f6e78870567b0b6025c9ae62fa153 Mon Sep 17 00:00:00 2001 From: Cam Cope Date: Tue, 5 Oct 2021 15:38:28 -0700 Subject: [PATCH 14/14] name constant, fix reply length check --- sshfs.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/sshfs.c b/sshfs.c index 5b961849..dea10c4d 100644 --- a/sshfs.c +++ b/sshfs.c @@ -139,6 +139,7 @@ #define MAX_PASSWORD 1024 #define DEFAULT_MAX_SFTP_MSG_LIMIT 65536 #define DEFAULT_MIN_SFTP_MSG_LIMIT 32768 +#define SFTP_PACKET_HEADER_PADDING 1024 /* Handling of multiple SFTP connections @@ -1401,7 +1402,7 @@ static int sftp_read(struct conn *conn, uint8_t *type, struct buffer *buf) if (res != -1) { if ((res = buf_get_uint32(&buf2, &len)) == -1) goto out; - if (len > sshfs.max_read) { + if (len > (sshfs.max_read + SFTP_PACKET_HEADER_PADDING)) { fprintf(stderr, "reply len too large: %u\n", len); res = -1; goto out; @@ -1639,18 +1640,18 @@ static void apply_naive_sftp_limits() { // OpenSSH SFTP server < v8.6p1 limit is 65536 bytes // TODO: Allow override? int warn = 0; - if (sshfs.max_read > (DEFAULT_MAX_SFTP_MSG_LIMIT - 1024)) { - sshfs.max_read = DEFAULT_MAX_SFTP_MSG_LIMIT - 1024; + if (sshfs.max_read > (DEFAULT_MAX_SFTP_MSG_LIMIT - SFTP_PACKET_HEADER_PADDING)) { + sshfs.max_read = DEFAULT_MAX_SFTP_MSG_LIMIT - SFTP_PACKET_HEADER_PADDING; warn = 1; } else if (sshfs.max_read == 0) { - sshfs.max_read = DEFAULT_MIN_SFTP_MSG_LIMIT - 1024; + sshfs.max_read = DEFAULT_MIN_SFTP_MSG_LIMIT - SFTP_PACKET_HEADER_PADDING; } if (sshfs.max_write > DEFAULT_MAX_SFTP_MSG_LIMIT) { - sshfs.max_write = DEFAULT_MAX_SFTP_MSG_LIMIT - 1024; + sshfs.max_write = DEFAULT_MAX_SFTP_MSG_LIMIT - SFTP_PACKET_HEADER_PADDING; warn = 1; } else if (sshfs.max_write == 0) { - sshfs.max_write = DEFAULT_MIN_SFTP_MSG_LIMIT - 1024; + sshfs.max_write = DEFAULT_MIN_SFTP_MSG_LIMIT - SFTP_PACKET_HEADER_PADDING; } if (warn) { @@ -1660,25 +1661,25 @@ static void apply_naive_sftp_limits() { static void apply_sftp_limits(struct sftp_limits *limits) { // sshfs.max_read has a user-set value or a default - if (sshfs.max_read > (limits->packet_length - 1024)) { + if (sshfs.max_read > (limits->packet_length - SFTP_PACKET_HEADER_PADDING)) { fprintf(stderr, "Read size too large. " "OpenSSH SFTP server message limit is %" PRIu64 "\n", - (limits->packet_length - 1024)); + (limits->packet_length - SFTP_PACKET_HEADER_PADDING)); abort(); } // sshfs.max_read has a user-set value or a default - if (sshfs.max_write > (limits->packet_length - 1024)) { + if (sshfs.max_write > (limits->packet_length - SFTP_PACKET_HEADER_PADDING)) { fprintf(stderr, "Write size too large. " "OpenSSH SFTP server message limit is %" PRIu64 "\n", - (limits->packet_length - 1024)); + (limits->packet_length - SFTP_PACKET_HEADER_PADDING)); abort(); } if (limits->read_length == 0) { if (sshfs.max_read == 0) { - sshfs.max_read = MAX(DEFAULT_MAX_SFTP_MSG_LIMIT - 1024, - limits->packet_length - 1024); + sshfs.max_read = MAX(DEFAULT_MAX_SFTP_MSG_LIMIT - SFTP_PACKET_HEADER_PADDING, + limits->packet_length - SFTP_PACKET_HEADER_PADDING); } } else if (limits->read_length > 0) { if (sshfs.max_read == 0 || sshfs.max_read > limits->read_length) { @@ -1688,8 +1689,8 @@ static void apply_sftp_limits(struct sftp_limits *limits) { if (limits->write_length == 0) { if (sshfs.max_write == 0) { - sshfs.max_read = MAX(DEFAULT_MAX_SFTP_MSG_LIMIT - 1024, - limits->packet_length - 1024); + sshfs.max_read = MAX(DEFAULT_MAX_SFTP_MSG_LIMIT - SFTP_PACKET_HEADER_PADDING, + limits->packet_length - SFTP_PACKET_HEADER_PADDING); } } else if (limits->write_length > 0) { if (sshfs.max_write == 0 || sshfs.max_write > limits->write_length) {