pkg / sftp

SFTP support for the go.crypto/ssh package
BSD 2-Clause "Simplified" License
1.49k stars 379 forks source link

Request attributes are not being propagated for SSH_FXP_OPEN operation #557

Open pavan-aeturi opened 10 months ago

pavan-aeturi commented 10 months ago

Hi Team, I'm facing the below issue where I'm looking for ways to block interrupted uploads (similar issue). So I want to access request attributes such as file-size during a put command. Even though the client sends the attributes during a put command, sftp-go server library doesn't seem to propagate it to handler

The restriction might be intentional and have a good reason behind it, in which case may I know the details of the reasoning? The library specification document too, doesn't talk about having this restriction. Please correct me if I'm wrong. Thanks in the advance.

Screenshot 2023-09-08 at 2 03 28 PM

`// New Request initialized based on packet data func requestFromPacket(ctx context.Context, pkt hasPath, baseDir string) *Request { request := &Request{ Method: requestMethod(pkt), Filepath: cleanPathWithBase(baseDir, pkt.getPath()), } request.ctx, request.cancelCtx = context.WithCancel(ctx)

switch p := pkt.(type) {
case *sshFxpOpenPacket:
    request.Flags = p.Pflags
case *sshFxpSetstatPacket:
    request.Flags = p.Flags
    request.Attrs = p.Attrs.([]byte)
case *sshFxpRenamePacket:
    request.Target = cleanPathWithBase(baseDir, p.Newpath)
case *sshFxpSymlinkPacket:
    // NOTE: given a POSIX compliant signature: symlink(target, linkpath string)
    // this makes Request.Target the linkpath, and Request.Filepath the target.
    request.Target = cleanPathWithBase(baseDir, p.Linkpath)
    request.Filepath = p.Targetpath
case *sshFxpExtendedPacketHardlink:
    request.Target = cleanPathWithBase(baseDir, p.Newpath)
}
return request

} `

puellanivis commented 10 months ago

There are unfortunately known issues with the opening of files and handling of attributes. I’ve done a complete rewrite of the client before, but due to early design decisions some of these just aren’t possible to address.

Though… in this case, :squint: it might be possible to bodge in request.Attrs if we make sure the sshFxpOpenPacket contains them at least.

drakkan commented 10 months ago

If a client sends size 0 because it does not support this optional attribute it will be difficult to distinguish it from clients that support the attribute if they upload a 0 byte file

puellanivis commented 10 months ago

Wouldn’t a client that does not support the optional attribute just not even set SSH_FILEXFER_ATTR_SIZE and thus be distinct from those that do support the attribute?

pavan-aeturi commented 10 months ago

yes @puellanivis, that makes sense, SSH_FILEXFER_ATTR_SIZE check should help figure out if it is actually set and something like the below should help resolve file-size on server side

// r *sftp.Request
if r.AttrFlags().Size && r.Attributes() != nil && r.Attributes().Size > 0 {
  h.logger.Info("File size of upload file_length: %d", r.Attributes().Size)
}
mafredri commented 7 months ago

I ended up implementing attrs for the old Server in #567, should be possible to use in the request server as well, I think. There are probably edge cases I haven't considered, though.