Profile name length limitation

Bug #1499544 reported by Tyler Hicks
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
AppArmor
Confirmed
Low
Unassigned
Canonical Click Reviewers tools (obsolete)
Fix Released
Undecided
Jamie Strandboge

Bug Description

The max profile name length is supposed to be (PATH_MAX - 1). However, there seems to be some sort of unintended limitation in place that is restricting it to 253 chars:

$ name=a; while [ $? -eq 0 ]; do prof="profile $name {}"; echo "$prof" | sudo apparmor_parser -qa && echo "$prof" | sudo apparmor_parser -qR && name=${name}a; done; echo "$name" | wc -m
apparmor_parser: Unable to add "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa". Unknown error (36): File name too long
253

That command should result in the value of (PATH_MAX - 1) being printed.

$ apparmor_parser -V
AppArmor parser version 2.9.1
Copyright (C) 1999-2008 Novell Inc.
Copyright 2009-2012 Canonical Ltd.
$ uname -a
Linux boyd 3.19.0-28-generic #30-Ubuntu SMP Mon Aug 31 15:52:51 UTC 2015 x86_64 x86_64 x86_64 GNU/Linux

Tyler Hicks (tyhicks)
description: updated
Revision history for this message
Seth Arnold (seth-arnold) wrote :

We should also verify that profile names of whatever length are accepted in all APIs: aa_change_hat(), aa_change_profile(), etc may have their own limits. And once we sort this out we should clearly document the size limits in apparmor.d(5).

Thanks

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

I'm adding a test to the review tools to make sure that the specified profile name is under the current (incorrect) max. When this bug is fixed, I'll update that test accordingly.

Revision history for this message
John Johansen (jjohansen) wrote :

So one limitation on the name length now, that didn't use to exist it the profile dir name in sysfs. This is going to clamp the name limit down to 255 - the length of the suffix eg.
  name.1

will reduce the name to a max of 253 while,
  name.1024

will reduce it to a max of 250.

The change_profile, change_hat apis have a limit of PAGE_SIZE - HEADER info. Which is is 15 for change_profile and 28 characters for change_hat. The limit for the name in the profile is 65k characters, and the dfa has no hard limit beyond what is allowed by the maximum number of states, which is currently 65k under dfa16, with the dfa32 extension we can support 2^24 states (note the number of characters that can be matched can be more than the maximum number of states due to pattern matching).

Unless we move away from using the profile name as a base for the directory entry in apparmorfs the limit will have to be around 250 characters. This should NOT be a problem as the profile name can be specified separately from the attachment so if an attachment of more than 245 characters is needed then a shorter name could be used and alongside the attachment.

The limit for the attachment is PATH_MAX.

Revision history for this message
John Johansen (jjohansen) wrote :

Please Note: the use of long profile names (anything over 32 chars) is problematic.

There are other interfaces that have smaller limits and standard interfaces/programs that will truncate long names.
Eg. ps -Z will truncate a name if it is too long (note: the get_procattr interface allows for PAGE_SIZE but applications may read less or truncate for display purposes).

the cipso interface has a hard limit of 32 characters, so some form of mangling will have to be used to send info over the that interface.

stacking of profiles can make for extremely long names, so stacking of 10 profiles (possible) with 32 characters will result in a name that is 320 characters long.

The kernel audit buffer is restricted to PAGE_SIZE for all information being audited, so long profile names may result in either some information not being audited (truncated) or the log message failing.

Note: the xattr size limit of 255 for name and 64k for associated data should not be a problem for static labels as the name is short apparmor, and even stacked profile names should fit in 64k.

Revision history for this message
John Johansen (jjohansen) wrote :

Another potential limit is the possible failure in pre audit to generate a stacked labels name. This would happen when the name allocation kmalloc fails because it is too large (under GFP_KERNEL conditions likely larger than a page, GFP_ATOMIC smaller) resulting in the audit being failed and access being denied based on that failure. The name generation could be moved to the merge phase, but that would just result in the failure being pushed earlier and into a GFP_ATOMIC context which has much tighter memory allocation constraints.

The generated name code could be reworked so that it will not allocate but just push the subnames into the audit buffer one at a time. This would work around this limitation, however it still leaves the limitation that a really long name could overwhelm the audit buffer.

Note: due to how auditing is done the stacked name for the task label will usually be audited as individual profiles but the label name of the object MAY be audited as a compound label. This could be reworked as well so that compound names are never audited

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

I'll be adding limits in the review tools so that we can avoid terribly long profile names in the click and snap stores (fyi, the profile name is the APP_ID and the APP_ID is the composition of the pkgname_binary_version. The limit I'm adding to the tools will be much shorter than our current 245 estimate). In practice, snappy and modern clicks will have much shorter names than traditional clicks due to how the store no longer requires reverse domain names for namespacing. In practice, the old long click profile names were pushing 80 characters and there are no reported bugs after almost two years.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

Another technique to consider wrt shortening profile names is to eschew the store's origin, which contributes to the length of the profile name. This would mean a (somewhat convoluted) privacy violation when installing snaps of the same name from different developers. Consider: install app foo from developer bar (today, creates app-specific dirs with foo.bar). Then user uninstalls foo from bar (which leaves data from the previous install) and installs foo from developer baz. If we don't create the app-specific dirs with foo.origin, then in this particular scenario, foo's data from the bar install is available to the baz install.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

To expand on comment #6, this is a comment from the review tools:
        # There are quite a few kernel interfaces that can cause problems with
        # long profile names. These are outlined in
        # https://launchpad.net/bugs/1499544. The big issue is that the audit
        # message must fit within PAGE_SIZE (at least 4096 on supported archs),
        # so long names could push the audit message to be too big, which would
        # result in a denial for that rule (but, only if the rule would've
        # allowed it). Giving a hard-error on maxlen since we know that this
        # will be a problem. The advisory length is what it is since we know
        # that compound labels are sometimes logged and so a snappy system
        # running an app in a snappy container or a QA testbed running apps
        # under LXC
        maxlen = 230 # 245 minus a bit for child profiles
        advlen = 100

maxlen gives an error and advlen a warning in the review tools. There is a corresponding error message to explain how to shorten and in the case of advlen, briefly why. Importantly, exceeding advlen will cause the app to not pass automated reviews so people will be motivated to keep it under this.

We aren't in a position to redefine the APP_ID for snappy and click now, but considering these changes to the review tools, we should be fine when stacking is implemented for the most common scenarios (apps in global namespace, apps running in a container on LXD on snappy or apps running in a container in a QA container test environment).

After evaluating the stacking work and its implications on profile name lengths, we can determine if we should do more than the above (eg, adjusting logging in the kernel, adjusting the APP_ID, etc, etc).

Changed in click-reviewers-tools:
status: New → Fix Committed
assignee: nobody → Jamie Strandboge (jdstrand)
Revision history for this message
Jamie Strandboge (jdstrand) wrote :

An idea for redefining the APP_ID if we think we should go that route would be to use a uuid for the profile name which is 36 characters long. The profiling tools would need to maintain a map between the uuid and the map of course.

Revision history for this message
Jamie Strandboge (jdstrand) wrote :

This was fixed in 0.35.

Changed in click-reviewers-tools:
status: Fix Committed → Fix Released
Christian Boltz (cboltz)
tags: added: aa-kernel aa-parser
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.