Subject: [Hipsec] draft-ietf-hip-native-api-09-pre

From: Miika Komu <[email-masked]>
Date: Fri, 21 Aug 2009 00:07:34 +0300

Hi,

we got an extra review to the native API from Stefan Götz. The new 
preversion is here:

http://www.iki.fi/miika/docs/draft-ietf-hip-native-api-09-pre1.txt

The changes are editorial readability changes throughout the document. 
Especially section 4.1 contains now more clarifications on the fields of 
sockaddr_hip structure and wildcards

We'd like to move on with the document, but we have two questions for 
the working group:

#1 How to future proof HITs in case we need 256 bit HITs? This is 
important also from the view point of comparison of HITs (currently 
draft suggests memcmp() in section 4.1. Unless, there's other 
suggestions, I'd go for alternative (i):

     * Alternative (i): separate sockaddr_hip structure for larger HITs
     * Alternative (ii): make larger HIT structure in sockaddr_hip with 
zero padding for 128 bit HITs

#2 How should the socket calls react to only-hip wildcard. Currently 
section 4.1.1 describes:

    With the HIP_HIT_ANY address,
    the underlying system allows only HIP-based data flows with the
    corresponding socket.  For incoming packets, the system transparently
    discards all other traffic arriving at the socket than HIP related.
    For outgoing packets, the system returns -1 in the socket call and
    sets errno to ECOMM when the system failed to deliver the packet over
    a HIP-based data channel.

---

From: "Ahrenholz, Jeffrey M" <[email-masked]>
Date: Thu, 20 Aug 2009 16:56:06 -0700

> The changes are editorial readability changes throughout the 
> document. Especially section 4.1 contains now more clarifications on 
> the fields of sockaddr_hip structure and wildcards

I read the new preversion and it looks pretty good. Only this minor editorial nits below:
Sect 3.1 Interaction with the Resolver
s/should be noticed/should be noted/

> zero padding for 128 bit HITs

yes I like alternative (i) better than (ii); maybe you could make use of ship_flags to indicate new ship_hit bit sizes in the future

note that in section 4.1.1 you also describe storing IPv6 and IPv4-mapped addresses in the ship_hit field; (should the ship_flags indicate these locator types?) in the future you would need to come up with new mappings, such as IPv4/IPv6-mapped to a 256 bit HIT.

why is HIP_HIT_ANY_TMP used for anonymous identifiers? maybe it should be named HIP_HIT_ANY_ANON or HIP_HIT_ANY_NOPUB?
 
>     a HIP-based data channel.

maybe you are asking if the text is enough?
it seems fairly clear to me:
- bind() to HIP_HIT_ANY and the socket provides you only with HIP data
- connect() to HIP_HIT_ANY and you are required to specify a locator (sect 4.6) (and I assume it would establish an association w/anonymous I1)
- error under other circumstances

-Jeff

---

From: Miika Komu <[email-masked]>
Date: Fri, 21 Aug 2009 07:42:46 +0300

Ahrenholz, Jeffrey M wrote:

Hi Jeff,

thanks for feedback! Let's iterate further.

> s/should be noticed/should be noted/

thanks.

> yes I like alternative (i) better than (ii); 

Ok. Notice that this option would need also flag in the resolver call to 
force the resolve to return larger HITs.

 > maybe you could make use of ship_flags to indicate new ship_hit bit 
sizes in the future

Your comment is related to option ii.

There is a 128-bit reserved field after the HIT in the sockaddr_hip 
structure, so this is possible even now. However, the problem is with 
existing applications because they will hard-code ACL checks with 
sizeof(128 bits), so we'd really need an union or something to force the 
sizeof to be always 256 bits. Otherwise, the application compares only 
128 bits of the HIT. Setting HIP_HIT_ANY_* will also become more tricky 
with the union and we'd need to specify explicitly where to fill them in 
(since the resolver is not currently specified to fill them in 
automatically). So, the 128-bit reserved field cannot really be used for 
extending the size of the HIT and we really need a completely new 
structure for them?

Jeff, what do you think about this? Are the associate problems now more 
clear? I think it's important to get this right.

> note that in section 4.1.1 you also describe storing IPv6 and IPv4-mapped addresses in the ship_hit field; (should the ship_flags indicate these locator types?) in the future you would need to come…

It's redundant information because the HIT has always a separate prefix. 
sockaddr_is_srcaddr() can be used to verify local HITs, but not peer. 
Should we actually have a function/macro called HIP_IS_HIT()? Or just 
add a flag as you suggested to avoid each application to hard-code their 
own macros for this? What do you think?

> why is HIP_HIT_ANY_TMP used for anonymous identifiers? maybe it should be named HIP_HIT_ANY_ANON or HIP_HIT_ANY_NOPUB?

To align it with RFC5014 constants. Do you think ANON would be more clear?

> maybe you are asking if the text is enough?

Yes and I was asking also what should the actual error value be (pick 
one from /usr/include/asm-generic/errno.h)? It could be also ETIMEDOUT 
because the peer failed to respond with an R1. Or in the case of 
hiccups, there was no response from the transport layer...

In comparison, it seems like SCTP API just typically ignores errno 
issues with "the variable errno is then set appropriately":

http://tools.ietf.org/html/draft-ietf-tsvwg-sctpsocket-19

> - error under other circumstances

There is a separate error message when there is no locator with 
connect() in section 4.2.1:

    When the system fails to
    provide the mapping, it returns -1 in the called sockets API function
    to the application and sets errno to EADDRNOTAVAIL.

---

From: "Henderson, Thomas R" <[email-masked]>
Date: Fri, 21 Aug 2009 11:15:27 -0700

>     a HIP-based data channel.

I would like to suggest these changes to the above paragraph:

    With the HIP_HIT_ANY address,
    the underlying system allows only HIP-based data flows with the
    corresponding socket.  For incoming packets, the system
    discards all non-HIP-related traffic arriving at the socket.
    For outgoing packets, the system returns -1 in the socket call and
    sets errno to an appropriate error type when the system failed to
deliver the packet over
    a HIP-based data channel.

rationale:
1) for incoming datagrams, "discards" rather than "transparently
discards" is a policy issue that is not related to the API (e.g. whether
a system returns some type of ICMP error is out of scope for the API
specification)
2) for outgoing packets, different systems appear to specify different
errnos under different circumstances, so it probably is too restrictive
to specify ECOMM here.  ECOMM, for instance, does not appear to be an
error type returned by Linux as a failure code for connect().

