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

From: Adam Roach <[email-masked]>
Date: Wed, 10 Dec 2003 18:20:14 -0600

Pekka Pessi [mailto:[email-masked]] wrote:
> 
> 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. 

I'll put text in to that effect.

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

I don't think this is necessarily an implication. As long as the
implementation can make certain that the response is sent from the
same IP address, that's all we really need.

> 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?

Yes, I missed this error case. I will add it.

> 	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?)?

Both of those cases are meant to be covered by the "STACK_UNDERFLOW"
reason code.

> 	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.

How could this happen? More specifically, how would such a piece
of state be created?

> 	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".

That's a good point. Unless there are objections to doing so, I'll
remove it.

> 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

It makes it NACKs three bytes larger, but I don't really see any
harm in that. Since part of the rationale behind having this
kind of mechanism is assistance in debugging, that seems worthwhile.

If anyone objects to this addition, please speak up!

> 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.)
> """

Okay, I've added both of these.

> 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.

Yes, that was an oversight. I've renamed it and changed it to reflect
the proper register width.

> 	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.
> """

Fixed.

> Spelling:

Thanks. I ran a spell checker over it. (I Finally found 'aspell', which
allows you to check XML-formatted documents. Yaay!)

/a

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

---

From: Pekka Pessi <[email-masked]>
Date: Thu, 11 Dec 2003 17:16:45 +0200

>same IP address, that's all we really need.

You are correct. However, it might be a good idea to advice the
reader that the source IP address can usually determined by
binding the local IP address.

...

>Both of those cases are meant to be covered by the "STACK_UNDERFLOW"

Ok. 

...

>How could this happen? More specifically, how would such a piece

By state identifier collision, for instance. But I agree, the
sender can deduct that from SEGFAULT.

...

>Okay, I've added both of these.

	It could also be mentioned that in the case of framing
	error, the SHA-1 hash is unusable.

--Pekka