Merge from trunk commit rev 3219
In testing against the 4.1 kernel, the syscall_sysctl testcase started
failing even in the unconfined case. What the test program does is
attempt to adjust the kernel.threads-max sysctl to be slightly larger
and see if the operation succeeds by reading the value back out. It
also attempts to save the original value and restore it. The test
was failing because (in VMs at least) the default value chosen by
the kernel for the kernel.threads-max setting was high enough that
attempts to increase it would be ignored (likely to prevent too much
use of kernel memory by threads), helpfully without any message being
report to dmesg. Thus, the initial read of the current value would
succeed, the write of that value + 1024 would appear to succeed,
but then reading the value back out and comparing it to the expected
value would fail, as it would still be the original value, not the
expected new value.
This patch attempts to address this by first attempting to raise
the value, and if that does not appear to work, to then attempt
to lower it. It also refactors the code a bit by creating helper
functions to perform the actual sysctl(2) calls to make the code a
bit easier to read.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
In some cases, the return value of name_to_prof_filename() is undefined.
This happens when deleting the to-be-confined binary while running
aa-genprof and leads to a not-too-helpful
File "/usr/lib/python3/dist-packages/apparmor/aa.py", line 265, in enforce
prof_filename, name = name_to_prof_filename(path)
TypeError: 'NoneType' object is not iterable
(reported by maslen on IRC)
This patch makes sure name_to_prof_filename() always returns None, None
(instead of undefined aka just None) so that at least the caller can
successfully split it into two None values.
For the exotic aa-genprof usecase given above, this at least improves
the error message to
Can't find $binary_name
(raised by enforce() via fatal_error())
The patch also changes fatal_error() to display the traceback first, and
the human-readable message at the end, which makes it more likely that
the user actually notices the human-readable message.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for both trunk and 2.9.
/run/systemd/notify which is needed on systems with systemd
Signed-off-by: Jamie Strandboge <jamie@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
"rcapparmor kill" results in a funny error message:
/lib/apparmor/rc.apparmor.functions: line 441: return: -v: invalid option
return: usage: return [n]
SLE12 includes a patch that prevents this error message, but also
prevents that $? is handed over correctly to rc_status. This means that
"rcapparmor kill" will happily display "done" even with a compiled-in
apparmor module that can't be unloaded.
This patch is the improved version - it adds a small helper function to
set $? (as handed over to aa_log_end_msg()) and then calls rc_status -v.
This means that "rcapparmor kill" now shows "failed" because it's
impossible to unload something that is compiled directly into the
kernel.
References: https://bugzilla.opensuse.org/show_bug.cgi?id=862170 (non-public)
Acked-by: Seth Arnold <seth.arnold@canonical.com> for 2.9 and trunk
Merge from trunk commit 3201
In the commit "Rev 3169: regression tests: have
ptrace use PTRACE_GETREGSET by default", I created
some ifdef magic to use the per arch general purpose
register data structures for various architectures,
including arm64. Unfortunately, in the upstream glibc commit
7d05a8168b
<bits/ptrace.h> is no longer included in the arm64 specific user.h,
which defined the structure as 'struct user_pt_regs'; instead user.h
was converted to define 'struct user_regs_struct'. Because of this,
the ptrace test fails to compile on arm64 when glibc is 2.20 or newer.
This patch adjusts the ptrace test to use the newer structure on arm64
if it's detected that a newer glibc is detected and reverts to using
the older one for older glibcs. It also adds an error when compiling
on architectures that haven't been incorporated yet.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
cux and CUx are valid exec permissions, so they should be accepted
by validate_profile_mode() ;-)
Acked-by: John Johansen <john.johansen@canonical.com> for trunk and 2.9
aa-logprof raises an exception if
- an include file contains a hat
- that file is included in a profile and
- aa-logprof hits an audit log entry for this profile
Reproducer ("works" on 2.9 and trunk):
python3 aa-logprof -f <(echo 'Jun 19 11:50:36 piorun kernel: [4474496.458789] audit: type=1400 audit(1434707436.696:153): apparmor="DENIED" operation="open" profile="/usr/sbin/apache2" name="/etc/gai.conf" pid=2910 comm="apache2" requested_mask="r" denied_mask="r" fsuid=0 ouid=0') -d ../profiles/apparmor.d/
This happens because profiles/apparmor.d/apache2.d/phpsysinfo was
already read when pre-loading the include files.
This patch changes aa.py parse_profile_data() to only raise the
exception if it is not handling includes currently.
Acked-by: Steve Beattie <steve@nxnw.org> for both trunk and 2.9.
For some (not yet known) reason, we get file_perm events without
request_mask set, which causes an aa-logprof crash.
Reproducer log entry:
Jun 19 12:00:55 piorun kernel: [4475115.459952] audit: type=1400 audit(1434708055.676:19629): apparmor="ALLOWED" operation="file_perm" profile="/usr/sbin/apache2" pid=3512 comm="apache2" laddr=::ffff:193.0.236.159 lport=80 faddr=::ffff:192.168.103.80 fport=61985 family="inet6" sock_type="stream" protocol=6
This patch changes logparser.py to ignore those events.
References: https://bugs.launchpad.net/apparmor/+bug/1466812/
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
According to the parser test profiles (which are the only
"documentation" I found about this), definition of boolean variables
is only allowed outside profiles, not inside them.
parse_profile_data() got it the wrong way round, therefore this patch
fixes the condition and updates the error message.
Acked-by: Steve Beattie <steve@nxnw.org> for both trunk and 2.9.
Ignore include files that were not read before (= don't exist in
include[], which typically happens for #include <directory>) so that
the profile_known_*() functions don't crash.
Note: Since the 2.9 code is too different, this patch only avoids the
crash, but doesn't ensure that the files in the included directory are
honored (which would need in a rewrite of the profile_known_*()
functions).
BTW: I tested with a network log entry and hope the best for
profile_known_capability() ;-)
References: https://bugs.launchpad.net/apparmor/+bug/1471425
Acked-by: Steve Beattie <steve@nxnw.org>
The only difference between PROFILE_MODE_RE and PROFILE_MODE_NT_RE
was that the latter one additionally allowed 'x', which looks wrong.
(Standalone 'x' is ok for deny rules, but those are handled by
PROFILE_MODE_DENY_RE.)
This patch completely drops PROFILE_MODE_NT_RE and the related code in
validate_profile_mode().
Also wrap the two remaining regexes in '^(...)+$' instead of doing it
inside validate_profile_mode(). This makes the code more readable and
also results in a 2% performance improvement when parsing profiles.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
Add the missing "pux" to PROFILE_MODE_RE and PROFILE_MODE_NT_RE.
Also move those regexes and PROFILE_MODE_DENY_RE directly above
validate_profile_mode() which is the only user.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Parsing of boolean assignments failed with
TypeError: '_sre.SRE_Match' object is not subscriptable
because of a missing ".groups()"
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Errors include typos ("DESCRIPT__ON"), missing value after #=EXRESULT
and #=EXRESULT=PASS (= instead of space).
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Merge from trunk revision 3169
Bug: https://bugs.launchpad.net/apparmor/+bug/1470985
The ptrace regression test fails to compile on the arm64 platform,
because it uses PTRACE_GETREGS and not the newer PTRACE_GETREGSET
interface for getting access to arch-specific register information[0].
However, fixing it is complicated by the fact that the struct name
for for the general purpose registers is not named consistently
across architectures. This patch attempts to address those issues,
and compiles at least on i386, amd64, arm64, arm (armhf), ppc64,
and ppc64el. The test is verified to continue to function correctly
on i386 and amd64.
[0] https://sourceware.org/ml/archer/2010-q3/msg00193.html
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: John Johansen <john.johansen@canonical.com>
Merge from trunk commit 3159
parser_regex.c includes libapparmor_re/aare_rules.h and thus it
should depend on it in the Makefile.
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
LSMs, such as AppArmor, aren't consulted when a program calls access(2).
This can result in access(2) returning 0 but a subsequent open(2)
failing.
The aa-status utility was doing the access() -> open() sequence and we
became aware of a large number of tracebacks due to open() failing for
lack of permissions. This patch catches any IOError exceptions thrown by
open(). It continues to print the same error message as before when
access() failed but also prints that error message when AppArmor blocks
the open of the apparmorfs profiles file.
https://launchpad.net/bugs/1466768
Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Hat declarations ("^hat,") were added in 2.3 for declaring external
hats, but in the meantime aren't supported by the parser anymore (tested
with 2.9.2 parser).
Additionally, if a profile contains both a hat declaration and the hat
("^hat { ...}"), the hat declaration can overwrite the content of the
hat on a "last one wins" base.
This is caused by setting 'declared' to True, which means write_piece()
will only write the "^hat," line, but not the "^hat { ... }" block.
Therefore no longer set 'declared' to True, print a warning that hat
declarations are no longer supported, and ignore the rule. This also
means that running aa-cleanprof can make the profile valid again :-)
Also no longer change 'hat' when hitting a profile declaration, which
also looks wrong.
Note: This change removes the only usage of 'declared'. A follow-up
patch (trunk only) will completely remove the 'declared' handling.
Reproducer profile (run aa-cleanprof on it):
(will crash in remove_duplicate_rules() 80% of the time - if so, try
multiple times. One of the next patches will fix that. Or just try 2.9,
which doesn't have the crash in remove_duplicate_rules().)
/usr/bin/true {
^FOO {
capability setgid,
}
# deletes the content of ^FOO when saving the profile! (last one wins)
# additionally, the parser says this is invalid syntax
^FOO,
}
See also the "Hat declarations" thread on the ML,
https://lists.ubuntu.com/archives/apparmor/2015-June/008107.html
Acked-by: Kshitij Gupta <kgupta8592@gmail.com> for both 2.9 and trunk.
Currently the cache file has its mtime set at creation time, but this
can lead to cache issues when a policy file is updated separately from
the cache. This makes it possible for an update to ship a policy file
that is newer than the what the cache file was generated from, but
result in a cache hit because the cache file was local compiled after
the policy file was package into an update (this requires the update
to set the mtime of the file when locally installed to the mtime of
the file in its update archive but this is commonly done, especially
in image based updates).
http://bugs.launchpad.net/bugs/1460152
Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Steve Beattie <steve@nxnw.org>
Merged from trunk revision 3117
Some parts of the AppArmor build system don't respect $CPPFLAGS.
The attached patch fixes this.
Acked-by: Steve Beattie <steve@nxnw.org>
The following patch:
- Brings the return to the correct indentation
- Adds a sorted call over the set keys of hat in the profile
Acked-by: Christian Boltz <apparmor@cboltz.de> for trunk and 2.9.
aa-complain, aa-enforce, aa-disable and aa-audit refused to change
profiles for non-existing binaries. This patch also allows paths
starting with /. This also makes it possible to use
aa-complain '/{usr/,}bin/ping'
and
aa-complain /etc/apparmor.d/bin.ping
This patch fixes https://bugs.launchpad.net/apparmor/+bug/1416346
Well, mostly - we still need to decide how we handle wildcards in
profile names:
aa-complain ping
aa-complain /usr/bin/ping
will still error out with "Profile not found" because it isn't an exact
match (and matching the wildcard would change more than the user wants).
Oh, and this patch also fixes the last failure in minitools_test.py.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Add a --no-reload parameter to aa-audit, aa-cleanprof, aa-complain,
aa-disable and aa-enforce. This makes it possible to change the
profile flags without reloading the profile.
Also change tools.py to honor the --no-reload parameter.
References: https://bugs.launchpad.net/apparmor/+bug/1458480
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
When switching the audit flag for network events in aa-logprof
(technically, it happens in aa.py ask_the_question()), the "(I)gnore"
button gets "lost".
This patch fixes the list of available buttons.
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Thanks to a broken optimization (which missed a "not" in the if
condition), aa-mergeprof never asked to merge capability rules.
Also fix a syntax / parameter error uncovered after fixing the
condition.
This patch is only meant for the 2.9 branch.
Acked-by: Steve Beattie <steve@nxnw.org>
Bug: https://launchpad.net/bugs/1210514
It did this in the old 2.8 code, but didn't in 2.9.x (first there was a
broken hat regex, then I commented out the hat handling to avoid
breakage caused by the broken regex).
This patch makes sure the hat flags get set when setting the flags for
the main profile.
Also change RE_PROFILE_HAT_DEF to use more named matches
(leadingwhitespace and hat_keyword). Luckily all code that uses the
regex uses named matches already, which means adding another (...) pair
doesn't hurt.
Finally adjust the tests:
- change _test_set_flags to accept another optional parameter
expected_more_rules (used to specify the expected hat definition)
- add tests for hats (with '^foobar' and 'hat foobar' syntax)
- add tests for child profiles, one of them commented out (see below)
Remaining known issues (also added as TODO notes):
- The hat and child profile flags are *overwritten* with the flags used
for the main profile. (That's well-known behaviour from 2.8 :-/ but we
have more flags now, which makes this more annoying.)
The correct behaviour would be to add or remove the specified flag,
while keeping other flags unchanged.
- Child profiles are not handled/changed if you specify the 'program'
parameter. This means:
- 'aa-complain smbldap-useradd' or 'aa-complain /usr/sbin/smbldap-useradd'
_will not_ change the flags for the nscd child profile
- 'aa-complain /etc/apparmor.d/usr.sbin.smbldap-useradd' _will_ change
the flags for the nscd child profile (and any other profile and
child profile in that file)
Even with those remaining issues (which need bigger changes in
set_profile_flags() and maybe also in the whole flags handling), the
patch improves things and fixes the regression from the 2.8 code.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Bug: https://launchpad.net/bugs/1501913
Samba 4.2 needs some more permissions for nmbd and winbindd.
To avoid overcomplicated profiles, change abstractions/samba to allow
/var/lib/samba/** rwk, (instead of **.tdb rwk) - this change already
fixes the nmbd profile.
winbindd additionally needs some more write permissions in /etc/samba/
(and also in /var/lib/samba/, which is covered by the abstractions/samba
change)
References: https://bugzilla.opensuse.org/show_bug.cgi?id=921098 and
https://bugzilla.opensuse.org/show_bug.cgi?id=923201
Note: this commit differs from trunk r3038 - the 2.9 version keeps the
/var/lib/samba/ lines in the winbindd profile avoid problems if for
some reason abstractions/samba isn't updated (*.rpmnew etc.)
Acked-by: Seth Arnold <seth.arnold@canonical.com>
I noticed "disconnected path" (run/nscd/*) events for ntpd while
updating to the latest openSUSE Tumbleweed.
Acked-by: Seth Arnold <seth.arnold@canonical.com> for trunk and 2.9.
aa-mergeprof failed to fail ;-) when it should raise an AppArmorException.
Instead, it failed with
AttributeError: 'module' object has no attribute 'AppArmorException'
I confirmed this bug in trunk and 2.9.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
(might get re-used later ;-)
Also add two tests for profile names not starting with / - the quoted
version wasn't catched as invalid before, so this change is actually
also a bugfix.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
reported by darix on IRC. This is needed if you have a bigger setup with
dovecot on a different (or multiple) machines
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Ensure nosetests sees all tests in the tests[] tuples. This requires
some name changes because nosetests thinks all function names containing
"test" are tests. (A "not a test" docorator would be an alternative, but
that would require some try/except magic to avoid a dependency on nose.)
To avoid nosetests thinks the functions are a test,
- rename setup_all_tests() to setup_all_loops()
- rename regex_test() to _regex_test() (in test-regex_matches.py)
Also add the module_name as parameter to setup_all_loops and always run
it (not only if __name__ == '__main__').
Known issue: nosetests errors out with
ValueError: no such test method in <class ...>: stub_test
when trying to run a single test generated out of tests[].
(debugging hint: stub_test is the name used in setup_test_loop().)
But that's still an improvement over not seeing those tests at all ;-)
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
Assume you have a profile like
/bin/foo {
/etc/ r,
network,
/usr/ r,
}
(important: there must be be a non-path rule between the two path blocks)
Then run aa-logprof and add another path event. When choosing (V)iew changes,
it will crash with a misleading
File ".../utils/apparmor/aamode.py", line 205, in split_mode
other = mode - user
TypeError: unsupported operand type(s) for -: 'collections.defaultdict' and 'set'
The reason for this is our beloved hasher, which is playing funny games
another time.
The patch wraps the hasher usage with a check for the parent element to
avoid auto-creation of empty childs, which then lead to the above crash.
BTW: This is another issue uncovered by the LibreOffice profile ;-)
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
Merge from trunk revision 3012
Update the postfix-common abstraction to cope with signal and unix
socket mediation, update the access to the sasl library locations
in a multiarch compliant way, and allow access to limited bits
of the filesystem paths under which postfix chroots itself to
(/var/spool/postfix/ on Ubuntu).
Signed-off-by: Steve Beattie <steve@nxnw.org>
Acked-by: Tyler Hicks <tyhicks@canonical.com>
serialize_profile_from_old_profiles() calls store_list_var() with an
empty hasher. This fails for "+=" because in this case store_list_var()
expects a non-empty hasher with the variable already defined, and raises
an exception because of the empty hasher.
This patch sets "correct = False" if a "+=" operation appears, which
means the variable will be written in "clean" mode instead.
Adding proper support for "add to variable" needs big changes (like
storing a variable's "history" - where it was initially defined and what
got added where).
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9.
the LibreOffice profile uncovered that handling of @{var} += is broken:
File ".../utils/apparmor/aa.py", line 3272, in store_list_var
var[list_var] = set(var[list_var] + vlist)
TypeError: unsupported operand type(s) for +: 'set' and 'list'
This patch fixes it:
- change separate_vars() to use and return a set instead of a list
(FYI: separate_vars() is only called by store_list_var())
- adoptstore_list_var() to expect a set
- remove some old comments in these functions
- explain the less-intuitive parameters of store_list_var()
Also add some tests for separate_vars() and store_list_var().
The tests were developed based on the old code, but not all of them
succeed with the old code.
As usual, the tests uncovered some interesting[tm] behaviour in
separate_vars() (see the XXX comments and tell me what the really
expected behaviour is ;-)
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Move the code that does the c -> a and d -> w replacement in denied_mask
and requested_mask so that it only runs for path and exec events, but not
for other events (like dbus and ptrace). The validate_log_mode() and
log_str_to_mode() calls are also moved.
Technically, this means moving code from parse_event() to the path
and exec sections in add_event_to_tree().
This also means aa-logprof no longer crashes if it hits a ptrace or
dbus event in the log.
The "if dmask:" and "if rmask:" checks are removed - if a path event
doesn't have these two, it is totally broken and worth a aa-logprof
crash ;-)
Also adjust the parse_event() tests to expect the "raw" mask instead of
a set.
Note: the 2.9 branch doesn't contain test-capability.py, therefore I
skipped this part of the patch for obvious reasons ;-)
This patch fixes
https://bugs.launchpad.net/apparmor/+bug/1426651 and
https://bugs.launchpad.net/apparmor/+bug/1243932
I manually tested that
- c and d log events are still converted to a and w
- aa-logprof handles exec events correctly
- ptrace events no longer crash aa-logprof
Note: add_event_to_tree() is not covered by tests.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk and 2.9
Merge from trunk revision 3001
Change serialize_parse_profile_start() to use parse_profile_start()
instead of using duplicated code.
The behaviour is mostly kept, with the exception that the function is
more strict now and raises exceptions instead of ignoring errors.
In practise, this won't change anything because the profiles are parsed
with parse_profile() (which calls parse_profile_start()) - and that
already errors out.
The tests are updated to match the more strict behaviour.
The next step would be to drop serialize_parse_profile_start()
completely, but this isn't urgent and can/should be done when we have
test coverage for serialize_profile_from_old_profile() one day ;-)
Acked-by: Steve Beattie <steve@nxnw.org>
Merge from trunk revision 3000
Fix is_skippable_dir() - the regex also matched things like
/etc/apparmor.d/dont_disable, while it should match on the full
directory name.
Also add some tests based on a real-world aa-logprof run (with "print (path)"
in is_skippable_dir()) and some additional "funny"[tm] dirs.
Needless to say that the tests
('dont_disable', False),
('/etc/apparmor.d/cache_foo', False),
will fail with the old is_skippable_dir().
Acked-by: Steve Beattie <steve@nxnw.org>
Merge from trunk revision 2999
Replace RE_PROFILE_START with RE_PROFILE_START_2 and adjust all
code sections that used RE_PROFILE_START_2.
The only real change is that test_get_flags_invalid_01 and
test_get_flags_invalid_02 now expect AppArmorException instead of
AppArmorBug.
Acked-by: Steve Beattie <steve@nxnw.org> for trunk
to use write_header(), and making set_profile_flags
more strict.
Merge from trunk revisions 2996, 2997, and 2998.
Changes in set_profile_flags():
- rewrite set_profile_flags to use parse_profile_start_line() and
write_header().
- replace the silent failure for non-existing files with a proper
exception (using lazy programming - the check is done by removing the
"if os.path.isfile()" check, open_file_read then raises the
exception ;-)
- comment out regex_hat_flag and the code that was supposed to handle
hat flags, which were totally broken. We'll need another patch to fix
it, and we also need to decide if we want to do that because it
introduces a behaviour change (currently, aa-complain etc. don't
change hat flags).
The tests for set_profile_flags() are also updated:
- prepend a space to comments because write_header always adds a space
between '{' and the comment
- remove a test with superfluous quotes that are no longer kept
(that's
just a profile cleanup, so dropping that test is the easiest way)
- update test_set_flags_10 and test_set_flags_12 to use the correct
profile name
- enable the tests for invalid (empty) flags
- update the test for a non-existing file
this patch makes set_profile_flags more strict:
- raise AppArmorBug if newflags contains only whitespace
- raise AppArmorBug if the file doesn't contain the specified profile or
no profile at all
The tests are adjusted to expect AppArmorBug instead of a silent
failure. Also, some tests are added for profile=None, which means to
change the flags for all profiles in a file.
- test_set_flags_08 is now test_set_flags_invalid_04
- test_set_flags_invalid_03 is changed to only contain one reason for
a failure, not two ;-)
Finally implement attachment handling
This patch implements attachment handling - aa-logprof now works with
profiles that have an attachment defined, instead of ignoring audit.log
entries for those profiles.
Changes:
- parse_profile_start_line(): remove workaround that merged the
attachment into the profile name
- parse_profile_data(): store attachment when parsing a profile
- update test_parse_profile_start_03,
test_serialize_parse_profile_start_03,
test_set_flags_nochange_09 and some parse_profile_start_line() tests -
they now expect correct attachment handling
Acked-by: Steve Beattie <steve@nxnw.org>
Merge from trunk revision 2990
Rewrite parse_profile_start() in aa.py to a more readable version.
The behaviour remains unchanged (and is covered by tests).
Acked-by: Kshitij Gupta <kgupta8592@gmail.com>
