Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Report unexpected trailing bytes at the end of INIT and OPEN SYN #1807

Open
jcos1 opened this issue Feb 28, 2025 · 4 comments
Open

Report unexpected trailing bytes at the end of INIT and OPEN SYN #1807

jcos1 opened this issue Feb 28, 2025 · 4 comments
Labels
bug Something isn't working

Comments

@jcos1
Copy link

jcos1 commented Feb 28, 2025

Describe the bug

What happens:
When there is a stray byte at the end of the OPEN SYN
Zenohd will reply with an OPEN ACK
It will not report anything about the trailing byte.
When sending other messages (KEEP_ALIVE [1,0,4] or PUT-FRAME) it will not process these messages.
Zenohd will also not report anything about those messages.
It will not recover (discarding that trailing byte).
It will still send KEEP_ALIVE messages.
After a while zenohd will time out.

What I expect to happen:
I think zenohd should report any unexpected bytes.
Perhaps it should also try to recover.

To reproduce

  1. Start Zenohd
  2. Do the handshake but add a trailing byte at the end of the OPEN SYN

For example, OPEN SYN:
[38,00,66,255,0,1 + COOKIE + 00]

zenohd config changes:

  "id": "7",
...
 "connect_scouted": null,
...
 "batching": {
       "enabled": false,
...
"sequence_number_resolution": "8bit",
...
"shared_memory": {
  "enabled": false
...
 "qos": {
     "enabled": false

To reproduce:

use tokio::time::Duration;
use tokio::net::TcpStream;
use tokio::io::{AsyncReadExt, AsyncWriteExt};
use tokio::time::sleep;

#[tokio::main]
async fn main() -> tokio::io::Result<()> {

    let mut buffer: [u8; 1024] = [0; 1024];  

    let mut stream = TcpStream::connect("127.0.0.1:7447").await?;
    println!("Connection ok");
    
    let z_init = [4,0, 1_u8,9,2,1];     
    println!("Sending INIT SYN {:x?}", z_init);
    stream.write_all(&z_init).await?;
    stream.flush().await?;
    println!("Waiting for INIT ACK");
    
    // INIT ACK        
    let bytes_read = stream.read(&mut buffer).await?;    
    println!("Received INIT ACK: {:x?}", &buffer[..bytes_read]);
    // Store the cookie received with the INIT ACK 
    let mut z_cookie = buffer[9..43].to_vec(); 
    println!("Cookie: {:x?}", z_cookie);

    // OPEN SYN
    let mut z_open = vec![38_u8,0, 66, 255, 0,1];         
    z_open.append(&mut z_cookie);  
    let mut bad_byte = vec![00]; // This shouldn't be here
    z_open.append(&mut bad_byte);  
    println!("Sending OPEN SYN {:x?}", z_open);      
    stream.write_all(&z_open).await?;    
    stream.flush().await?;

    println!("Waiting for OPEN ACK");
    let bytes_read = stream.read(&mut buffer).await?;    
    println!("OPEN ACK: {:x?}", &buffer[..bytes_read]);

    // KEEP ALIVE
    let z_keep_alive = [1_u8,0,4];
    println!("Sending KEEP_ALIVE{:x?}", z_keep_alive);
    stream.write_all(&z_keep_alive).await?;
    stream.flush().await?;
    let bytes_read = stream.read(&mut buffer).await?;    
    println!("KEEP_ALIVE ACK: {:x?}", &buffer[..bytes_read]);

    let z_put = [9u8,0,5,15,61,0,1,65,1,1,65]; 
    println!("Sending FRAME: {:x?}", z_put);
    stream.write_all(&z_put).await?;
    stream.flush().await?;
    let bytes_read = stream.read(&mut buffer).await?;    
    println!("PUT reply: {:x?}", &buffer[..bytes_read]); // it replies with a keep alive
    
    sleep(Duration::from_millis(2000)).await;

    // KEEP ALIVE
    let z_keep_alive = [1_u8,0,4];
    println!("Sending KEEP_ALIVE{:x?}", z_keep_alive);
    stream.write_all(&z_keep_alive).await?;
    stream.flush().await?;
    let bytes_read = stream.read(&mut buffer).await?;    
    println!("KEEP_ALIVE ACK: {:x?}", &buffer[..bytes_read]);

    Ok(())
}

System info

  • Zenohd v1.2.0 on windows
  • The actual client sending the stray byte is a PLC, but a code snippet with the same result is shown above.
@jcos1 jcos1 added the bug Something isn't working label Feb 28, 2025
@Mallets
Copy link
Member

Mallets commented Mar 5, 2025

I don't think this is a bug... the protocol doesn't allow for trailing bytes nor bytes misalignment.
When the protocol is not respected than undefined behaviour is to be expected.

Keeping in mind this kind of behaviour affects only the connection being established because the PLC is not respecting the protocol.

Is this happening with zenoh-pico or with a custom implementation of the protocol?

@jcos1
Copy link
Author

jcos1 commented Mar 5, 2025

This is happening with a custom implementation of the protocol.

Perhaps it can be marked as a feature request?

It would help during debugging and troubleshooting.

@kydos
Copy link
Member

kydos commented Mar 7, 2025

I don't think this is a bug... the protocol doesn't allow for trailing bytes nor bytes misalignment. When the protocol is not
respected than undefined behaviour is to be expected.

Trailing bytes will be interpreted as the beginning of the next message, and with highly likelihood that will be considered as either an unknown or ill-formed message and the session will be closed. If the trailing bytes end-up representing a valid Zenoh message, then those will be interpreted in the context of the current state of the protocol. Thus I am not sure it is accurate to talk about undefined behaviour.

@jcos1
Copy link
Author

jcos1 commented Mar 7, 2025

That's correct.
It keeps reading bytes until the session times out because zenohd can't figure out what to do with the bytes it is receiving.

A nice feature would be to make zenohd print out any unprocessed (=received but not interpreted) bytes (when RUST_LOG is DEBUG or TRACE)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants