Subject: [rohc] Comments to draft-roach-sigcomp-nack-01

From: Pekka Pessi <[email-masked]>
Date: Sat, 18 Oct 2003 01:09:23 +0300

Hi,

I finally managed to do the review of draft-roach-sigcomp-nack-01.txt. 
I've divided my comments in technical issues, featuritis, nits and
spelling.

--Pekka

Technical issues:

	In the 2.2 last paragraph talks about destination address of
	NACK:

>>>For a connectionless transport, such as UDP, the NACK message is sent
>>>back to the originator of the failed message at the port and IP
>>>address from which the message was sent.  Note that this may or may
>>>not be the same port to which the appliation would typically receive
>>>messages.

	I'd also like to state that the NACK should be sent from the
	port and IP to which the original message was sent. This because
	the sender has most probably connect()ed the sending socket in
	order to catch ICMPs. So, when the original UDP message has

	(ip_p=UDP, ip_src=x, ip_dst=y, uh_sport=v, uh_dport=w)

	then the NACK should have

	(ip_p=UDP, ip_src=y, ip_dst=x, uh_sport=w, uh_dport=v)

	This also means that the socket used to receive SIGCOMP messages
	should not be bound to INADDR_ANY. Not to mention in6addr_any.

Comment to 3.2 Reason Codes:

	The set of reason codes seems to be rather complete. However,
	selecting reason code is not straightforward if the STATE-ACCESS
	has zero state-length but nonzero state-begin. INVALID_STATE_PROBE?

	What about errors mentioned by RFC 3320 section 8.7 item 7
	(RETURN_WITHOUT_CALL? POP_WITHOUT_PUSH? Or are these errors just
	ignored by current wisdom?)?

	When the initial bytecode state item exceeds the UDVM memory, it
	would be more informative get BYTECODES_TOO_LARGE instead of
	SEGFAULT. Also, if the initial bytecode state item has
	state_instruction outside UDVM memory INVALID_CODE_LOCATION
	would be more informative than SEGFAULT.

	Also, there is a security problem with ID_TOO_SHORT. If an
	attacker can guess the initial 6 bytes of a state identifier, he
	can quickly obtain the rest of the state ID with the help of
	ID_TOO_SHORT. With a message including the state contents in,
	for instance, a branch parameter of a SIP Via header, the
	attacker can then obtain the state contents from UDVM host. So,
	I'd recommend leaving out ID_TOO_SHORT. Instead of it,
	STATE_NOT_FOUND should be returned. Alternatively, mention
	ID_TOO_SHORT in "Security Considerations".

Featuritis:

	What about including PC and the byte code of the failed
	instruction in the NACK, like:

             0   1   2   3   4   5   6   7
           +---+---+---+---+---+---+---+---+
           | 1   1   1   1   1 | T |   0   |
           +---+---+---+---+---+---+---+---+
           |                               |
           :    returned feedback item     :
           |                               |
           +---+---+---+---+---+---+---+---+
           |         code_len = 0          |
           +---+---+---+---+---+---+---+---+
           | code_len = 0  |  version = 1  |
           +---+---+---+---+---+---+---+---+
           |          Reason Code          |
           +---+---+---+---+---+---+---+---+
           |  OPCODE of failed instruction |
           +---+---+---+---+---+---+---+---+
           |  PC-HI of failed instruction  |
           +---+---+---+---+---+---+---+---+
           |  PC-LO of failed instruction  |
           +---+---+---+---+---+---+---+---+
           |                               |
           : SHA-1 Hash of failed message  :
           |                               |
           +---+---+---+---+---+---+---+---+
           |                               |
           :         Error Details         :
           |                               |
           +---+---+---+---+---+---+---+---+

                 Figure 1: SigComp NACK Message Format

Featuritis 2: INTERNAL_ERROR

"""
INTERNAL_ERROR

	UDVM encountered an unexpected condition that prevented it from
	decompressing the message.
"""

Featuritis 3: FRAMING_ERROR

"""
FRAMING_ERROR

	UDVM encountered a framing error (unquoted 0xFF 80 .. 0xFF FE in
	input stream.)
"""

Nits:

	The description of BAD_BITORDER is a bit misleading -
	input_bit_order is a 16-bit register, so I'd expect BAD_BITORDER
	if any one of the upper 13 is set. I'd also write it
	BAD_INPUT_BIT_ORDER.

	Spell out instructions in TOO_MANY_BITS_REQUESTED:

"""
        An INPUT-BITS or INPUT-HUFFMAN instruction was encountered that
	attempted to input more than 16 bits.
"""

Spelling:

	s/appliation/application/
	s/applcation/application/
	s/applicaition/application/
	s/reciever/receiver/
	s/beleives/believes/
	s/occured/occurred/
	s/retreive/retrieve/
	s/receipient/recipient/
	s/decompresson/decompression/
	s/futher/further/

_______________________________________________
Rohc mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/rohc