[Swan-dev] shunk_t vs chunk_t

[Andrew Cagney]

I've pushed the below.  It was a side effect of fixing a memory leak.
The cast is gone.

diff --git a/programs/pluto/security_selinux.h
b/programs/pluto/security_selinux.h
index 6f88f62c48..b8ef6120d4 100644
--- a/programs/pluto/security_selinux.h
+++ b/programs/pluto/security_selinux.h
@@ -27,13 +27,14 @@
 #include <selinux/context.h>
 #endif

+#include "shunk.h"
 #include "chunk.h"

 struct logger;

 void init_selinux(struct logger *logger);

-bool se_label_match(const chunk_t *a, const chunk_t *b, struct logger *logger);
+bool se_label_match(shunk_t a, chunk_t b, struct logger *logger);

 #endif /* HAVE_LABELED_IPSEC */


On Mon, 1 Mar 2021 at 14:58, Andrew Cagney <andrew.cagney at gmail.com> wrote:
>
> You do realize that sec_label is guaranteed to be NUL terminated and,
> hence, the below grotesk hack isn't needed?
>
> >
> > Here are the four calls to se_label match.  Together there are eight
> > chunk_t * parameters, each for a security label.  In only one case is
> > the parameter cast to chunk_t *.
> >
> > programs/pluto/connections.c:2480:                          && se_label_match(&sec_label, &sr->this.sec_label, logger)
> > programs/pluto/ikev1_spdb_struct.c:114: if (!se_label_match((chunk_t *)&sec_label, &c->spd.this.sec_label, st->st_logger)) {
> > programs/pluto/ikev2_ts.c:887:                  if (!se_label_match(&cur_i->sec_label, &d->spd.this.sec_label, logger)) {
> > programs/pluto/ikev2_ts.c:903:                                  } else if (se_label_match(&ends->r->sec_label, &d->spd.this.sec_label, logger)) {
> >
> > | - the sec label provided by the acquire is NUL terminated byte stream
> > | (note, not string) (I hacked IKEv1 to enforce this, somewhere I've a
> > | patch for IKEv2)
> >
> > Please do fix the IKEv2 case.  Right now there is code that
> > carelessly enforces the rule partially.  For example, in two places
> > in score_ends_seclabel().
> >
> > | - since the sec_label bubbling up from the acquire points into a
> > | readonly-ish stack structure it's arguable that it should be a shunk_t
> >
> > I would think so.
> >
> > | where things break down is with IKEv2's struct traffic_selector (IKEv1
> > | very clearly clones the sec_label into struct state).
> > |
> > | My hunch is that IKEv2's struct traffic_selector should be a shunk_t.
> > | It starts out pointing into the pbs_in structure but then, when
> > | selected and saved, is cloned and stored in the same fields as for
> > | IKEv1 with lots of pointers updated.
> >
> > Is the only magic in freeing the shunk?  Maybe we need a
> > free_shunk_content() where this magic is contained.
> > _______________________________________________
> > Swan-dev mailing list
> > Swan-dev at lists.libreswan.org
> > https://lists.libreswan.org/mailman/listinfo/swan-dev