Merge lp:~cviethen/hipl/pisa-pairing into lp:hipl

[Christoph Viethen]

This implements a command "hipconf pair <ip-address | hostname>" (in case a hostname is given, it will be resolved to an IP address before getting passed to hipd).

This causes the local hipd to attempt an opportunistic base exchange towards the given IP (i.e., leaving the destination HIT open).

The idea is to use this for a pairing scenario for the PiSA project: users would connect their mobile device to their trust point locally (say, by cable), engage "hipconf pair <ip-address-of-trustpoint>" on that client device, and consequently have this mobile device turn up in the list of host associations on the trust point side. (From there on, an entry from that list could be trivially copied into a config file, causing the mobile device to be a "known" or "paired" with the trust point, enabling special treatment, network access privileges etc.)

Thanks!

[Miika Komu (miika-iki)]

I have few comments.

1. The "pair" is somewhat misleading, I think it can be mixex with HIT-IP mapping which has a different hipconf command. I think "bex", "trigger" or something else could be more relevant to RFC5201 terminology.
2. The functionality is not modular enough. I would add the HIT as an option for triggering the bex (to trigger a normal base exchange).

The biggest problem is however:

3. The functionality what this does is largely redundant with "hipconf add server". It could be piggypacked based on this functionality with a "do not register anything" extra parameter. Note this would not prevent having a different "hipconf foobar" option (this last comment is about code reuse, not syntax).

review: Needs Fixing

[Stefan Götz (stefan.goetz-deactivatedaccount)]

Hi Christoph!

Nice feature!

> === modified file 'hipd/netdev.c'
> --- hipd/netdev.c 2011-05-06 11:54:08 +0000
> +++ hipd/netdev.c 2011-06-06 16:15:55 +0000
> @@ -726,6 +726,105 @@
> }
>
> /**
> + * Begin an opportunistic base exchange with the IP address specified in the
> + * user message, creating an appropriate HIP association if necessary.
> + *
> + * @param msg the user message - it needs to contain an IP as its only parameter

[M] documentation: Hm - 'user message' might be used elsewhere but it is
confusing as it dates back to the good ol days of HIP residing in the kernel.
How about hipconf message? Which parameter type is used for the IP address? Is
it an IPv4 or IPv6 address? Please be more precise for people who would like to
know how exactly to use your function.

[M] policy: please cover this function with unit tests

> +int hip_netdev_pair(const struct hip_common *msg)

[M] style: please use full const correctness: 'const struct hip_common *const msg'

> + /* a few simple plausibility checks */
> + if (!(ipv6_addr_is_hit(&our_hit) && ipv6_addr_is_hit(&peer_hit) && hip_hidb_hit_is_our(&our_hit))) {
> + HIP_ERROR("Internal error.\n");

[L] usability: even if no one will try to interpret this message however helpful
it tries to be, it is still painfully useless in its current form :)
[L] would it make sense to use HIP_ASSERT() here?

> + /* if we got an entry in the HADB already, proceed to sending I1 */
> + entry = hip_hadb_find_byhits(&our_hit, &peer_hit);
> + if (entry && !ipv6_addr_any(&entry->our_addr)) {
> + goto send_i1;

[M] cleanliness: urgh, goto programming? I thought we decided that that is a bad
idea like what? 40 years ago? ;-) Please use a simple if block here

> + /* if not, create such an entry first */
> + if (hip_hadb_add_peer_info(&peer_hit, peer_ip, NULL, NULL)) {
> + HIP_ERROR("Creation of HADB entry failed.\n");
> + return -1;
> + }

[M] correctness: in case of a later error, this hadb entry is not removed again.
Would that make sense?

[L] style: overall, this is a very long function. I believe it would be cleaner
to split it at least into the part that parses the message and determines all
other input parameters and the part that actually creates the HADB entry and
sends the I1.

> === modified file 'lib/core/conf.c'
> --- lib/core/conf.c 2011-05-31 13:40:37 +0000
> +++ lib/core/conf.c 2011-06-06 16:15:55 +0000
> @@ -234,6 +236,7 @@
> "shotgun on|off\n"
> "id-to-addr hit|lsi\n"
> "broadcast on|off\n"
> + "pair <ip|ip6|hostname>\n"

[M] documentation/usability: even if the other commands do not do it: please add
a brief description of the effect of this command.

> @@ -2353,6 +2362,95 @@
> }
>
> /**
> + * Handles the hipconf @c pair command.
> + *
> + * @param msg a pointer to the buffer where the message for hipd will
> + * be written.
> + * @param action (currently unused)
> + * @param opt an array of pointers to the command line arguments after
> + * the action and type.
> + * @param optc the number of elements in the array (@b 0).
> + * @param send_only (currently unused)
> + * @ret...

review: Needs Fixing

[Stefan Götz (stefan.goetz-deactivatedaccount)]

What's the status of this proposal? It has been dormant for 6 months now. Are the reasons against deleting it?

[Miika Komu (miika-iki)]

I believe this branch can be abandoned as the following branch seems to contain a later incarnation of the same idea:

https://code.launchpad.net/~cviethen/hipl/pisa-pairing-tng/+merge/87876

Unmerged revisions

5952. By Christoph Viethen on 2011-06-06

Add "hipconf pair <ip or hostname>" command, an appropriate user message,
and the necessary code in hipd (hipd/netdev.c:hip_netdev_pair()) to establish
the following pairing mechanism:

With only the destination IP specified by the user, send an I1 message there
with an unspecified destination HIT (opportunistic mode), causing a base
exchange. As a result, a host association between the two nodes is
created, enabling this information to be used for pairing two nodes (PiSA
project).

=== modified file 'hipd/netdev.c'
--- hipd/netdev.c	2011-05-06 11:54:08 +0000
+++ hipd/netdev.c	2011-06-06 16:15:55 +0000
@@ -726,6 +726,105 @@
 }
 
 /**
+ * Begin an opportunistic base exchange with the IP address specified in the
+ * user message, creating an appropriate HIP association if necessary.
+ *
+ * @param msg the user message - it needs to contain an IP as its only parameter
+ * @return    zero on success and non-zero on error
+ * @note This function will find the source IP (by looking up the destination IP
+ *       in the routing table) and the source HIT (by taking the default HIT)
+ *       on its own. At the same time, the destination HIT will be empty
+ *       (opportunistic mode). Accordingly, only the destination IP is required
+ *       as input.
+ */
+int hip_netdev_pair(const struct hip_common *msg)
+{
+    struct       in6_addr our_ip;
+    hip_hit_t             our_hit, peer_hit;
+
+    struct hip_hadb_state *entry = NULL;
+
+
+    /* find source & dest IP address */
+    const struct hip_tlv_common *const param   = hip_get_param(msg, HIP_PARAM_IPV6_ADDR);
+    const struct in6_addr *const       peer_ip = hip_get_param_contents_direct(param);
+
+    if (!param) {
+        HIP_ERROR("No dest. IP in user message.\n");
+        return -1;
+    }
+
+    if (hip_select_source_address(&our_ip, peer_ip)) {
+        HIP_ERROR("Cannot find our source IP.\n");
+        return -1;
+    }
+
+    HIP_DEBUG_IN6ADDR("pairing, src IP", &our_ip);
+    HIP_DEBUG_IN6ADDR("pairing, dest IP", peer_ip);
+
+    /* find source & dest HIT */
+    if (hip_get_default_hit(&our_hit)) {
+        HIP_ERROR("Cannot find our own HIT.\n");
+        return -1;
+    }
+
+    if (hip_opportunistic_ipv6_to_hit(peer_ip, &peer_hit, HIP_HIT_TYPE_HASH100)) {
+        HIP_ERROR("hip_opportunistic_ipv6_to_hit failed.\n");
+        return -1;
+    }
+
+    HIP_DEBUG_HIT("pairing, src hit", &our_hit);
+    HIP_DEBUG_HIT("pairing, dst hit", &peer_hit);
+
+    /* a few simple plausibility checks */
+    if (!(ipv6_addr_is_hit(&our_hit) && ipv6_addr_is_hit(&peer_hit) && hip_hidb_hit_is_our(&our_hit))) {
+        HIP_ERROR("Internal error.\n");
+        return -1;
+    }
+
+    /* if we got an entry in the HADB already, proceed to sending I1 */
+    entry = hip_hadb_find_byhits(&our_hit, &peer_hit);
+    if (entry && !ipv6_addr_any(&entry->our_addr)) {
+        goto send_i1;
+    }
+
+    /* if not, create such an entry first */
+    if (hip_hadb_add_peer_info(&peer_hit, peer_ip, NULL, NULL)) {
+        HIP_ERROR("Creation of HADB entry failed.\n");
+        return -1;
+    }
+
+    if (!(entry = hip_hadb_find_byhits(&our_hit, &peer_hit))) {
+        HIP_ERROR("Internal lookup error.\n");
+        return -1;
+    }
+
+    entry->local_udp_port = (hip_nat_status ? hip_get_local_nat_udp_port() : 0);
+    entry->peer_udp_port  = (hip_nat_status ? hip_get_peer_nat_udp_port() : 0);
+    entry->nat_mode       = hip_nat_status;
+
+send_i1:
+    if ((entry->hip_msg_retrans.buf == NULL) ||
+        (entry->hip_msg_retrans.count == 0)) {
+        HIP_DEBUG("Expired retransmissions, sending i1\n");
+    } else {
+        HIP_DEBUG("I1 was already sent, ignoring\n");
+        return 0;
+    }
+
+    if (ipv6_addr_any(&entry->our_addr)) {
+        ipv6_addr_copy(&entry->our_addr, &our_ip);
+    }
+
+    if (hip_send_i1(&entry->hit_our, &entry->hit_peer, entry)) {
+        HIP_ERROR("Sending of I1 failed.\n");
+        return -1;
+    }
+
+    return 0;
+}
+
+/**
  * Create a HIP association (if one does not exist already) and
  * trigger a base exchange with an I1 packet using the given
  * arguments. This function also supports HIP-based loopback

=== modified file 'hipd/netdev.h'
--- hipd/netdev.h	2011-03-29 18:12:08 +0000
+++ hipd/netdev.h	2011-06-06 16:15:55 +0000
@@ -47,6 +47,7 @@
 int hip_remove_iface_all_local_hits(void);
 int hip_add_iface_local_route(const hip_hit_t *local_hit);
 int hip_select_source_address(struct in6_addr *src, const struct in6_addr *dst);
+int hip_netdev_pair(const struct hip_common *msg);
 int hip_netdev_trigger_bex_msg(const struct hip_common *msg);
 void hip_add_address_to_list(struct sockaddr *addr, int ifindex, int flags);
 

=== modified file 'hipd/user.c'
--- hipd/user.c	2011-05-16 11:39:06 +0000
+++ hipd/user.c	2011-06-06 16:15:55 +0000
@@ -709,6 +709,10 @@
         HIP_DEBUG("hip_broadcast_status =  %d (should be %d)\n",
                   hip_broadcast_status, HIP_MSG_BROADCAST_OFF);
         break;
+    case HIP_MSG_PAIR:
+        HIP_DEBUG("HIP_MSG_PAIR\n");
+        err = hip_netdev_pair(msg);
+        break;
     default:
         HIP_ERROR("Unknown socket option (%d)\n", msg_type);
         err = -ESOCKTNOSUPPORT;

=== modified file 'lib/core/builder.c'
--- lib/core/builder.c	2011-05-16 11:39:06 +0000
+++ lib/core/builder.c	2011-06-06 16:15:55 +0000
@@ -1150,6 +1150,7 @@
     case HIP_MSG_REINIT_FULLRELAY:   return "HIP_MSG_REINIT_FULLRELAY";
     case HIP_MSG_FIREWALL_START:     return "HIP_MSG_FIREWALL_START";
     case HIP_MSG_MANUAL_UPDATE_PACKET: return "HIP_MSG_MANUAL_UPDATE_PACKET";
+    case HIP_MSG_PAIR:               return "HIP_MSG_PAIR";
     default:
         return lmod_get_packet_identifier(msg_type);
     }

=== modified file 'lib/core/conf.c'
--- lib/core/conf.c	2011-05-31 13:40:37 +0000
+++ lib/core/conf.c	2011-06-06 16:15:55 +0000
@@ -137,7 +137,8 @@
 /* unused, was ACTION_HANDOVER 39 */
 #define ACTION_MANUAL_UPDATE 40
 #define ACTION_BROADCAST 41
-#define ACTION_MAX 42 /* exclusive */
+#define ACTION_PAIR 42
+#define ACTION_MAX 43 /* exclusive */
 
 /**
  * TYPE_ constant list, as an index for each action_handler function.
@@ -187,7 +188,8 @@
 /* unused, was TYPE_HANDOVER 42 */
 #define TYPE_MANUAL_UPDATE 43
 #define TYPE_BROADCAST     44
-#define TYPE_MAX           45 /* exclusive */
+#define TYPE_PAIR          45
+#define TYPE_MAX           46 /* exclusive */
 
 /* #define TYPE_RELAY         22 */
 
@@ -234,6 +236,7 @@
     "shotgun on|off\n"
     "id-to-addr hit|lsi\n"
     "broadcast on|off\n"
+    "pair <ip|ip6|hostname>\n"
 ;
 
 /**
@@ -596,6 +599,8 @@
         }
     } else if (!strcmp("broadcast", argv[1])) {
         ret = ACTION_BROADCAST;
+    } else if (!strcmp("pair", argv[1])) {
+        ret = ACTION_PAIR;
     }
 
     return ret;
@@ -633,6 +638,7 @@
     case ACTION_HIT_TO_IP:
     case ACTION_HIT_TO_IP_SET:
     case ACTION_BROADCAST:
+    case ACTION_PAIR:
         count = 1;
         break;
     case ACTION_ADD:
@@ -722,6 +728,8 @@
         ret = TYPE_LSI_TO_HIT;
     } else if (strcmp("broadcast", argv[1]) == 0) {
         ret = TYPE_BROADCAST;
+    } else if (strcmp("pair", argv[1]) == 0) {
+        ret = TYPE_PAIR;
     } else {
         HIP_DEBUG("ERROR: NO MATCHES FOUND \n");
     }
@@ -765,6 +773,7 @@
     case ACTION_HIT_TO_IP:
     case ACTION_HIT_TO_IP_SET:
     case ACTION_BROADCAST:
+    case ACTION_PAIR:
         type_arg = 2;
         break;
     case ACTION_MANUAL_UPDATE:
@@ -2353,6 +2362,95 @@
 }
 
 /**
+ * Handles the hipconf @c pair command.
+ *
+ * @param msg    a pointer to the buffer where the message for hipd will
+ *               be written.
+ * @param action (currently unused)
+ * @param opt    an array of pointers to the command line arguments after
+ *               the action and type.
+ * @param optc   the number of elements in the array (@b 0).
+ * @param send_only (currently unused)
+ * @return       zero on success, or negative error value on error.
+ */
+static int hip_conf_handle_pair(struct hip_common *msg,
+                                UNUSED int action,
+                                const char *opt[],
+                                int optc,
+                                UNUSED int send_only)
+{
+    int                   err;
+    struct in6_addr       ipv6_addr;
+    struct addrinfo      *addrinfo_result = NULL;
+    const struct addrinfo addrinfo_hints  = { .ai_family   = AF_UNSPEC,
+                                              .ai_protocol = IPPROTO_UDP };
+
+    if (optc != 0) {
+        HIP_ERROR("Too many arguments for pair command.\n");
+        return -1;
+    }
+
+    /* use getaddrinfo() to find an address for the given hostname;
+     *  it won't do any lookups in case the user just provides an
+     *  IP address */
+
+    err = getaddrinfo(opt[0], NULL, &addrinfo_hints, &addrinfo_result);
+
+    /* We don't specify what address family we want, and we may get a
+     *  linked list of multiple results: IPv4, IPv6 or anything else
+     *  the specific implementation likes to come up with. The "best"
+     *  results are supposed to come first (cf. RFC 3484), with the
+     *  admin having some influence by tweaking /etc/gai.conf.
+     *
+     * Accordingly, we stop walking the linked list as soon as we find
+     *  an entry that points to an actual IP address, hoping that
+     *  everything was set up wisely. */
+
+    if (err == 0) {
+        int                    done   = 0;
+        const struct addrinfo *ai_res = addrinfo_result;
+
+        while (done == 0 && ai_res != NULL) {
+            if (ai_res->ai_addr->sa_family == AF_INET) {
+                /* turn address into an IPv4-mapped IPv6 address
+                 *  (RFC 4291, 2.5.5.2.) */
+                IPV4_TO_IPV6_MAP(&((struct sockaddr_in *) ai_res->ai_addr)->sin_addr, &ipv6_addr);
+                done = 1;
+            }
+            if (ai_res->ai_addr->sa_family == AF_INET6) {
+                memcpy(&ipv6_addr, &((struct sockaddr_in6 *) ai_res->ai_addr)->sin6_addr, sizeof(struct in6_addr));
+                done = 1;
+            }
+            ai_res = ai_res->ai_next;
+        }
+
+        if (addrinfo_result != NULL) {
+            freeaddrinfo(addrinfo_result);
+        }
+
+        if (done == 0) {
+            HIP_ERROR("Failed to find any IP address for hostname.");
+            return -1;
+        }
+    } else {
+        HIP_ERROR("Failed to resolve hostname: %s\n", gai_strerror(err));
+        return -1;
+    }
+
+    if ((err = hip_build_user_hdr(msg, HIP_MSG_PAIR, 0))) {
+        HIP_ERROR("Failed to build user message header: %s\n", strerror(err));
+        return -1;
+    }
+
+    if ((err = hip_build_param_contents(msg, &ipv6_addr, HIP_PARAM_IPV6_ADDR, sizeof(struct in6_addr)))) {
+        HIP_ERROR("Failed to build IP address parameter to hipconf user message.\n");
+        return -1;
+    }
+
+    return 0;
+}
+
+/**
  * Function pointer array containing pointers to handler functions.
  * Add a handler function for your new action in the action_handler[] array.
  * If you added a handler function here, do not forget to define that function
@@ -2425,6 +2523,7 @@
     NULL,                               /* 42: unused, was TYPE_HANDOVER */
     hip_conf_handle_manual_update,      /* 43: TYPE_MANUAL_UPDATE */
     hip_conf_handle_broadcast,          /* 44: TYPE_BROADCAST */
+    hip_conf_handle_pair,               /* 45: TYPE_PAIR */
     NULL     /* TYPE_MAX, the end. */
 };
 

=== modified file 'lib/core/icomm.h'
--- lib/core/icomm.h	2011-05-16 11:39:06 +0000
+++ lib/core/icomm.h	2011-06-06 16:15:55 +0000
@@ -158,6 +158,7 @@
 #define HIP_MSG_FIREWALL_STATUS                  201
 #define HIP_MSG_BROADCAST_OFF                    202
 #define HIP_MSG_BROADCAST_ON                     203
+#define HIP_MSG_PAIR                             204
 /* @} */
 
 /* inclusive */