[apparmor] [PATCH 1/4] tests: Update the regression tests for v6 policy

John Johansen john.johansen at canonical.com
Wed Mar 26 22:06:10 UTC 2014


On 03/26/2014 10:10 AM, Tyler Hicks wrote:
> On 2014-03-26 12:00:45, Tyler Hicks wrote:
>> From: John Johansen <john.johansen at canonical.com>
>>
>> This updates the regression tests for v6 policy. It refactors the
>> required_features test into a have_features fn, and a new
>> requires_features fn (renamed to catch all instances make sure they
>> where right)
>>
>> The have_features fn is then applied to several test to make them
>> conditionally apply based off of availability of the feature
>> and policy version.
>>
>> Signed-off-by: John Johansen <john.johansen at canonical.com>
> 
> Note that I didn't ack this patch yet. I have a little bit of review
> feedback below.
> 
>> ---
>>  tests/regression/apparmor/dbus_eavesdrop.sh   |  2 +-
>>  tests/regression/apparmor/dbus_message.sh     |  2 +-
>>  tests/regression/apparmor/dbus_service.sh     |  2 +-
>>  tests/regression/apparmor/prologue.inc        | 23 ++++++++++++++++++-----
>>  tests/regression/apparmor/tcp.sh              |  1 +
>>  tests/regression/apparmor/unix_fd_server.sh   | 12 +++++++-----
>>  tests/regression/apparmor/unix_socket_file.sh |  1 +
>>  7 files changed, 30 insertions(+), 13 deletions(-)
>>
>> diff --git a/tests/regression/apparmor/dbus_eavesdrop.sh b/tests/regression/apparmor/dbus_eavesdrop.sh
>> index 8006003..fe26b91 100755
>> --- a/tests/regression/apparmor/dbus_eavesdrop.sh
>> +++ b/tests/regression/apparmor/dbus_eavesdrop.sh
>> @@ -18,7 +18,7 @@ pwd=`cd $pwd ; /bin/pwd`
>>  bin=$pwd
>>  
>>  . $bin/prologue.inc
>> -required_features dbus
>> +requires_features dbus
>>  . $bin/dbus.inc
>>  
>>  args="--session"
>> diff --git a/tests/regression/apparmor/dbus_message.sh b/tests/regression/apparmor/dbus_message.sh
>> index aeefe2a..30b1592 100755
>> --- a/tests/regression/apparmor/dbus_message.sh
>> +++ b/tests/regression/apparmor/dbus_message.sh
>> @@ -18,7 +18,7 @@ pwd=`cd $pwd ; /bin/pwd`
>>  bin=$pwd
>>  
>>  . $bin/prologue.inc
>> -required_features dbus
>> +requires_features dbus
>>  . $bin/dbus.inc
>>  
>>  listnames="--type=method_call --session --name=org.freedesktop.DBus /org/freedesktop/DBus org.freedesktop.DBus.ListNames"
>> diff --git a/tests/regression/apparmor/dbus_service.sh b/tests/regression/apparmor/dbus_service.sh
>> index 8a44a2c..451a661 100755
>> --- a/tests/regression/apparmor/dbus_service.sh
>> +++ b/tests/regression/apparmor/dbus_service.sh
>> @@ -17,7 +17,7 @@ pwd=`cd $pwd ; /bin/pwd`
>>  bin=$pwd
>>  
>>  . $bin/prologue.inc
>> -required_features dbus
>> +requires_features dbus
>>  . $bin/dbus.inc
>>  
>>  service="--$bus --name=$dest $path $iface"
>> diff --git a/tests/regression/apparmor/prologue.inc b/tests/regression/apparmor/prologue.inc
>> index b50d3d5..396d203 100755
>> --- a/tests/regression/apparmor/prologue.inc
>> +++ b/tests/regression/apparmor/prologue.inc
>> @@ -21,19 +21,32 @@
>>  #
>>  # For this file, functions are first, entry point code is at end, see "MAIN"
>>  
>> -required_features()
>> +#use $() to retreive the failure message or "true" if success
>> +have_features()
>>  {
>>  	if [ ! -e "/sys/kernel/security/apparmor/features/" ] ; then
>> -		echo "Kernel feature masks not supported. Skipping tests ..."
>> -		exit 0
>> +		echo "Kernel feature masks not supported."
>> +		return 1;
>>  	fi
>>  
>>  	for f in $@ ; do
>>  		if [ ! -e "/sys/kernel/security/apparmor/features/$f" ] ; then
>> -			echo "Required feature $f not available. Skipping tests ..."
>> -			exit 0
>> +			echo "Required feature '$f' not available."
>> +			return 2;
>>  		fi
>>  	done
>> +
>> +	echo "true"
> 
> Not a blocker by any means, but I thought it was odd to echo "true" upon
> success... (continued below)
> 
so yes a little odd, I can't remember all the reasons for doing it this
way but a couple reasons are it allows for this

  if [ $(have_features $@) != "true" ]

while at the same time allowing

  msg=$(have_features $@)
  if [ $msg != "true" ] ; then
     do something with message

of course you can also map message strings to different error codes


I don't really care, either way

>> +	return 0;
>> +}
>> +
>> +requires_features()
>> +{
>> +	local res=$(have_features $@)
>> +	if [ "$res" != "true" ] ; then
> 
> ... because it seems like checking for a 0 return code would be cleaner.
> 
> This is just a silly test suite function, so I don't care much either
> way. Its just an observation.
> 
yeah it probably would be, though IIRC I didn't want the error message
output for every invocation of the fn. But we could get that by
outputting the message and returning 0.  So we can test for the return
code only when needed, and also use the message return

>> +		echo "$res. Skipping tests ..."
>> +		exit 0
>> +	fi
>>  }
>>  
>>  requires_query_interface()
>> diff --git a/tests/regression/apparmor/tcp.sh b/tests/regression/apparmor/tcp.sh
>> index f1c884d..73eff1b 100755
>> --- a/tests/regression/apparmor/tcp.sh
>> +++ b/tests/regression/apparmor/tcp.sh
>> @@ -15,6 +15,7 @@ pwd=`cd $pwd ; /bin/pwd`
>>  bin=$pwd
>>  
>>  . $bin/prologue.inc
>> +requires_features network
>>  
>>  port=34567
>>  ip="127.0.0.1"
>> diff --git a/tests/regression/apparmor/unix_fd_server.sh b/tests/regression/apparmor/unix_fd_server.sh
>> index 4de3b26..6bc5158 100755
>> --- a/tests/regression/apparmor/unix_fd_server.sh
>> +++ b/tests/regression/apparmor/unix_fd_server.sh
>> @@ -132,10 +132,12 @@ runchecktest "fd passing; confined -> confined (no perm)" fail $file $socket $fd
>>  sleep 1
>>  rm -f ${socket}
>>  
>> -# FAIL - confined client, no access to the socket file
>> +if [ "$(have_features policy/versions/v6)" == "true" ] ; then
> 
> See below
> 
>> +    # FAIL - confined client, no access to the socket file
>>  
>> -genprofile $file:$okperm $socket:rw $fd_client:px -- image=$fd_client $file:$okperm
>> -runchecktest "fd passing; confined client w/o socket access" fail $file $socket $fd_client
>> +    genprofile $file:$okperm $socket:rw $fd_client:px -- image=$fd_client $file:$okperm
>> +    runchecktest "fd passing; confined client w/o socket access" fail $file $socket $fd_client
>>  
>> -sleep 1
>> -rm -f ${socket}
>> +    sleep 1
>> +    rm -f ${socket}
>> +fi
>> diff --git a/tests/regression/apparmor/unix_socket_file.sh b/tests/regression/apparmor/unix_socket_file.sh
>> index 6f38acb..dbb923c 100755
>> --- a/tests/regression/apparmor/unix_socket_file.sh
>> +++ b/tests/regression/apparmor/unix_socket_file.sh
>> @@ -27,6 +27,7 @@ pwd=`cd $pwd ; /bin/pwd`
>>  bin=$pwd
>>  
>>  . $bin/prologue.inc
>> +requires_features policy/versions/v6
> 
> My current Ubuntu Trusty kernel and the several previous release kernels
> don't have $aafs/features/policy/versions/v6, or the versions/
> directory at all, but these tests ran just fine. With this change, these
> tests will be skipped on those kernels.
> 
> What's the point of this v6 check and what kernels have
> $aafs/features/policy/versions/v6?
> 
right so this is the mediate unix sockets on connect behavior. This was added
in Saucy but done poorly and there was no way to auto detect between old and
new semantics.

This lead to problems for the backport kernels, and chroots, lxc, running older
userspaces on a newer kernel.

To fix this we introduced extra versioning. This appears in the ipc test kernels
in the dbus-deb ppa.

So older userspaces that don't understand the v6 semanitc, or newer userspaces
run on older kernels, work with the old v5 semantic.

This does lead to one unfortunate case of a trusty kernel on saucy will fail
open. That is it does not enforce the mediate on unix socket behavior that
saucy did (because the userspace does not provide anything to the kernel to
be able to determine it should enforce the new semantic). This should be
acceptable because that combination is not supported, and saucy will go out
of support entirely soon. Making this work for precise, trusty and forward
was far more important.


> Tyle
> 
>>  
>>  client=$bin/unix_socket_file_client
>>  socket=${tmpdir}/unix_socket_file.sock
>> -- 
>> 1.9.1
>>
>>
>> -- 
>> AppArmor mailing list
>> AppArmor at lists.ubuntu.com
>> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor




More information about the AppArmor mailing list