| Message ID | 20210311154105.195494-1-jwcart2@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] checkpolicy: Do not automatically upgrade when using "-b" flag | expand |
On Thu, Mar 11, 2021 at 4:41 PM James Carter <jwcart2@gmail.com> 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@gmail.com> > --- > 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? 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 >
On Sun, Mar 14, 2021 at 7:27 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > On Thu, Mar 11, 2021 at 4:41 PM James Carter <jwcart2@gmail.com> 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@gmail.com> > > --- > > 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 > > >
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; 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;
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@gmail.com> --- checkpolicy/checkpolicy.c | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-)