On Tue, Apr 15, 2014 at 10:22:15AM -0700, john.johan...@canonical.com wrote:
> This will allow for the parser to invalidate its caches separate of whether
> the kernel policy version has changed. This can be desirable if a parser
> bug is discovered, a new version the parser is shipped and we need to
> force cache files to be regenerated.
> 
> Policy current stores a 32 bit version number in the header binary policy.
> For newer policy (> v5 kernel abi) split this number into 3 separate
> fields policy_version, parser_abi, kernel_abi.
> 
> If binary policy with a split version number is loaded to an older
> kernel it will be correctly rejected as unsupported as those kernels
> will see it as a none v5 version. For kernels that only support v5
> policy on the kernel abi version is written.
> 
> The rules for policy versioning should be
> policy_version:
>   Set by text policy language version. Parsers that don't understand
>   a specified version may fail, or drop rules they are unaware of.
> 
> parser_abi_version:
>   gets bumped when a userspace bug is discovered that requires policy be
>   recompiled. The policy version could be reset for each new kernel version
>   but since the parser needs to support multiple kernel versions tracking
>   this is extra work and should be avoided.
> 
> kernel_abi_version:
>   gets bumped when semantic changes need to be applied. Eg unix domain
>   sockets being mediated at connect.
> 
>   the kernel abi version does not encapsulate all supported features.
>   As kernels could have different sets of patches supplied. Basic feature
>   support is determined by the policy_mediates() encoding in the policydb.
> 
>   As such comparing cache features to kernel features is still needed
>   to determine if cached policy is best matched to the kernel.
> 
> Signed-off-by: John Johansen <john.johan...@canonical.com>

Acked-by: Seth Arnold <seth.arn...@canonical.com>

... with the very strong caveat that this ack depends upon one of the
future patches in the series which switches to htole64() and friends.
(Wow, these patches have been outstanding for far too long. Sorry.)

Thanks


