On Sun, Mar 14, 2021 at 7:27 AM Nicolas Iooss <nicolas.iooss@xxxxxxx> wrote:
>
> On Thu, Mar 11, 2021 at 4:41 PM James Carter <jwcart2@xxxxxxxxx> wrote:
> >
> > When reading a binary policy, do not automatically change the version
> > to the max policy version supported by libsepol or, if specified, the
> > value given using the "-c" flag.
> >
> > If the binary policy version is less than or equal to version 23
> > (POLICYDB_VERSION_PERMISSIVE) than do not automatically upgrade the
> > policy and if a policy version is specified by the "-c" flag, only set
> > the binary policy to the specified version if it is lower than the
> > current version.
> >
> > If the binary policy version is greater than version 23 than it should
> > be set to the maximum version supported by libsepol or, if specified,
> > the value given by the "-c" flag.
> >
> > The reason for this change is that policy versions 20
> > (POLICYDB_VERSION_AVTAB) to 23 have a more primitive support for type
> > attributes where the datums are not written out, but they exist in the
> > type_attr_map. This means that when the binary policy is read by
> > libsepol, there will be gaps in the type_val_to_struct and
> > p_type_val_to_name arrays and policy rules can refer to those gaps.
> > Certain libsepol functions like sepol_kernel_policydb_to_conf() and
> > sepol_kernel_policydb_to_cil() do not support this behavior and need
> > to be able to identify these policies. Policies before version 20 do not
> > support attributes at all and can be handled by all libsepol functions.
> >
> > Signed-off-by: James Carter <jwcart2@xxxxxxxxx>
> > ---
> > checkpolicy/checkpolicy.c | 16 +++++++++++++---
> > 1 file changed, 13 insertions(+), 3 deletions(-)
> >
> > diff --git a/checkpolicy/checkpolicy.c b/checkpolicy/checkpolicy.c
> > index 5841c5c4..e7b225b8 100644
> > --- a/checkpolicy/checkpolicy.c
> > +++ b/checkpolicy/checkpolicy.c
> > @@ -106,7 +106,7 @@ static int handle_unknown = SEPOL_DENY_UNKNOWN;
> > static const char *txtfile = "policy.conf";
> > static const char *binfile = "policy";
> >
> > -unsigned int policyvers = POLICYDB_VERSION_MAX;
> > +unsigned int policyvers = 0;
>
> Hi,
> This change breaks "make test" when testing secilc. This is because
> secilc/Makefile uses "checkpolicy -V" to compute a policy version:
>
> POL_VERS = $(shell $(CHECKPOLICY) -V | cut -f 1 -d ' ')
>
> Before your patch:
>
> $ checkpolicy -V
> 33 (compatibility range 33-15)
>
> After:
>
> $ checkpolicy -V
> 0 (compatibility range 33-15)
>
> .. which makes secilc/Makefile try using "secilc -c 0 ...", which does not work.
>
> Should "policyvers ? policyvers : POLICYDB_VERSION_MAX" also be used
> when "checkpolicy -V" displays version information?
>
Yes it should. I'll send an updated patch.
Thanks,
Jim
> Thanks,
> Nicolas
>
> >
> > static __attribute__((__noreturn__)) void usage(const char *progname)
> > {
> > @@ -588,6 +588,16 @@ int main(int argc, char **argv)
> > exit(1);
> > }
> > }
> > +
> > + if (policydbp->policyvers <= POLICYDB_VERSION_PERMISSIVE) {
> > + if (policyvers > policydbp->policyvers) {
> > + fprintf(stderr, "Binary policies with version <= %u cannot be upgraded\n", POLICYDB_VERSION_PERMISSIVE);
> > + } else if (policyvers) {
> > + policydbp->policyvers = policyvers;
> > + }
> > + } else {
> > + policydbp->policyvers = policyvers ? policyvers : POLICYDB_VERSION_MAX;
> > + }
> > } else {
> > if (conf) {
> > fprintf(stderr, "Can only generate policy.conf from binary policy\n");
> > @@ -629,6 +639,8 @@ int main(int argc, char **argv)
> > policydb_destroy(policydbp);
> > policydbp = &policydb;
> > }
> > +
> > + policydbp->policyvers = policyvers ? policyvers : POLICYDB_VERSION_MAX;
> > }
> >
> > if (policydb_load_isids(&policydb, &sidtab))
> > @@ -654,8 +666,6 @@ int main(int argc, char **argv)
> > }
> > }
> >
> > - policydb.policyvers = policyvers;
> > -
> > if (!cil) {
> > if (!conf) {
> > policydb.policy_type = POLICY_KERN;
> > --
> > 2.26.2
> >
>
