[apparmor] [PATCH 16/31] parser: Move match file handling to separate file

John Johansen john.johansen at canonical.com
Tue Jan 27 18:12:01 UTC 2015


On 01/22/2015 07:34 PM, Tyler Hicks wrote:
> On 2015-01-22 10:17:26, John Johansen wrote:
>> On 12/05/2014 04:22 PM, Tyler Hicks wrote:
>>> In addition, define MATCH_STRING_SIZE macro instead of using a magic
>>> number.
>>>
>> err, why? The match file is just the old (deprecated) way of specifying
>> what features are available in the kernel.
>>
>> I really don't see a reason to separate it from features, in fact worse
>> than that I want to hide the existence of match and not have it be
>> part of a public api because it has never been upstream nor will it.
> 
> I was not aware of the history of the match file. Was this an Ubuntu
> thing, SUSE thing, or more widespread than a single distro?
> 
So it is an apparmor pre upstreaming thing, and suse and ubuntu have
carried patches to add it back in. Unfortunately the parser and maybe
tools where relying on it until a couple of years ago. If that had
been fixed correctly during upstreaming we could have just ripped it
out today.

> Separating it out seemed like the right thing to do because I thought
> that the parser was the only thing that would ever need to worry about
> it. For example, systemd would not need to use the aa_match API.
> 
> However, from what you've explained, it makes sense to hide it behind
> the aa_features API. If the match file support was specific to a single
> distro, it may even make sense for that distro to carry a patch to
> libapparmor to support it. You tell me what to do here and I'll do it.
> 
sadly it isn't single distro. I think some one needs to spend a little
bit of time to nail down exactly what breaks with out this. I know I
tried dropping from Ubuntu kernels a few years ago and ran into issues
and had to revert, but I need to investigate, especially the suse end
of things.

We may be at a point where we can just drop it. If not I am leaning
towards just keeping it internal to the parser. If this is the case
and it takes less work we can just keep this api change and not expose
it in the library.


