[apparmor] [patch] Add debug info to profile_storage()

[John Johansen]

On 10/21/2015 01:53 PM, Christian Boltz wrote:
> Hello,
> 
> Am Dienstag, 20. Oktober 2015 schrieb John Johansen:
>> On 07/20/2015 12:22 PM, Christian Boltz wrote:
>>> for debugging, it's helpful to know which part of the code
>>> initialized a profile_storage and for which profile and hat this
>>> was done.
>>>
>>> This patch adds an 'info' array with that information, adds the
>>> corresponding parameters to profile_storage() and changes the
>>> callers to deliver some useful content.
>>
>> like kshitij, I'm not thrilled with the extra storage, nor do I care
>> for the extra parameters.
>>
>> But I am all for debug, and it has proven helpful to you so
>>
>> Acked-by: John Johansen <john.johansen at canonical.com>
> 
> As I already said on IRC yesterday, the patch doesn't apply cleanly 
> anymore on the latest code, and the changes are big enough for a fresh 
> review ;-)  (even if the basics of the patch are still the same)
> 
> So here's an updated patch - changes since v1:
> - document 'filename' and 'info' in profile_storage()
> - update a code section that was added in the meantime (around 
>   required_hats)
> - do not include the filename in the details parameter because we 
>   already have it in 'filename'
> - rename 'details' to 'calledby'
> 
> Also note that I changed the patch filename to 01-* (was 81-*) ;-)

Acked-by: John Johansen <john.johansen at canonical.com>

> 
> 
> [ 01-profile_storage-debug-info.diff ]
> 
> === modified file ./utils/apparmor/aa.py
> --- utils/apparmor/aa.py        2015-10-20 23:43:11.058010000 +0200
> +++ utils/apparmor/aa.py        2015-10-21 22:23:55.538056796 +0200
> @@ -443,12 +443,12 @@
>          return {local_profile: extras[local_profile]}
>      return dict()
>  
> -def profile_storage():
> +def profile_storage(profilename, hat, calledby):
>      # keys used in aa[profile][hat]:
>      # a) rules (as dict): alias, include, lvar
>      # b) rules (as hasher): allow, deny
>      # c) one for each rule class
> -    # d) other: external, flags, name, profile, attachment, initial_comment,
> +    # d) other: external, flags, name, profile, attachment, initial_comment, filename, info,
>      #           profile_keyword, header_comment (these two are currently only set by set_profile_flags())
>  
>      # Note that this function doesn't explicitely init all those keys (yet).
> @@ -456,6 +456,9 @@
>  
>      profile = hasher()
>  
> +    # profile['info'] isn't used anywhere, but can be helpful in debugging.
> +    profile['info'] = {'profile': profilename, 'hat': hat, 'calledby': calledby}
> +
>      profile['capability']       = CapabilityRuleset()
>      profile['change_profile']   = ChangeProfileRuleset()
>      profile['network']          = NetworkRuleset()
> @@ -472,7 +475,7 @@
>  
>  def create_new_profile(localfile, is_stub=False):
>      local_profile = hasher()
> -    local_profile[localfile] = profile_storage()
> +    local_profile[localfile] = profile_storage('NEW', localfile, 'create_new_profile()')
>      local_profile[localfile]['flags'] = 'complain'
>      local_profile[localfile]['include']['abstractions/base'] = 1
>  
> @@ -504,7 +507,7 @@
>          if re.search(hatglob, localfile):
>              for hat in sorted(cfg['required_hats'][hatglob].split()):
>                  if not local_profile.get(hat, False):
> -                    local_profile[hat] = profile_storage()
> +                    local_profile[hat] = profile_storage('NEW', hat, 'create_new_profile() required_hats')
>                  local_profile[hat]['flags'] = 'complain'
>  
>      if not is_stub:
> @@ -1491,7 +1494,7 @@
>                              if ynans == 'y':
>                                  hat = exec_target
>                                  if not aa[profile].get(hat, False):
> -                                    aa[profile][hat] = profile_storage()
> +                                    aa[profile][hat] = profile_storage(profile, hat, 'handle_children()')
>                                  aa[profile][hat]['profile'] = True
>  
>                                  if profile != hat:
> @@ -1614,7 +1617,7 @@
>                  hats = [profile] + hats
>  
>              for hat in hats:
> -                log_obj[profile][hat] = profile_storage()
> +                log_obj[profile][hat] = profile_storage(profile, hat, 'ask_the_questions()')
>  
>                  for capability in sorted(log_dict[aamode][profile][hat]['capability'].keys()):
>                      capability_obj = CapabilityRule(capability, log_event=aamode)
> @@ -2602,7 +2605,7 @@
>      if do_include:
>          profile = file
>          hat = file
> -        profile_data[profile][hat] = profile_storage()
> +        profile_data[profile][hat] = profile_storage(profile, hat, 'parse_profile_data() do_include')
>          profile_data[profile][hat]['filename'] = file
>  
>      for lineno, line in enumerate(data):
> @@ -2621,7 +2624,7 @@
>                  raise AppArmorException('Profile %(profile)s defined twice in %(file)s, last found in line %(line)s' %
>                      { 'file': file, 'line': lineno + 1, 'profile': combine_name(profile, hat) })
>  
> -            profile_data[profile][hat] = profile_storage()
> +            profile_data[profile][hat] = profile_storage(profile, hat, 'parse_profile_data() profile_start')
>  
>              if attachment:
>                  profile_data[profile][hat]['attachment'] = attachment
> @@ -3026,7 +3029,7 @@
>              # if hat is already known, the filelist check some lines below will error out.
>              # nevertheless, just to be sure, don't overwrite existing profile_data.
>              if not profile_data[profile].get(hat, False):
> -                profile_data[profile][hat] = profile_storage()
> +                profile_data[profile][hat] = profile_storage(profile, hat, 'parse_profile_data() hat_def')
>                  profile_data[profile][hat]['filename'] = file
>  
>              flags = matches.group('flags')
> @@ -3076,7 +3079,7 @@
>                  if re.search(hatglob, parsed_prof):
>                      for hat in cfg['required_hats'][hatglob].split():
>                          if not profile_data[parsed_prof].get(hat, False):
> -                            profile_data[parsed_prof][hat] = profile_storage()
> +                            profile_data[parsed_prof][hat] = profile_storage(parsed_prof, hat, 'parse_profile_data() required_hats')
>  
>      # End of file reached but we're stuck in a profile
>      if profile and not do_include:
> 
> 
> Regards,
> 
> Christian Boltz
>