search

pkg / sftp

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

wrong assignment of linkpath and targetpath for Symlink #524

Closed georgmu closed 1 year ago

georgmu commented 1 year ago

For symlink operations, linkpath and targetpath are in the wrong order

The official spec https://datatracker.ietf.org/doc/html/draft-ietf-secsh-filexfer-02#section-6.10 says

   The SSH_FXP_SYMLINK request will create a symbolic link on the
   server.  It is of the following format

        uint32     id
        string     linkpath
        string     targetpath

but in packet.go, sshFxpSymlinkPacket marshal/unmarshal switches the order of linkpath and targetpath.

puellanivis commented 1 year ago

This is known, and while it is technically a bug, everyone has to implement this bug for compatibility reasons: https://github.com/openssh/openssh-portable/blob/master/PROTOCOL#L381-L392

Eventually the SSH_FXP_SYMLINK packet was deprecated and removed from later versions, but since the version we’re implementing is also the presumed default version for max compat, literally everyone has to deal with this. 🙃

I will however put out a PR that adds this information in the comments for this packet in the main package, and corrects the section number reference in the internal/encoding/ssh/filexfer package.

puellanivis commented 1 year ago

Oh, also closing as this is a will not fix.

georgmu commented 1 year ago

ok, sorry for that. But there is bigger problem with cleanPathWithBase of the link target. I will open a separate issue for that.

puellanivis commented 1 year ago

Don’t be sorry, I found it myself, and tried to fix it, but it broke things, and so I had to figure out why. That’s why I added the comment in the SymlinkPacket so I wouldn’t forget again.