Re: [PATCH] sftpserver: Support some openssh extensions

[Jakub Jelen <jjelen@xxxxxxxxxx>]

On Tue, 2018-08-28 at 08:03 -0400, Chris Townsend wrote:
> Hi Jakub,
> 
> This only allows the libssh sftp server to receive the extended
> messages
> from a client.  The libssh sftp server does not implement any of the
> actual command handling on a host and is left up to the developer
> using
> the API on how to do that.

Hello Chris,
thank you for clarification! That was what I missed.

I am wondering if the content of the extension or the list of supported
extension should not be handled by the developer using the API.

The proposed way we announce something, which in the end might not be
implemented at all.

But for short term, the fix looks good.

Thanks,
Jakub

> Regards,
> Chris
> 
> On 08/28/2018 03:56 AM, Jakub Jelen wrote:
> > Does this actually implement the the extensions or just sends and
> > receives the extended messages?
> > 
> > I don't know a lot about SFTP, but there is nothing referencing the
> > rename nor to hardlink in the attached patch.
> > 
> > Jakub
> > 
> > 
> > On Mon, 2018-08-27 at 10:45 -0500, Alberto Aguirre wrote:
> > > From: Chris Townsend <christopher.townsend@xxxxxxxxxxxxx>
> > > 
> > > Add support for "hardlink@xxxxxxxxxxx" and
> > > "posix-rename@xxxxxxxxxxx" extensions.
> > > 
> > > Signed-off-by: Alberto Aguirre <albaguirre@xxxxxxxxx>
> > > ---
> > >  include/libssh/sftp.h |  3 +++
> > >  src/sftp.c            |  7 ++++++-
> > >  src/sftpserver.c      | 28 ++++++++++++++++++++++++++++
> > >  3 files changed, 37 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/include/libssh/sftp.h b/include/libssh/sftp.h
> > > index b07f269f..aac7af2b 100644
> > > --- a/include/libssh/sftp.h
> > > +++ b/include/libssh/sftp.h
> > > @@ -123,6 +123,7 @@ struct sftp_client_message_struct {
> > >      sftp_session sftp;
> > >      uint8_t type;
> > >      uint32_t id;
> > > +    char *submessage; /* for extended messages */
> > >      char *filename; /* can be "path" */
> > >      uint32_t flags;
> > >      sftp_attributes attr;
> > > @@ -862,6 +863,7 @@ LIBSSH_API const char
> > > *sftp_client_message_get_filename(sftp_client_message msg)
> > >  LIBSSH_API void
> > > sftp_client_message_set_filename(sftp_client_message
> > > msg, const char *newname);
> > >  LIBSSH_API const char
> > > *sftp_client_message_get_data(sftp_client_message msg);
> > >  LIBSSH_API uint32_t
> > > sftp_client_message_get_flags(sftp_client_message msg);
> > > +LIBSSH_API const char
> > > *sftp_client_message_get_submessage(sftp_client_message msg);
> > >  LIBSSH_API int sftp_send_client_message(sftp_session sftp,
> > > sftp_client_message msg);
> > >  LIBSSH_API int sftp_reply_name(sftp_client_message msg, const
> > > char
> > > *name,
> > >      sftp_attributes attr);
> > > @@ -1011,6 +1013,7 @@ LIBSSH_API void
> > > sftp_handle_remove(sftp_session
> > > sftp, void *handle);
> > >  #define SFTP_RENAME SSH_FXP_RENAME
> > >  #define SFTP_READLINK SSH_FXP_READLINK
> > >  #define SFTP_SYMLINK SSH_FXP_SYMLINK
> > > +#define SFTP_EXTENDED SSH_FXP_EXTENDED
> > >  
> > >  /* openssh flags */
> > >  #define SSH_FXE_STATVFS_ST_RDONLY 0x1 /* read-only */
> > > diff --git a/src/sftp.c b/src/sftp.c
> > > index 87b6ff94..82b71578 100644
> > > --- a/src/sftp.c
> > > +++ b/src/sftp.c
> > > @@ -219,7 +219,12 @@ int sftp_server_init(sftp_session sftp){
> > >      return -1;
> > >    }
> > >  
> > > -  if (ssh_buffer_add_u32(reply, ntohl(LIBSFTP_VERSION)) < 0) {
> > > +  if (ssh_buffer_pack(reply, "dssss",
> > > +                      ntohl(LIBSFTP_VERSION),
> > > +                      "posix-rename@xxxxxxxxxxx",
> > > +                      "1",
> > > +                      "hardlink@xxxxxxxxxxx",
> > > +                      "1") < 0) {
> > >      ssh_set_error_oom(session);
> > >      ssh_buffer_free(reply);
> > >      return -1;
> > > diff --git a/src/sftpserver.c b/src/sftpserver.c
> > > index 68fdb3d2..3cb30090 100644
> > > --- a/src/sftpserver.c
> > > +++ b/src/sftpserver.c
> > > @@ -202,6 +202,29 @@ sftp_client_message
> > > sftp_get_client_message(sftp_session sftp) {
> > >          return NULL;
> > >        }
> > >        break;
> > > +    case SSH_FXP_EXTENDED:
> > > +      rc = ssh_buffer_unpack(payload,
> > > +                             "s",
> > > +                             &msg->submessage);
> > > +      if (rc != SSH_OK) {
> > > +        ssh_set_error_oom(session);
> > > +        sftp_client_message_free(msg);
> > > +        return NULL;
> > > +      }
> > > +
> > > +      if (strcmp(msg->submessage, "hardlink@xxxxxxxxxxx") == 0
> > > ||
> > > +          strcmp(msg->submessage, "posix-rename@xxxxxxxxxxx") ==
> > > 0)
> > > {
> > > +        rc = ssh_buffer_unpack(payload,
> > > +                               "sS",
> > > +                               &msg->filename,
> > > +                               &msg->data);
> > > +        if (rc != SSH_OK) {
> > > +          ssh_set_error_oom(session);
> > > +          sftp_client_message_free(msg);
> > > +          return NULL;
> > > +        }
> > > +      }
> > > +      break;
> > >      default:
> > >        ssh_set_error(sftp->session, SSH_FATAL,
> > >                      "Received unhandled sftp message %d", msg-
> > > > type);
> > > 
> > > @@ -242,12 +265,17 @@ uint32_t
> > > sftp_client_message_get_flags(sftp_client_message msg){
> > >  	return msg->flags;
> > >  }
> > >  
> > > +const char
> > > *sftp_client_message_get_submessage(sftp_client_message
> > > msg){
> > > +        return msg->submessage;
> > > +}
> > > +
> > >  void sftp_client_message_free(sftp_client_message msg) {
> > >    if (msg == NULL) {
> > >      return;
> > >    }
> > >  
> > >    SAFE_FREE(msg->filename);
> > > +  SAFE_FREE(msg->submessage);
> > >    ssh_string_free(msg->data);
> > >    ssh_string_free(msg->handle);
> > >    sftp_attributes_free(msg->attr);
> 
> 
-- 
Jakub Jelen
Software Engineer
Security Technologies
Red Hat, Inc.