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

docs: add a preliminary protocol description #162

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lynxis
Copy link
Contributor

@lynxis lynxis commented Mar 18, 2021

Describe the protocol using packetdiag.

@@ -25,7 +25,7 @@

# Add any Sphinx extension module names here, as strings. They can be extensions
# coming with Sphinx (named 'sphinx.ext.*') or your custom ones.
extensions = ['sphinx.ext.intersphinx']
extensions = ['sphinx.ext.intersphinx', 'sphinxcontrib.packetdiag']
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this change do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, that looks neat. :)

Tunneldigger is only supporting the T bit in the first bit to
mark a packet as control data. All other fields of RFC3931
are ignore and have a different meaning..
All fields are encoding as network byte order.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
All fields are encoding as network byte order.
All fields are encoded with network byte order.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ack


* The T bit must be 1
* Version must be 1
* Type of the PDU
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's a "PDU"?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tunneldigger calls these CONTROL_TYPE or msg_type; that might be more suited terminology.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I don't think we use this term anywhere in tunneldigger though?

24-31: Version
32-39: Type
40-47: Length
48-63: Value
Copy link
Member

@RalfJung RalfJung Mar 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The length of Value is given by Length; why do you write 48-63 here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it's a packet diagraph. I would like to have a value in there. It's only an example.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that it is not at all clear this is just an example. And that makes it very confusing IMO.

cd tunneldigger/docs/
wireshark -Xlua_script:wireshark-tunneldigger.lua

Wireshark might decode the user data as a different protocol (e.g. Cisco HDLC). This can be changed by:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth also explaining how to change the port; tunneldiger is often run on a port different than 8942.

Describe the protocol using packetdiag.
Add tcpdump pcap filter examples and describe
how to use the wireshark dissector.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants