netlink: abolish check_config and output_config

Apparently, a long time ago, MSG5 and MSG6 used to contain some
netlink configuration data, which the receiver of the MSG5 or MSG6
would check.

However, for a long time now the output_config function has been a
no-op and the check function has unconditionally eaten and discarded
anything extra in the message.

Furthermore, because the MSG6 is not retransmitted, this mechanism
couldn't be reliable without a protocol change.  So the existing
interface is defective.

So, abolish it the interface, the dummy implementation, and all the
call sites.  The check_config call sites in site.c now instead
directly discard any unexpected data at the end of MSG5 and MSG6.

This patch should cause no behavioural change in actual operation.

Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>

diff --git a/netlink.c b/netlink.c
index 4151e33688dbb91033dc9c90903c3e7202c83cec..486df6fd1515f24ce4204fb3229a4ec22ac8f925 100644 (file)
--- a/netlink.c
+++ b/netlink.c
@@ -803,27 +803,6 @@ static void netlink_signal_handler(void *sst, int signum)
     netlink_dump_routes(st,True);
 }
 
     netlink_dump_routes(st,True);
 }
 

-static void netlink_inst_output_config(void *sst, struct buffer_if *buf)
-{
-/*    struct netlink_client *c=sst; */
-/*    struct netlink *st=c->nst; */
-
-    /* For now we don't output anything */
-    BUF_ASSERT_USED(buf);
-}
-
-static bool_t netlink_inst_check_config(void *sst, struct buffer_if *buf)
-{
-/*    struct netlink_client *c=sst; */
-/*    struct netlink *st=c->nst; */
-
-    BUF_ASSERT_USED(buf);
-    /* We need to eat all of the configuration information from the buffer
-       for backward compatibility. */
-    buf->size=0;
-    return True;
-}
-

 static void netlink_inst_set_mtu(void *sst, int32_t new_mtu)
 {
     struct netlink_client *c=sst;
 static void netlink_inst_set_mtu(void *sst, int32_t new_mtu)
 {
     struct netlink_client *c=sst;


@@ -910,8 +889,6 @@ static closure_t *netlink_inst_create(struct netlink *st,
     c->ops.reg=netlink_inst_reg;
     c->ops.deliver=netlink_inst_incoming;
     c->ops.set_quality=netlink_set_quality;
     c->ops.reg=netlink_inst_reg;
     c->ops.deliver=netlink_inst_incoming;
     c->ops.set_quality=netlink_set_quality;

-    c->ops.output_config=netlink_inst_output_config;
-    c->ops.check_config=netlink_inst_check_config;

     c->ops.set_mtu=netlink_inst_set_mtu;
     c->nst=st;
 
     c->ops.set_mtu=netlink_inst_set_mtu;
     c->nst=st;
 

diff --git a/secnet.h b/secnet.h
index ecbb9959bc7d2fbc824514a82d5397b574f39c88..5f8e43bc5545b5f2a3ba2e9daf4178cca5ffb283 100644 (file)
--- a/secnet.h
+++ b/secnet.h
@@ -438,8 +438,6 @@ struct netlink_if {
     netlink_register_fn *reg;
     netlink_deliver_fn *deliver;
     netlink_link_quality_fn *set_quality;
     netlink_register_fn *reg;
     netlink_deliver_fn *deliver;
     netlink_link_quality_fn *set_quality;

-    netlink_output_config_fn *output_config;
-    netlink_check_config_fn *check_config;

     netlink_set_mtu_fn *set_mtu;
 };
 
     netlink_set_mtu_fn *set_mtu;
 };
 

diff --git a/site.c b/site.c
index fcc36d8671230dd40e6ec1b60d6c5f1dc13ad3cf..05206f6531db344f533b1bea54f489fc89c28416 100644 (file)
--- a/site.c
+++ b/site.c
@@ -628,7 +628,6 @@ static bool_t generate_msg5(struct site *st)
     buffer_init(&st->buffer,st->transform->max_start_pad+(4*4));
     /* Give the netlink code an opportunity to put its own stuff in the
        message (configuration information, etc.) */
     buffer_init(&st->buffer,st->transform->max_start_pad+(4*4));
     /* Give the netlink code an opportunity to put its own stuff in the
        message (configuration information, etc.) */

-    st->netlink->output_config(st->netlink->st,&st->buffer);

     buf_prepend_uint32(&st->buffer,LABEL_MSG5);
     st->new_transform->forwards(st->new_transform->st,&st->buffer,
                                &transform_err);
     buf_prepend_uint32(&st->buffer,LABEL_MSG5);
     st->new_transform->forwards(st->new_transform->st,&st->buffer,
                                &transform_err);


@@ -660,11 +659,8 @@ static bool_t process_msg5(struct site *st, struct buffer_if *msg5,
        slog(st,LOG_SEC,"MSG5/PING packet contained wrong label");
        return False;
     }
        slog(st,LOG_SEC,"MSG5/PING packet contained wrong label");
        return False;
     }

-    if (!st->netlink->check_config(st->netlink->st,msg5)) {
-       slog(st,LOG_SEC,"MSG5/PING packet contained bad netlink config");
-       return False;
-    }
-    CHECK_EMPTY(msg5);
+    /* Older versions of secnet used to write some config data here
+     * which we ignore.  So we don't CHECK_EMPTY */

     return True;
 }
 
     return True;
 }
 


@@ -677,7 +673,6 @@ static bool_t generate_msg6(struct site *st)
     buffer_init(&st->buffer,st->transform->max_start_pad+(4*4));
     /* Give the netlink code an opportunity to put its own stuff in the
        message (configuration information, etc.) */
     buffer_init(&st->buffer,st->transform->max_start_pad+(4*4));
     /* Give the netlink code an opportunity to put its own stuff in the
        message (configuration information, etc.) */

-    st->netlink->output_config(st->netlink->st,&st->buffer);

     buf_prepend_uint32(&st->buffer,LABEL_MSG6);
     st->new_transform->forwards(st->new_transform->st,&st->buffer,
                                &transform_err);
     buf_prepend_uint32(&st->buffer,LABEL_MSG6);
     st->new_transform->forwards(st->new_transform->st,&st->buffer,
                                &transform_err);


@@ -709,11 +704,8 @@ static bool_t process_msg6(struct site *st, struct buffer_if *msg6,
        slog(st,LOG_SEC,"MSG6/PONG packet contained invalid data");
        return False;
     }
        slog(st,LOG_SEC,"MSG6/PONG packet contained invalid data");
        return False;
     }

-    if (!st->netlink->check_config(st->netlink->st,msg6)) {
-       slog(st,LOG_SEC,"MSG6/PONG packet contained bad netlink config");
-       return False;
-    }
-    CHECK_EMPTY(msg6);
+    /* Older versions of secnet used to write some config data here
+     * which we ignore.  So we don't CHECK_EMPTY */

     return True;
 }
 
     return True;
 }