libtelnet icon indicating copy to clipboard operation
libtelnet copied to clipboard

Add support for TELNET EXTENDED-OPTIONS-LIST (RFC861)

Open okofish opened this issue 5 years ago • 7 comments

This adds transparent support for negotiation and subnegotiation of TELNET options on the Extended Options List - see RFC861. This standard extends the maximum possible options number from 255 to 511 by wrapping negotiation/subnegotiation commands for these options inside a subnegotiation command for option 255 (EXOPL). No options utilizing this feature were ever defined, but it is nonetheless an Internet standard, and I happen to have an application that requires it.

The only significant user-facing change is the addition of the short telopt_extended member to event structs negotiate_t and subnegotiate_t. I didn't want to change the type of the existing unsigned char telopt member as this could break existing user code, so I added the new member. Going forward, though, there should be no need for anyone to use telopt - I added a note to that effect in the readme.

A few more notes:

  • I don't really get PROXY mode so I didn't particularly touch any code involving it - feel free to point out any spots that need new conditions or anything like that.
  • I changed the types of several arguments regarding options from unsigned char to short, but I didn't add any checks that the supplied option number is 511 or less. I suppose these are needed in telnet_negotiate, telnet_begin_sb, and telnet_subnegotiation - anywhere else? Should they just call _error and return?
  • I tried to prevent VS Code from having its way with the code formatting but I may have missed a spot or two - sorry. Feel free to correct it if so.

okofish avatar Apr 13 '19 07:04 okofish

I did not completed with a review, but one point I don't like is the introduction of another usage of "short" - it has a "slightly antiquated flavor". Why not just use ''int''?

mhei avatar Apr 13 '19 08:04 mhei

I did not completed with a review, but one point I don't like is the introduction of another usage of "short" - it has a "slightly antiquated flavor". Why not just use ''int''?

Changed. I was sort of imitating the telnet_telopt_t struct in that regard, but it really doesn't matter.

okofish avatar Apr 13 '19 14:04 okofish

The CI Windows failure looks like a Travis problem, I need to move this all to Azure Pipelines anyway.

@okofish:

I don't really get PROXY mode

The key gist of PROXY mode is to act as a transparent passthru. It can be useful for Wireshark-like tools, for example. Basically, PROXY won't automatically respond to negotiation, and assumes some other layer (e.g. another remote end) is doing the actual negotiation, and that libtelnet is effectively just working as a protocol parser only.

Everything else looks good to me.

Did you want to add yourself to the AUTHORS file, especially as this is a significant feature?

seanmiddleditch avatar May 27 '19 06:05 seanmiddleditch

Did you want to add yourself to the AUTHORS file, especially as this is a significant feature?

Done, thanks for the reminder.

I added input validation on the three functions that take a telopt as an argument to make sure it's within the valid range.

okofish avatar May 27 '19 18:05 okofish

This is awesome, are we getting this merged soon?

thefallentree avatar Oct 25 '19 21:10 thefallentree

Ack, sorry, I'm terrible at paying attention to GH notifications! :/ I'll figure out what's up with the Travis CI and then merge this.

seanmiddleditch avatar Feb 05 '20 18:02 seanmiddleditch

merged as https://github.com/fluffos/libtelnet/commit/e9c2128ae999f6549ad71367a959c1b5e330a958

thefallentree avatar Mar 03 '20 09:03 thefallentree