> Tyler
> 
>>
>>
>>> Signed-off-by: Tyler Hicks <tyhicks at canonical.com>
>>> ---
>>>  parser/Makefile      | 10 +++++++---
>>>  parser/features.c    | 19 -------------------
>>>  parser/features.h    |  1 -
>>>  parser/match.c       | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>>>  parser/match.h       | 26 ++++++++++++++++++++++++++
>>>  parser/parser_main.c |  1 +
>>>  6 files changed, 80 insertions(+), 23 deletions(-)
>>>  create mode 100644 parser/match.c
>>>  create mode 100644 parser/match.h
>>>
>>> diff --git a/parser/Makefile b/parser/Makefile
>>> index c50398f..7f2a532 100644
>>> --- a/parser/Makefile
>>> +++ b/parser/Makefile
>>> @@ -81,10 +81,11 @@ SRCS = parser_common.c parser_include.c parser_interface.c parser_lex.c \
>>>         parser_yacc.c parser_regex.c parser_variable.c parser_policy.c \
>>>         parser_alias.c common_optarg.c lib.c network.c \
>>>         mount.cc dbus.cc profile.cc rule.cc signal.cc ptrace.cc \
>>> -       af_rule.cc af_unix.cc features.c policy_cache.c kernel_interface.c
>>> +       af_rule.cc af_unix.cc features.c policy_cache.c kernel_interface.c \
>>> +       match.c
>>>  HDRS = parser.h parser_include.h immunix.h mount.h dbus.h lib.h profile.h \
>>>         rule.h common_optarg.h signal.h ptrace.h network.h af_rule.h af_unix.h \
>>> -       features.h policy_cache.h kernel_interface.h
>>> +       features.h policy_cache.h kernel_interface.h match.h
>>>  TOOLS = apparmor_parser
>>>  
>>>  OBJECTS = $(patsubst %.cc, %.o, $(SRCS:.c=.o))
>>> @@ -243,7 +244,10 @@ mount.o: mount.cc mount.h parser.h immunix.h rule.h
>>>  common_optarg.o: common_optarg.c common_optarg.h parser.h libapparmor_re/apparmor_re.h
>>>  	$(CXX) $(EXTRA_CFLAGS) -c -o $@ $<
>>>  
>>> -features.o: features.c features.h parser.h libapparmor_re/apparmor_re.h
>>> +features.o: features.c features.h parser.h match.h libapparmor_re/apparmor_re.h
>>> +	$(CXX) $(EXTRA_CFLAGS) -c -o $@ $<
>>> +
>>> +match.o: match.c match.h parser.h
>>>  	$(CXX) $(EXTRA_CFLAGS) -c -o $@ $<
>>>  
>>>  policy_cache.o: policy_cache.c policy_cache.h parser.h features.h
>>> diff --git a/parser/features.c b/parser/features.c
>>> index d195e2b..1adeba9 100644
>>> --- a/parser/features.c
>>> +++ b/parser/features.c
>>> @@ -166,22 +166,3 @@ int load_features(const char *name)
>>>  
>>>  	return 0;
>>>  }
>>> -
>>> -void set_features_by_match_file(void)
>>> -{
>>> -	autofclose FILE *ms = fopen(MATCH_FILE, "r");
>>> -	if (ms) {
>>> -		autofree char *match_string = (char *) malloc(1000);
>>> -		if (!match_string)
>>> -			goto no_match;
>>> -		if (!fgets(match_string, 1000, ms))
>>> -			goto no_match;
>>> -		if (strstr(match_string, " perms=c"))
>>> -			perms_create = 1;
>>> -		kernel_supports_network = 1;
>>> -		return;
>>> -	}
>>> -
>>> -no_match:
>>> -	perms_create = 1;
>>> -}
>>> diff --git a/parser/features.h b/parser/features.h
>>> index f0d9adc..aaa4229 100644
>>> --- a/parser/features.h
>>> +++ b/parser/features.h
>>> @@ -21,7 +21,6 @@
>>>  
>>>  #include "parser.h"
>>>  
>>> -#define MATCH_FILE "/sys/kernel/security/" MODULE_NAME "/matching"
>>>  #define FEATURES_FILE "/sys/kernel/security/" MODULE_NAME "/features"
>>>  
>>>  extern char *features_string;
>>> diff --git a/parser/match.c b/parser/match.c
>>> new file mode 100644
>>> index 0000000..770d548
>>> --- /dev/null
>>> +++ b/parser/match.c
>>> @@ -0,0 +1,46 @@
>>> +/*
>>> + *   Copyright (c) 2014
>>> + *   Canonical, Ltd. (All rights reserved)
>>> + *
>>> + *   This program is free software; you can redistribute it and/or
>>> + *   modify it under the terms of version 2 of the GNU General Public
>>> + *   License published by the Free Software Foundation.
>>> + *
>>> + *   This program is distributed in the hope that it will be useful,
>>> + *   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + *   GNU General Public License for more details.
>>> + *
>>> + *   You should have received a copy of the GNU General Public License
>>> + *   along with this program; if not, contact Novell, Inc. or Canonical
>>> + *   Ltd.
>>> + */
>>> +
>>> +#include <stdio.h>
>>> +#include <string.h>
>>> +#include <stdlib.h>
>>> +
>>> +#include "match.h"
>>> +#include "lib.h"
>>> +#include "parser.h"
>>> +
>>> +#define MATCH_STRING_SIZE 1000
>>> +
>>> +void set_features_by_match_file(void)
>>> +{
>>> +	autofclose FILE *ms = fopen(MATCH_FILE, "r");
>>> +	if (ms) {
>>> +		autofree char *match_string = (char *) malloc(MATCH_STRING_SIZE);
>>> +		if (!match_string)
>>> +			goto no_match;
>>> +		if (!fgets(match_string, MATCH_STRING_SIZE, ms))
>>> +			goto no_match;
>>> +		if (strstr(match_string, " perms=c"))
>>> +			perms_create = 1;
>>> +		kernel_supports_network = 1;
>>> +		return;
>>> +	}
>>> +
>>> +no_match:
>>> +	perms_create = 1;
>>> +}
>>> diff --git a/parser/match.h b/parser/match.h
>>> new file mode 100644
>>> index 0000000..deb7c00
>>> --- /dev/null
>>> +++ b/parser/match.h
>>> @@ -0,0 +1,26 @@
>>> +/*
>>> + *   Copyright (c) 2014
>>> + *   Canonical, Ltd. (All rights reserved)
>>> + *
>>> + *   This program is free software; you can redistribute it and/or
>>> + *   modify it under the terms of version 2 of the GNU General Public
>>> + *   License published by the Free Software Foundation.
>>> + *
>>> + *   This program is distributed in the hope that it will be useful,
>>> + *   but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + *   GNU General Public License for more details.
>>> + *
>>> + *   You should have received a copy of the GNU General Public License
>>> + *   along with this program; if not, contact Novell, Inc. or Canonical
>>> + *   Ltd.
>>> + */
>>> +
>>> +#ifndef __AA_MATCH_H
>>> +#define __AA_MATCH_H
>>> +
>>> +#define MATCH_FILE "/sys/kernel/security/" MODULE_NAME "/matching"
>>> +
>>> +void set_features_by_match_file(void);
>>> +
>>> +#endif /* __AA_MATCH_H */
>>> diff --git a/parser/parser_main.c b/parser/parser_main.c
>>> index 4af54a5..2888a1a 100644
>>> --- a/parser/parser_main.c
>>> +++ b/parser/parser_main.c
>>> @@ -41,6 +41,7 @@
>>>  
>>>  #include "lib.h"
>>>  #include "features.h"
>>> +#include "match.h"
>>>  #include "kernel_interface.h"
>>>  #include "parser.h"
>>>  #include "parser_version.h"
>>>
>>
>>
>>





More information about the AppArmor mailing list