Tom

---

From: Miika Komu <[email-masked]>
Date: Mon, 24 Aug 2009 11:56:02 +0300

Henderson, Thomas R wrote:

Hi,

>> transparently

seems reasonable to me.

---

From: "Ahrenholz, Jeffrey M" <[email-masked]>
Date: Mon, 24 Aug 2009 10:16:50 -0700

> So, the 128-bit reserved field cannot really be used for 

OK, I overlooked the 128-bit ship_reserved field. Yes it seems like we
need a new structure; right now we're considering 256-bit HITs but what
about 512, 1024, etc.?

> clear? I think it's important to get this right.

will larger HITs share the same prefix as existing 128-bit HITs? 

if we have different prefixes, API calls can use sockaddr with
sa_family=AF_HIP and we can inspect the prefix to determine the bitsize;
this is a departure from the sa_family alone determining the size of the
structure

> add a flag as you suggested to avoid each application to 
> hard-code their own macros for this? What do you think?

Yes I think this HIP_IS_HIT() macro would be helpful.

> > why is HIP_HIT_ANY_TMP used for anonymous identifiers? 
> maybe it should be named HIP_HIT_ANY_ANON or HIP_HIT_ANY_NOPUB?
> 

The _TMP is OK, makes sense to align with RFC 5014 terminology.

> http://tools.ietf.org/html/draft-ietf-tsvwg-sctpsocket-19

