Re: [PATCH] Add request_denied and unimplemented_packet callbacks on server side

[Aris Adamantiadis <aris@xxxxxxxxxxxx>]

Hi Andreas, Nicolas

Let me some more time to review this patch.

Aris

On 2/03/16 13:21, Andreas Schneider wrote:
> On Saturday 27 February 2016 02:41:34 Nicolas Viennot wrote:
>> When dealing with keepalive packets sent from the server, the clients
>> reply. Having a callback on that reply allows measuring client latency.
>> However, clients behave differently:
>> * libssh sends an unimplemented packet reply to keepalive requests.
>> * other clients like openssh sends a request denied packet to keepalive
>> requests.
>> These callbacks allow the server to measure the latency to the clients when
>> sending keepalive requests.
>
> Looks fine for me, Aris?
>
>
>> ---
>>  include/libssh/callbacks.h | 25 +++++++++++++++++++++++++
>>  src/channels.c             |  5 +++++
>>  src/packet.c               |  8 ++++++--
>>  3 files changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/libssh/callbacks.h b/include/libssh/callbacks.h
>> index 20c540e..22f8b08 100644
>> --- a/include/libssh/callbacks.h
>> +++ b/include/libssh/callbacks.h
>> @@ -296,6 +296,21 @@ typedef int (*ssh_gssapi_accept_sec_ctx_callback)
>> (ssh_session session,
>>  typedef int (*ssh_gssapi_verify_mic_callback) (ssh_session session,
>>   ssh_string mic, void *mic_buffer, size_t mic_buffer_size, void *userdata);
>>
>> +/**
>> + * @brief Handle a notification from the client that it received an
>> unimplemented packet.
>> + * @param session current session handler
>> + * @param seq packet sequence number
>> + * @param userdata Userdata to be passed to the callback function.
>> + */
>> +typedef void (*ssh_client_unimplemented_packet_callback) (ssh_session
>> session, uint32_t seq, void *userdata);
>> +
>> +/**
>> + * @brief Handle an unhandled client request denied callback.
>> + * @param session current session handler
>> + * @param userdata Userdata to be passed to the callback function.
>> + */
>> +typedef void (*ssh_client_request_denied_callback) (ssh_session
>> session, void *userdata);
>> +
>>
>>  /**
>>   * This structure can be used to implement a libssh server, with
>> appropriate callbacks.
>> @@ -345,6 +360,16 @@ struct ssh_server_callbacks_struct {
>>    /* This function will be called when a MIC needs to be verified.
>>     */
>>    ssh_gssapi_verify_mic_callback gssapi_verify_mic_function;
>> +
>> +  /** This function will be called when the client notifies the server of
>> an +   * unimplemented packet.
>> +   */
>> +  ssh_client_unimplemented_packet_callback
>> client_unimplemented_packet_function;
>> +
>> +  /** This function will be called when the client notifies the server of
>> an +   * non handled deny of request.
>> +   */
>> +  ssh_client_request_denied_callback client_request_denied_function;
>>  };
>>  typedef struct ssh_server_callbacks_struct *ssh_server_callbacks;
>>
>> diff --git a/src/channels.c b/src/channels.c
>> index ffcb1d5..6a179f9 100644
>> --- a/src/channels.c
>> +++ b/src/channels.c
>> @@ -2039,6 +2039,11 @@ SSH_PACKET_CALLBACK(ssh_request_denied){
>>    if(session->global_req_state != SSH_CHANNEL_REQ_STATE_PENDING){
>>      SSH_LOG(SSH_LOG_RARE, "SSH_REQUEST_DENIED received in incorrect state
>> %d", session->global_req_state);
>> +
>> +    if (ssh_callbacks_exists(session->server_callbacks,
>> client_request_denied_function)){
>> +      session->server_callbacks->client_request_denied_function(
>> +        session, session->server_callbacks->userdata);
>> +    }
>>    } else {
>>      session->global_req_state=SSH_CHANNEL_REQ_STATE_DENIED;
>>    }
>> diff --git a/src/packet.c b/src/packet.c
>> index 7d6957c..7b98811 100644
>> --- a/src/packet.c
>> +++ b/src/packet.c
>> @@ -464,8 +464,7 @@ SSH_PACKET_CALLBACK(ssh_packet_unimplemented){
>>      uint32_t seq;
>>      int rc;
>>
>> -    (void)session; /* unused */
>> -    (void)type;
>> +    (void)type; /* unused */
>>      (void)user;
>>
>>      rc = ssh_buffer_unpack(packet, "d", &seq);
>> @@ -477,6 +476,11 @@ SSH_PACKET_CALLBACK(ssh_packet_unimplemented){
>>      SSH_LOG(SSH_LOG_RARE,
>>              "Received SSH_MSG_UNIMPLEMENTED (sequence number %d)",seq);
>>
>> +    if (ssh_callbacks_exists(session->server_callbacks,
>> client_unimplemented_packet_function)){
>> +      session->server_callbacks->client_unimplemented_packet_function(
>> +        session, seq, session->server_callbacks->userdata);
>> +    }
>> +
>>      return SSH_PACKET_USED;
>>  }