> 
> ---
>  parser/parser.h           |   40 +++++++++++++++++++++++++++++++++++++++-
>  parser/parser_common.c    |   42 +++++++++++++++++++++++++++++++++++++++++-
>  parser/parser_interface.c |   18 +++---------------
>  parser/parser_main.c      |   40 +++++++++++++++++++++++++++++++++++++---
>  parser/parser_regex.c     |    2 +-
>  parser/parser_yacc.y      |    5 ++++-
>  6 files changed, 125 insertions(+), 22 deletions(-)
> 
> --- 2.9-test.orig/parser/parser.h
> +++ 2.9-test/parser/parser.h
> @@ -253,11 +253,49 @@
>                               goto fail_target;                               
> \
>       } while (0)
>  
> +
> +#define u8  unsigned char
> +#define u16 uint16_t
> +#define u32 uint32_t
> +#define u64 uint64_t
> +#if __BYTE_ORDER == __BIG_ENDIAN
> +#  define cpu_to_le16(x) ((u16)(bswap_16 ((u16) x)))
> +#  define cpu_to_le32(x) ((u32)(bswap_32 ((u32) x)))
> +#  define cpu_to_le64(x) ((u64)(bswap_64 ((u64) x)))
> +#else
> +#  define cpu_to_le16(x) ((u16)(x))
> +#  define cpu_to_le32(x) ((u32)(x))
> +#  define cpu_to_le64(x) ((u64)(x))
> +#endif
> +
> +/* The encoding for kernal abi > 5 is
> + * 28-31: reserved
> + * 20-27: policy version
> + * 12-19: policy abi version
> + * 11:    force complain flag
> + * 10:    reserved
> + * 0-9:   kernel abi version
> + */
> +#define ENCODE_VERSION(C, P, PABI, KABI)             \
> +({                                                   \
> +     u32 version = (KABI) & 0x3ff;                   \
> +     if ((KABI) > 5) {                               \
> +             version |= (C) ? 1 << 11 : 0;           \
> +             version |= ((PABI) & 0xff) << 12;       \
> +             version |= ((P) & 0xff) << 20;          \
> +     }                                               \
> +     version;                                        \
> +})
> +
>  /* from parser_common.c */
> +extern uint32_t policy_version;
> +extern uint32_t parser_abi_version;
> +extern uint32_t kernel_abi_version;
> +
> +extern int force_complain;
>  extern int perms_create;
>  extern int net_af_max_override;
>  extern int kernel_load;
> -extern int kernel_policy_version;
>  extern int kernel_supports_network;
>  extern int kernel_supports_policydb;
>  extern int kernel_supports_mount;
> --- 2.9-test.orig/parser/parser_common.c
> +++ 2.9-test/parser/parser_common.c
> @@ -22,10 +22,50 @@
>  #define _(s) gettext(s)
>  #include "parser.h"
>  
> +/* Policy versioning is determined by a combination of 3 values:
> + * policy_version:     version of txt policy
> + * parser_abi_version: version of abi revision of policy generated by parser
> + * kernel_abi_version: version of abi revision for the kernel
> + *
> + * The version info is stored in a single 32 bit version field in the
> + * header portion of each binary policy file.
> + *
> + * policy_version:
> + *   a gross revision number indicating what features and semantics are
> + *   expected by the text policy. This does not necessarily map directly
> + *   to a feature set as a kernel may not have all the supported features
> + *   patched/builtin.
> + *
> + *   policy_version is not supported by kernels that only support v5
> + *   kernel abi, so it will not be written when creating policy for
> + *   those kernels.
> + *
> + * kernel_abi_version:
> + *   should be set to the highest version supported by both the parser and
> + *   the kernel.
> + *   This allows new kernels to detect old userspaces, and new parsers
> + *   to support old kernels and policies semantics.
> + *
> + * parser_abi_version:
> + *   should be bumped when a compiler error or some other event happens
> + *   and policy cache needs to be forced to be recomputed, when the
> + *   policy_version or kernel version has not changed.
> + *
> + *   parser_abi_version is not supported by kernels that only support
> + *   v5 kernel abi so it will not be written when creating policy for those
> + *   kernels.
> + *
> + * Default values set to v5 kernel abi before the different versioning
> + * numbers where supported.
> + */
> +uint32_t policy_version = 2;
> +uint32_t parser_abi_version = 0;
> +uint32_t kernel_abi_version = 5;
> +
> +int force_complain = 0;
>  int perms_create = 0;                   /* perms contain create flag */
>  int net_af_max_override = -1;           /* use kernel to determine af_max */
>  int kernel_load = 1;
> -int kernel_policy_version = 5;               /* default to base version */
>  int kernel_supports_network = 0;        /* kernel supports network rules */
>  int kernel_supports_policydb = 0;    /* kernel supports new policydb */
>  int kernel_supports_mount = 0;               /* kernel supports mount rules 
> */
> --- 2.9-test.orig/parser/parser_interface.c
> +++ 2.9-test/parser/parser_interface.c
> @@ -40,23 +40,10 @@
>  #include <libintl.h>
>  #define _(s) gettext(s)
>  
> -#define u8  unsigned char
> -#define u16 uint16_t
> -#define u32 uint32_t
> -#define u64 uint64_t
>  
>  #define BUFFERINC 65536
>  //#define BUFFERINC 16
>  
> -#if __BYTE_ORDER == __BIG_ENDIAN
> -#  define cpu_to_le16(x) ((u16)(bswap_16 ((u16) x)))
> -#  define cpu_to_le32(x) ((u32)(bswap_32 ((u32) x)))
> -#  define cpu_to_le64(x) ((u64)(bswap_64 ((u64) x)))
> -#else
> -#  define cpu_to_le16(x) ((u16)(x))
> -#  define cpu_to_le32(x) ((u32)(x))
> -#  define cpu_to_le64(x) ((u64)(x))
> -#endif
>  
>  #define SD_CODE_SIZE (sizeof(u8))
>  #define SD_STR_LEN (sizeof(u16))
> @@ -680,9 +667,10 @@
>  
>  int sd_serialize_top_profile(sd_serialize *p, Profile *profile)
>  {
> -     int version;
> +     uint32_t version;
>  
> -     version = kernel_policy_version;
> +     version = ENCODE_VERSION(force_complain, policy_version,
> +                              parser_abi_version, kernel_abi_version);
>  
>       if (!sd_write_name(p, "version"))
>               return 0;
> --- 2.9-test.orig/parser/parser_main.c
> +++ 2.9-test/parser/parser_main.c
> @@ -84,7 +84,6 @@
>  char *cacheloc = NULL;
>  
>  /* per-profile settings */
> -int force_complain = 0;
>  
>  static int load_features(const char *name);
>  
> @@ -841,7 +840,7 @@
>       if (strstr(features_string, "file {"))  /* pre policydb is file= */
>               kernel_supports_policydb = 1;
>       if (strstr(features_string, "v6"))
> -             kernel_policy_version = 6;
> +             kernel_abi_version = 6;
>       if (strstr(features_string, "network"))
>               kernel_supports_network = 1;
>       if (strstr(features_string, "mount"))
> @@ -941,6 +940,40 @@
>       return rc;
>  }
>  
> +#if __BYTE_ORDER == __BIG_ENDIAN
> +#  define le16_to_cpu(x) ((uint16_t)(bswap_16 (*(uint16_t *) x)))
> +#else
> +#  define le16_to_cpu(x) (*(uint16_t *)(x))
> +#endif
> +
> +const char header_string[] = "\004\010\000version\000\002";
> +#define HEADER_STRING_SIZE 12
> +static bool valid_cached_file_version(const char *cachename)
> +{
> +     char buffer[16];
> +     FILE *f;
> +     if (!(f = fopen(cachename, "r"))) {
> +             PERROR(_("Error: Could not read cache file '%s', 
> skipping...\n"), cachename);
> +             return false;
> +     }
> +     size_t res = fread(buffer, 1, 16, f);
> +     fclose(f);
> +     if (res < 16)
> +             return false;
> +
> +     /* 12 byte header that is always the same and then 4 byte version # */
> +     if (memcmp(buffer, header_string, HEADER_STRING_SIZE) != 0)
> +             return false;
> +
> +     uint32_t version = cpu_to_le32(ENCODE_VERSION(force_complain,
> +                                                   policy_version,
> +                                                   parser_abi_version,
> +                                                   kernel_abi_version));
> +     if (memcmp(buffer + 12, &version, 4) != 0)
> +             return false;
> +
> +     return true;
> +}
>  
>  /* returns true if time is more recent than mru_tstamp */
>  #define mru_t_cmp(a) \
> @@ -1045,7 +1078,8 @@
>               /* Load a binary cache if it exists and is newest */
>               if (!skip_read_cache &&
>                   stat(cachename, &stat_bin) == 0 &&
> -                 stat_bin.st_size > 0 && (mru_t_cmp(stat_bin.st_mtim))) {
> +                 stat_bin.st_size > 0 && (mru_t_cmp(stat_bin.st_mtim)) &&
> +                 valid_cached_file_version(cachename)) {
>                       if (show_cache)
>                               PERROR("Cache hit: %s\n", cachename);
>                       retval = process_binary(option, cachename);
> --- 2.9-test.orig/parser/parser_regex.c
> +++ 2.9-test/parser/parser_regex.c
> @@ -692,7 +692,7 @@
>        */
>  
>       /* note: this activates unix domain sockets mediation on connect */
> -     if (kernel_policy_version > 5 &&
> +     if (kernel_abi_version > 5 &&
>           !prof->policy.rules->add_rule(mediates_file, 0, AA_MAY_READ, 0, 
> dfaflags))
>               goto out;
>       if (kernel_supports_mount &&
> --- 2.9-test.orig/parser/parser_yacc.y
> +++ 2.9-test/parser/parser_yacc.y
> @@ -284,7 +284,10 @@
>                        */
>                       yyerror(_("Profile attachment must begin with a '/'."));
>               prof->flags = $3;
> -             if (force_complain)
> +             if (force_complain && kernel_abi_version == 5)
> +                     /* newer abis encode force complain as part of the
> +                      * header
> +                      */
>                       prof->flags.complain = 1;
>  
>               post_process_file_entries(prof);
> 
> 
> -- 
> AppArmor mailing list
> AppArmor@lists.ubuntu.com
> Modify settings or unsubscribe at: 
> https://lists.ubuntu.com/mailman/listinfo/apparmor
> 

Attachment: signature.asc
Description: Digital signature

-- 
AppArmor mailing list
AppArmor@lists.ubuntu.com
Modify settings or unsubscribe at: 
https://lists.ubuntu.com/mailman/listinfo/apparmor

Reply via email to