I think that Tom's suggested text addresses this with the "set errno
appropriately" language.

-Jeff

---

From: Miika Komu <[email-masked]>
Date: Tue, 25 Aug 2009 00:26:25 +0300

Hi,

thanks Jeff for feedback!

> need a new structure; right now we're considering 256-bit HITs but what

Good point. The maximum size for a socket address structure is 255 
octets. Unfortunately, the old "endpoint descriptor" concept was voted 
out (at apps and hip area) and it would have been nice for this purpose. 
It proposed basically a handle that could hide an identifier of any size.

> will larger HITs share the same prefix as existing 128-bit HITs? 

Good question and we probably don't have a good answer for this now.

> this is a departure from the sa_family alone determining the size of the

The sockets API basically gives constant size data structures for 
applications. I think we shouldn't break this design pattern. So 
reserving a constant size extension field is ok, but variable size 
structure (with the same name) is probably not a good idea.

Based on this discussion, it appears that the best way for future 
proofing is to include a flag for getaddrinfo() in the hints arguments 
that will result in sockaddr_hip_long structures instead of 
sockaddr_hip. This way, applications get exactly what they want and can 
store the HITs easily in their ACLs. I'd also remove the extra space 
reserved space because it seems that it's not very usable for future 
proofing as it is now.

Jeff, how do you feel about this or do you have a better suggestion?

> Yes I think this HIP_IS_HIT() macro would be helpful.

Ok. Then we'll have one function for testing the local HITs and a second 
one for testing both local and peer HITs. Is this ok or should they be 
unified?

> The _TMP is OK, makes sense to align with RFC 5014 terminology.

Ok.

> I think that Tom's suggested text addresses this with the "set errno

Agreed.

---

From: Andrew McGregor <[email-masked]>
Date: Tue, 25 Aug 2009 09:39:19 +1200

On 25/08/2009, at 9:26 AM, Miika Komu wrote:

> proofing is to include a flag for getaddrinfo() in the hints  
> arguments that will result in sockaddr_hip_long structures instead  
> of sockaddr_hip. This way, applications get exactly what they want  
> and can store the HITs easily in their ACLs. I'd also remove the  
> extra space reserved space because it seems that it's not very  
> usable for future proofing as it is now.
>

I think you'd be better to define more than a flag... an explicit size  
(or enum) would be better.  That way if some other odd size becomes a  
requirement later, it's not such an annoyance to fit in to the API

Andrew

---

From: "Ahrenholz, Jeffrey M" <[email-masked]>
Date: Mon, 24 Aug 2009 15:24:08 -0700

> Ok. Then we'll have one function for testing the local HITs 
> and a second one for testing both local and peer HITs. Is this ok or 
> should they be unified?

seems like separate functions/macros are OK since sockaddr_is_srcaddr()
attempts to align with inet6_is_srcaddr()

the HIP_IS_HIT() macro I would think of as being much simpler, just
checks for the presence of a HIT prefix similar to the
IN6_IS_ADDR_LINKLOCAL() macro

-Jeff

---

From: "Ahrenholz, Jeffrey M" <[email-masked]>
Date: Mon, 24 Aug 2009 15:24:18 -0700

> requirement later, it's not such an annoyance to fit in to the API

yes, a size or flag makes sense; looks like struct addrinfo already has
"size_t ai_addrlen;" but that field must be zero when used as hints
(looking at getaddrinfo(3) man)

other calls such as connect(), bind(), and sendto() already take a
"socklen_t addrlen" argument

-Jeff

---

From: Andrew McGregor <[email-masked]>
Date: Tue, 25 Aug 2009 10:27:14 +1200

> "socklen_t addrlen" argument

Ah, in that case... getaddrinfo should return whatever it can, and  
applications should be required to check lengths if they care.  That  
avoids coding in bugs early.

Andrew

---

From: Miika Komu <[email-masked]>
Date: Tue, 25 Aug 2009 10:44:44 +0300

Andrew McGregor wrote:

Hi,

>> "socklen_t addrlen" argument

the socket address structure already contains the size of the socket 
within the first field. Having yet another size within the structure or 
a variable length socket address structure is confusing for people used 
to constant size structures such as sockaddr_in, sockaddr_in6 and 
sockaddr_storage.

Have a look at section "EXAMPLES" on getaddrinfo() FreeBSD man pages:

http://www.gsp.com/cgi-bin/man.cgi?section=3&topic=getaddrinfo

If there is an additional length field and it must be used in socket 
calls, it disrupts the flow of the client side example.

> avoids coding in bugs early.

The problem is that they do have check lengths when the applications 
fill in the socket address structures by themselves. I think it also a 
show stopper for allocations from the heap (struct addrinfo foo;). 
Dynamic allocations get somewhat tricky because defining the allocated 
size is not trivial.

A way to avoid the memory-related problem and "client flow" problem 
would be to define sockaddr_hip to contain the maximum number of bytes 
similarly as sockaddr_storage does. Would that work for you?

We would still need the actual HIT field to be defined as an union to 
facilitate naming of larger HITs withing the structure in a backwards 
compatible way. Right?

---

From: Miika Komu <[email-masked]>
Date: Thu, 27 Aug 2009 10:29:40 +0300

Miika Komu wrote:

Hi,

> facilitate naming of larger HITs withing the structure in a backwards 

Actually, sockaddr_un is variable length, but getaddrinfo() returns only 
fixed sized identifiers. Anyway, this is what a fixed size structure 
would look like:

struct sockaddr_hip {
	sa_family_t    ship_family;
	in_port_t      ship_port;
	uint32_t       ship_pad;
	uint64_t       ship_flags;
	union {
		hip_hit_t      ship_hit;
		uint8_t        ship_reserved[MAX];
	} ship_id;
};

According to some quick compilation testing, the implementation size of 
MAX in linux is:

   * 112 in the above definition
   * 122 if we get rid of the pad field and changes flags to 16 bits

Size of sockaddr_storage is 128 on linux which should determine the 
maximum size of sockaddr_hip.

Comments?

---

From: Miika Komu <[email-masked]>
Date: Mon, 31 Aug 2009 16:37:37 +0300

Miika Komu wrote:

Hi,

a new preversion is here:

http://www.iki.fi/miika/docs/draft-ietf-hip-native-api-09-pre2.txt

it includes the future proofing changes in the sockaddr_hip structure in 
section 4.1. I also wrote the ship_len explicitly to the structure since 
we were discussing on size issues.

Section 4.5 contains the sockaddr_is_hit() definition as requested by 
Jeff. Please note that it is not a macro due to future proofing.

Both the sockaddr_is_srcaddr() and sockaddr_is_hit() function take care 
of the wildcards currently as follows:

    The function returns also
    returns -1 on a sockaddr_hip structure containing a HIP_ENDPOINT_ANY
    or HIP_HIT_ANY_* wildcard.

Jeff and Andrew, please comment on the mailing if you are satisfied with 
the changes so that I can publish the final version of the draft.

---

From: "Ahrenholz, Jeffrey M" <[email-masked]>
Date: Mon, 31 Aug 2009 08:05:36 -0700

Miika,

> section 4.1. I also wrote the ship_len explicitly to the 
> structure since we were discussing on size issues.

In section 7 you might mention using the optional ship_len field (maybe
instead of flags and prefix.) 

> Both the sockaddr_is_srcaddr() and sockaddr_is_hit() function 
> take care of the wildcards currently as follows:
> 

This is good since the wildcards aren't HITs.

> the changes so that I can publish the final version of the draft.

Just a couple of nits:

Section 4.5
s/Application can call/Applications can call/
s/function returns also returns -1/function also returns -1/

Section 7
s/to indicate that can support/to indicate that it can support/
s/instead directly using/instead of directly using/

-Jeff