Re: [apparmor] private apparmor security bug on public list?
On Tue, Nov 06, 2018 at 01:55:45PM -0600, Tyler Hicks wrote: > On 2018-11-06 20:48:40, Jann Horn wrote: > > I'm subscribed to apparmor@lists.ubuntu.com, and I noticed that I got > > bug mail for https://bugs.launchpad.net/bugs/1800789 via this list > > when the bug was still marked as a security bug. > > The problem looks to be in the bug subscription configuration for the > apparmor-profiles project: > > https://bugs.launchpad.net/apparmor-profiles/+subscriptions > > The ~apparmor-dev team is subscribed which is described here: > > https://launchpad.net/~apparmor-dev > > You can see that the email address listed is the address for this list. > > It is worth noting that the apparmor project itself does not have this > problem: > > https://bugs.launchpad.net/apparmor/+subscriptions It should be noted that in this instance, the messages around https://bugs.launchpad.net/bugs/1800789 were held for moderation, but I mistakenly let them through, not realizing why they had been held for moderation. (In this case, it was not a drastic mistake, as https://gitlab.com/apparmor/apparmor-profiles/issues/3 had already been filed publicly.) -- Steve Beattie http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH profile 1/1] dnsmasq: Add permission to open log files
Hi Petr, On Mon, Oct 08, 2018 at 04:44:01PM +0200, Petr Vorel wrote: > --log-facility option needs to have permission to open files. > Use '*' to allow using more files (for using more dnsmasq instances). > > Signed-off-by: Petr Vorel LGTM as well, I've applied to trunk[1] and cherry-picked back to the older stable releases. Thanks! [1] https://gitlab.com/apparmor/apparmor/commit/025c7dc6a131da24c31e41ad32753015a0ec0f76 -- Steve Beattie http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] AppArmor's new logo
On Fri, Jun 15, 2018 at 02:17:26PM -0700, Seth Arnold wrote: > On Thu, Jun 14, 2018 at 12:38:00PM -0700, John Johansen wrote: > > Also I would like to add a special thanks to Noah Davis who provided the > > designs. > > Thanks Noah! Yes, thank you, Noah, for taking the effort to draft and iterate on the design, very much appreciated! The result looks great! -- Steve Beattie http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [Bug 1669800] Re: Liferea AppArmor profile
Hi Raúl, Sorry your profile slipped through the cracks. I'm going to open this against the apparmor-profiles project, as that's likely a better location for it. Thanks! ** Also affects: apparmor-profiles Importance: Undecided Status: New -- You received this bug notification because you are a member of AppArmor Developers, which is subscribed to AppArmor Profiles. https://bugs.launchpad.net/bugs/1669800 Title: Liferea AppArmor profile Status in AppArmor Profiles: New Status in apparmor package in Ubuntu: Invalid Bug description: Test plan: 1. Load Liferea from the unity launcher 2. Add new RSS feed. 3. Check for updates. 4. Read an entry of a feed. 5. Open entry in tab. 6. Open entry in browser. 7. Open entry in external browser. 8. Close window. 9. Open window from the message notifications. 10. Export OPML RSS feed. 11. Import OPML RSS feed. 12. Close window. ProblemType: Bug DistroRelease: Ubuntu 16.10 Package: apparmor 2.10.95-4ubuntu5.1 ProcVersionSignature: Ubuntu 4.8.0-39.42-generic 4.8.17 Uname: Linux 4.8.0-39-generic x86_64 ApportVersion: 2.20.3-0ubuntu8.2 Architecture: amd64 CurrentDesktop: Unity Date: Fri Mar 3 15:14:29 2017 InstallationDate: Installed on 2015-07-31 (580 days ago) InstallationMedia: Ubuntu 14.04.2 LTS "Trusty Tahr" - Release amd64 (20150218.1) ProcKernelCmdline: BOOT_IMAGE=/vmlinuz-4.8.0-39-generic root=/dev/mapper/ubuntu--vg-root ro quiet splash noht SourcePackage: apparmor UpgradeStatus: Upgraded to yakkety on 2016-11-01 (121 days ago) mtime.conffile..etc.apparmor.d.abstractions.base: 2017-02-25T03:27:12.649925 mtime.conffile..etc.apparmor.d.abstractions.ubuntu-browsers.d.java: 2017-02-26T19:05:27.563689 mtime.conffile..etc.apparmor.d.abstractions.ubuntu-browsers.d.ubuntu-integration: 2017-02-20T21:15:58.771432 mtime.conffile..etc.apparmor.d.abstractions.ubuntu-browsers.d.user-files: 2017-02-18T19:33:07.157766 mtime.conffile..etc.apparmor.d.abstractions.ubuntu-unity7-base: 2017-02-23T22:37:50.022685 mtime.conffile..etc.apparmor.d.tunables.xdg-user-dirs: 2017-02-22T02:08:02.499216 To manage notifications about this bug go to: https://bugs.launchpad.net/apparmor-profiles/+bug/1669800/+subscriptions -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [administrivia] git conversion complete; gitlab projects set up
On Sun, Nov 05, 2017 at 12:34:42PM +0100, intrigeri wrote: > Steve Beattie: > > As agreed upon in the last meeting, I've converted the apparmor bzr > > branches to a git repository. I have also pushed that repository and the > > apparmor-profiles git repository to the apparmor project on gitlib. > > Excellent, thanks! > > > I did set up launchpad to mirror the gitlab tree: > > Was the same done for apparmor-profiles? I've pushed changes to GitLab > today but I don't see them mirrored on Launchpad. It was not, but I'll fix that. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [administrivia] git commit message style discussion
[Discussing the immediate issue before opening to the larger topic.] On Thu, Nov 02, 2017 at 01:00:50PM -0700, John Johansen wrote: > > We walked through a merge yesterday with this merge request: > > > > https://gitlab.com/apparmor/apparmor/merge_requests/1 > > > > The audit trail of who merged the code is implicitly present in the > > merge commit. By default, there's no information about who reviewed the > > changes but the merge commit contains a mention of where to find the > > merge request and that page will contain much more info about who > > reviewed which parts of the merge request. > > > That makes the dangerous assumption we keep our infrastructure on gitlab, > and don't endup migrating again (this is the 4 or 5 migration in the > projects history). Indeed[0]. > I would strongly prefer having that information > integral to the commit message. > > > I'm fine with the default merge commit message. I think Steve had an > > issue with the subject line of the default merge commit message. I'll > > let him voice his opposition to it and maybe he'll have a better suggestion. It's part of a bigger discussion that I've been meaning to raise about our commit messages. The issue I have with something like: > Merge branch 'make-variable' into 'master' > > all: Use the MAKE variable > > See merge request apparmor/apparmor!1 is twofold. The first issue is that I often use tools that initially only expose the --one-line subject for commits (e.g. for identifying fixes to cherry-pick back to older releases). So I want as much identifying information in the one-line commit message to distinguish what a specific commit is about. Having it be "merged branch 'some-topic' into 'master'" is kinda wasteful and depends heavily on the person naming the branch to be merged in a sufficiently descriptive manner. (An example: Tyler named his merge branch that added cscope files to the gitignore file as 'csope' -- in a year, would you guess from the name that that merge only affected the gitignore file?) (As an aside, there is I think a cultural difference between bzr tools and git tools: in bzr, when looking at a commit history (whether one-line or not), the individual commits of merged branches are elided or unexpanded, whereas git tools seem to incorporate/expand them. I'm used to and prefer the bzr behavior because if I'm spelunking history, I don't have to do anything to ignore an irrelevant branch, whereas I don't have that option with git without making the tool work on specific revisions. To give an example, I've put up screenshots from bzr qlog, gitg, and tig viewing the apparmor tree at https://imgur.com/a/sUMHs (gitk screenshot elided because it's an even bigger usability disaster.) I will say that gitlab's generated graph is pretty nice: https://gitlab.com/apparmor/apparmor/network/master but even it, like every other git tool I've looked at (caveat, I haven't look at that many) doesn't let you contract or expand branches.) The second issue is related to John's, in that for a merge of a single commit, the above message is... okay, since the actual commit will likely have most of the justification for the merge proposal captured in the commit message itself. For multi-commit merges, I'd prefer to see the explanation/justification for the merge in the merge commit -- think the explanation that goes in message [00/xx] of e.g. a quilt series, Adding the reviewer information there, as John suggested in IRC, would be good, too. > uhmm, no that really fails the migration test To be fair, we failed a bit with the migration from bzr to git, in that particularly for cherry-picked commits pulled to older releases, we used trunk bzr revisions as back-references. With a bunch of manual effort, I might have been able to convert into something universally meaningful. But I agree that going forward, we should try to avoid losing such information/references. Thanks for the discussion. [0] Historical transitions: Immunix internal hosting + bugzilla -> Novell Forge -> Launchpad -> Gitlab. It also marks the fourth VCS used: CVS (private) -> svn (private) -[history break]-> svn (public) -> bzr -> git. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [administrivia] git conversion complete; gitlab projects set up
On Wed, Nov 01, 2017 at 03:46:17PM -0500, Tyler Hicks wrote: > > Am Mittwoch, 1. November 2017, 08:27:12 CET schrieb Steve Beattie: > >> There more work to do to flesh out the above and standardize on some > >> practices around git, but this should let us make progress. > > > > One thing we use for the openSUSE infrastructure salt code is the > > "Protected Branches" feature: > > https://docs.gitlab.com/ce/user/project/protected_branches.html > > > > Protected branches prevent force pushing and deleting a branch, which > > IMHO makes sense for master and the apparmor-* maintenance branches. > > (Ideally we'll never notice that we have that sort of protection, but it > > helps to prevent accidents.) > > This sounds like a very good thing to enable. Agreed, I'll set this up for the apparmor and apparmor-profiles repos. > > That's something time will tell, and it probably also depends on the > > size of the patch. (I'll assume everybody has notifications for new merge > > requests enabled in the gitlab config, right? ;-) > > I recently contributed a fairly complex patch set to a GitHub project > and will assume that it is a similar experience in GitLab in order to > give my opinion here. > > I really enjoyed the web-based merge request workflow and think it can > be an improvement over the mailing list patchset based flow. However, > I'd strongly recommend that we require contributors to: > > 1) Create a merge request > 2) Receive feedback from maintainers > 3) If changes are required, fold changes necessary to address feedback > into the existing patches, rebase, and force push to their merge request > branch. > > #3 is necessary to avoid a bunch of fixup patches that shouldn't be > standalone. It also makes for an bisect-able tree since there's no > broken commits being merged (with separate fixup commits). > > > I also wonder how to handle the Acked-by messages in case we use merge > > requests - while it's possible with git rebase + using the "reword" > > keyword, it means we'll have to force-push to those branches before > > merging them. > > What the maintainer did for the GitHub contribution that I mentioned > above was to merge my pull request into a local branch, interactive > rebase to add his Signed-off-by, and then push the resulting branch to > to the master branch on GitHub. Do you have a pointer to the merge request/commit so that we can see what that ended up looking like in git? -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [administrivia] git conversion complete; gitlab projects set up
Hi, As agreed upon in the last meeting, I've converted the apparmor bzr branches to a git repository. I have also pushed that repository and the apparmor-profiles git repository to the apparmor project on gitlib. The gitlab project: https://gitlab.com/apparmor The apparmor userspace project: https://gitlab.com/apparmor/apparmor The apparmor profiles project: https://gitlab.com/apparmor/apparmor-profiles There more work to do to flesh out the above and standardize on some practices around git, but this should let us make progress. I did set up launchpad to mirror the gitlab tree: https://code.launchpad.net/~apparmor-dev/apparmor/+git/apparmor During the sprint last week, I had cleaned up the pending launchpad merge proposals (I was re-working the bit of one that I had committed the rest of), but a new one came in on Monday, which I'll review tomorrow. All the bzr branches and merge proposals are still available from launchpad at: https://code.launchpad.net/apparmor/+branches https://code.launchpad.net/apparmor/+activereviews Going forward, we should make use of the merge facilities that gitlab provides. Thanks! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [Merge] ~intrigeri/apparmor-profiles/+git/apparmor-profiles:totem-vs-nvidia into apparmor-profiles:master
Review: Approve LGTM. Thanks! -- https://code.launchpad.net/~intrigeri/apparmor-profiles/+git/apparmor-profiles/+merge/332963 Your team AppArmor Developers is subscribed to branch apparmor-profiles:master. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [Merge] ~intrigeri/apparmor-profiles/+git/apparmor-profiles:gnome-3.26 into apparmor-profiles:master
On Thu, Oct 26, 2017 at 09:15:42AM -, intrigeri wrote: > Good news: "Totem → bwrap → totem-video-thumbnailer" now seems to > work just fine with PUx, contrary to how it was last time I tested :) > I think that's because Totem started passing "--chdir /" to bwrap, > and my understanding of bubblewrap.c is that the fallback to cwd = > $HOME only happens when --chdir is not passed. So we now get the > security benefits of bwrap, without relying on it too much to clean > up its environment (that's one of the important things to enforce > the security boundaries bwrap wants to guarantee so I trust it's done > carefully, but still, less trusted code is always good). > > => case closed. -- To be clear, since bwrap is setuid, the kernel is always going to set the flag to filter environment variables, regardless of what the apparmor transition policy is, but the environment variable filtering done by glibc's ld.so is pretty limited; you can see the current list of filtered variables in https://sourceware.org/git/gitweb.cgi?p=glibc.git;a=blob;f=sysdeps/generic/unsecvars.h;hb=HEAD These are notably environment variables that could let an attacker compromise a program that is setuid or otherwise has different privileges than the caller (e.g. AppArmor execution transitions). For example, loading a malicious library via LD_PRELOAD allows code to execute before bwrap ever has a chance to try to filter its environment. Thanks for confirming that things work as expected. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ https://code.launchpad.net/~intrigeri/apparmor-profiles/+git/apparmor-profiles/+merge/332769 Your team AppArmor Developers is subscribed to branch apparmor-profiles:master. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH 2/2] Add JSON interface to UI_Changes
On Thu, Oct 26, 2017 at 01:55:17PM +0200, Christian Boltz wrote: > Hello, > > Am Donnerstag, 26. Oktober 2017, 13:50:20 CEST schrieb Christian Boltz: > > +json_response('changes')["response"] # wait for it to delay > > deletion of difftemp (and ignore response content) > > That's what I get for rewording the comment - s/ it / response / > so v2 of this simple patch is: > > > === modified file 'utils/apparmor/ui.py' > --- utils/apparmor/ui.py2017-10-25 22:36:48 + > +++ utils/apparmor/ui.py2017-10-26 11:52:45 + > @@ -45,7 +45,7 @@ > def set_json_mode(): > global UI_mode > UI_mode = 'json' > -jsonout = {'dialog': 'apparmor-json-version', 'data': '2.13'} > +jsonout = {'dialog': 'apparmor-json-version', 'data': '2.12'} > write_json(jsonout) > > # reads the response on command line for json and verifies the response > @@ -257,7 +257,7 @@ > if UI_mode == 'json': > jsonout = {'dialog': 'changes', 'header':header, 'filename': > difftemp.name} > write_json(jsonout) > -json_response('changes')["response"] # response gets ignored, > therefore not assigning to a variable > +json_response('changes')["response"] # wait for response to delay > deletion of difftemp (and ignore response content) > else: >subprocess.call('less %s' % difftemp.name, shell=True) > difftemp.close() Acked-by: Steve Beattie <st...@nxnw.org>. Thanks! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] utils: stop rewriting shbang lines in setup script
utils: stop rewriting shbang lines in setup script The python setup tools script is set to rewrite the shbang line of scripts installed in ${PREFIX}/bin/ if the PYTHON environment variable is set. Unfortunately, this (a) only covers the aa-easyprof script as the rest are installed in ${PREFIX}/sbin/, and (b) we've deprecated python 2 support, and hardcoded python3 as the interpreter for all of the python scripts in the utils/ directory. The only use for this feature would be if for some reason the utils did not work properly with the default python3 interpreter and a specific version was needed to be set, but I don't think that warrants keeping the extra bit of code complexity around (and indeed, the snippet that does this is forcibly disabled in Debian/Ubuntu). Therefore, drop the shbang rewriting entirely. Signed-off-by: Steve Beattie <st...@nxnw.org> --- utils/python-tools-setup.py |9 + 1 file changed, 1 insertion(+), 8 deletions(-) Index: b/utils/python-tools-setup.py === --- a/utils/python-tools-setup.py +++ b/utils/python-tools-setup.py @@ -41,14 +41,7 @@ class Install(_install, object): self.mkpath(prefix + os.path.dirname(scripts[0])) for s in scripts: f = prefix + s -# If we have a defined python version, use it instead of the system -# default -if 'PYTHON' in os.environ: -lines = open(os.path.basename(s)).readlines() -lines[0] = '#! /usr/bin/env %s\n' % os.environ['PYTHON'] -open(f, 'w').write("".join(lines)) -else: -self.copy_file(os.path.basename(s), f) +self.copy_file(os.path.basename(s), f) configs = ['easyprof/easyprof.conf'] self.mkpath(prefix + "/etc/apparmor") -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [Merge] ~intrigeri/apparmor-profiles/+git/apparmor-profiles:gnome-3.26 into apparmor-profiles:master
Review: Approve Can PUx be used for bwrap instead, to scrub the environment before invoking bubblewrap? Unconfined execution without environment scrubbing (of e.g. LD_LIBRARY_PATH) is really problematic. Otherwise, looks good to me. I'm merging with the following changes - convert bwrap permission to scrub environment variables (PUx) - add "owner @{HOME}/.cache/totem/ rw," to the totem abstraction, to cover the additional rejection Vincas reported. If it turns out bwrap really does need unfiltered environment variables, then please report back and we can adjust. Thanks! -- https://code.launchpad.net/~intrigeri/apparmor-profiles/+git/apparmor-profiles/+merge/332769 Your team AppArmor Developers is subscribed to branch apparmor-profiles:master. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] test git repo
On Sat, Sep 30, 2017 at 07:23:47AM -0500, Goldwyn Rodrigues wrote: > On 09/26/2017 03:26 PM, Steve Beattie wrote: > > I've made available a test apparmor git repository at > > > > https://code.launchpad.net/~sbeattie/apparmor/+git/apparmor > > > > You can git clone it via > > > > git clone https://git.launchpad.net/~sbeattie/apparmor/+git/apparmor > > > > Please feel free to take check out it, as I'd like to cut over > > permanently to git in the next day or two. > > > > I have not yet done the work to make the tarball generation work with > > git and the .bzrignore needs to be converted to .gitignore, but > > otherwise the tree should be complete. > > > > Please let me know if you see any issues with the repo. Thanks! > > Would this be the final location of the repository? No. In the shortest of terms, when we officially cut over, it would appear at https://code.launchpad.net/~apparmor-dev/apparmor/+git/apparmor with the shortname for the repo at https://git.launchpad.net/apparmor (an alias for https://git.launchpad.net/~apparmor-dev/apparmor/+git/apparmor) > I was thinking of putting it in github[1] and using travis-ci[2] > for continuous integration. [The repo does not have to be under my > name. It is just the repo I used for bzr->git bridge.] I am not done > with the travis as yet. > > [1] https://github.com/goldwynr/apparmor > [2] https://travis-ci.org/goldwynr/apparmor Beyond the immediate conversion landing location, I'm supportive of moving code-hosting and merge requests to github or gitlab and the integrated CI environments that they enable. The whole point of converting to git is to make it easier for people to contribute, and choice of code hosting location affects that. > And yes, it will provide mail notifications for pending pull requests > and failed builds which could be directed to mailing lists. To be fair, we currently have some automated builds in launchpad that send failures to this list; I just don't moderate them through because they are currently failing due to build bit rot issues (though there's a legitimate build/test failure that occurs in libapparmor on trunk and 2.11, which is what's blocking me from releasing a 2.11 maintenance release). But the lack of email notifications from launchpad on git commits is an issue, one that I was promised would have been addressed by now. It can be worked around by having a commit hook that pushes to a web service that would send out the email notification, but that strikes me as an excessive hoop to jump through. Thanks! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] test git repo
On Tue, Oct 03, 2017 at 09:11:47AM +0200, intrigeri wrote: > Steve Beattie: > > Beyond the immediate conversion landing location, I'm supportive > > of moving code-hosting and merge requests to github or gitlab and > > the integrated CI environments that they enable. The whole point > > of converting to git is to make it easier for people to contribute, > > and choice of code hosting location affects that. > > Yes. I'm not sure I'll manage to gather enough energy to join the IRC > meeting (after the lng day that will be for me), so I'll share my > thoughts here. Thanks, it's appreciated. > Git is still a second-class citizen in Launchpad, which makes the > contributor experience worse than it could be, and worse than it is on > more opinionated (towards Git) platforms. *I* manage to get around it > mostly thanks to browser bookmarks and history. I doubt it offers > a smooth experience for first-time and pass-by contributors. > > For example: > > 1. On https://code.launchpad.net/~intrigeri I see my bzr branches but >my Git branches are somewhat hidden behind a "View Git >repositories" link. > > 2. The AppArmor Profiles repo was migrated to Git a while ago. This >is great but the only way I manage to find the "Active reviews" >page every time I need is is… a browser bookmark: >https://code.launchpad.net/apparmor-profiles/+activereviews > >I've just spent 5 minutes trying to find a link to it, and >eventually managed to find one to the Git merge requests via >"Code" → "master" → "Branch merges". But the page I'm landing on >does not list bzr merge requests. > >In comparison, in GitLab and GitHub pull requests are one click >away from a project's home page. Yes, all the above is all too true. Launchpad has frankly not had the level of development investment to make drive-by or even semi-frequent contributions via git convenient or easy. Even finding a specific git tree you're looking for on launchpad can be problematic. So to be explicit, I'm not aware of anyone seriously suggesting we stay with Launchpad. What I'd personally rather hear are the pros and cons of maintaining a project on github vs gitlab, because I don't have experience doing so with either service. Most of my personal experience with either has been related to tracking down individual commits to cherry-pick for security updates, along with filing PRs. (I personally have a mild bias against github due to it not being open source and seemingly fostering a culture of harassment and abuse.) > OTOH I guess using Launchpad's bug tracking system makes it vastly > easier to track the status of AppArmor issues upstream / in Ubuntu, > compared to what it would be if we moved bug tracking to another > platform. I don't directly benefit from it myself, but it probably > matters. But perhaps there's nice integration available between > Launchpad's bug tracking and {GitHub,GitLab} Git hosting? There is sadly not, most of the remote bug tracking work done in launchpad was implemented before the rise of github. Really, the only caveat with bug tracking is that we have a non-trivial amount of open issues in Launchpad, that people like Christian have put in a significant amount of work to triage and organize, that will need to be dealt with one way or the other. But it's also a pretty poor outsider experience to have the primary bug reporting location in one place and the rest of the project interaction in another. Thanks again for your feedback! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] test git repo
[Resending as apparently mailman decided to eat the first copy. Apologies if you receive multiple copies.] On Sat, Sep 30, 2017 at 07:23:47AM -0500, Goldwyn Rodrigues wrote: > On 09/26/2017 03:26 PM, Steve Beattie wrote: > > I've made available a test apparmor git repository at > > > > https://code.launchpad.net/~sbeattie/apparmor/+git/apparmor > > > > You can git clone it via > > > > git clone https://git.launchpad.net/~sbeattie/apparmor/+git/apparmor > > > > Please feel free to take check out it, as I'd like to cut over > > permanently to git in the next day or two. > > > > I have not yet done the work to make the tarball generation work with > > git and the .bzrignore needs to be converted to .gitignore, but > > otherwise the tree should be complete. > > > > Please let me know if you see any issues with the repo. Thanks! > > Would this be the final location of the repository? No. In the shortest of terms, when we officially cut over, it would appear at https://code.launchpad.net/~apparmor-dev/apparmor/+git/apparmor with the shortname for the repo at https://git.launchpad.net/apparmor (an alias for https://git.launchpad.net/~apparmor-dev/apparmor/+git/apparmor) > I was thinking of putting it in github[1] and using travis-ci[2] > for continuous integration. [The repo does not have to be under my > name. It is just the repo I used for bzr->git bridge.] I am not done > with the travis as yet. > > [1] https://github.com/goldwynr/apparmor > [2] https://travis-ci.org/goldwynr/apparmor Beyond the immediate conversion landing location, I'm supportive of moving code-hosting and merge requests to github or gitlab and the integrated CI environments that they enable. The whole point of converting to git is to make it easier for people to contribute, and choice of code hosting location affects that. > And yes, it will provide mail notifications for pending pull requests > and failed builds which could be directed to mailing lists. To be fair, we currently have some automated builds in launchpad that send failures to this list; I just don't moderate them through because they are currently failing due to build bit rot issues (though there's a legitimate build/test failure that occurs in libapparmor on trunk and 2.11, which is what's blocking me from releasing a 2.11 maintenance release). But the lack of email notifications from launchpad on git commits is an issue, one that I was promised would have been addressed by now. It can be worked around by having a commit hook that pushes to a web service that would send out the email notification, but that strikes me as an excessive hoop to jump through. Thanks! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] test git repo
On Sat, Sep 30, 2017 at 07:50:56AM +0200, intrigeri wrote: > Steve Beattie: > > Please feel free to take check out it, > > At first glance it looks good! > > I've compared the content of the trees and they are the same (modulo > the last revision in bzr that I guess will be converted before the > final switch). Yes, the conversion was done with a combination of git-remote-bzr and reposurgeon, so everything is reproducible[0], and can incorporate additional commits to the bzr branches until we cut over formally. And indeed, I've regenerated the repo and pushed again to https://code.launchpad.net/~sbeattie/apparmor/+git/apparmor which includes the latest commit/cherry-picks. That said, it's akin to a rebase mostly, so a simple fetch/pull will likely have problems. > One thing I've noticed is that the way changes are backported from > master to older branches (i.e. tons of cherry-picks) makes history > hard to analyze, i.e. it's very hard to tell "what do we have in > master but not in apparmor-2.11". One way we fix that problem in other > projects is to fork topic branches not off master, but off the oldest > maintenance branch the topic branch is a candidate for, and then we > merge the topic branch into all candidate maintenance branches, no > cherry-pick involved, no commit duplication, and history becomes more > useful :) Do you have a smallish example git tree you can point to? I want to make sure it looks nothing like what upstream php does[1], which makes it nearly impossible to tease out how a patch was cherry-picked for a specific newer branch[2], > > as I'd like to cut over permanently to git in the next day or two. > > /me is excited! Thanks a lot for doing this work. I'm glad other people are excited, because this conversion exercise has emphatically reinforced what a colossal disincentive git is for me. Thanks for the feedback. [0] If you're interested, the relevant bits to generating everything are viewable at https://git.launchpad.net/~sbeattie/+git/reposurgeon-working-dirs/tree/apparmor-manual-conversion [1] http://git.php.net/?p=php-src.git [2] For a specific random example: https://bugs.php.net/bug.php?id=74111 aka CVE-2017-12933. Original commit is http://git.php.net/?p=php-src.git;a=commit;h=f8c514ba6b7962a219296a837b2dbc22f749e736 which got applied to the php 5.6 branch and then merged forward onto the php 7.x branches... but possibly as http://git.php.net/?p=php-src.git;a=commit;h=3a25a56a92ac1d0d6028a8ecd32ccf03bcd71ade ? However, doing 'git tag --contains' on f8c514ba6b7962a219296a837b2dbc22f749e736 and 3a25a56a92ac1d0d6028a8ecd32ccf03bcd71ade shows both commits in the 7.0.22 tag... so what actually applies to 7.0? Attempting to use tig to visualize what's happening just leads to nonsense. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] test git repo
Hello, I've made available a test apparmor git repository at https://code.launchpad.net/~sbeattie/apparmor/+git/apparmor You can git clone it via git clone https://git.launchpad.net/~sbeattie/apparmor/+git/apparmor Please feel free to take check out it, as I'd like to cut over permanently to git in the next day or two. I have not yet done the work to make the tarball generation work with git and the .bzrignore needs to be converted to .gitignore, but otherwise the tree should be complete. Please let me know if you see any issues with the repo. Thanks! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [Merge] ~sdeziel/apparmor-profiles/+git/apparmor-profiles:thunderbird-icedove-debian into apparmor-profiles:master
Review: Approve Thanks. I merged this as-is (and appreciate the followup commit that maintained the merged usr where appropriate). I did raise an eyebrow at + # other commonly used locations + /{data,media,mnt,srv}/** r, + owner /{data,media,mnt,srv}/** rw, in that for /srv/ I personally tend to place system service data files there, rather than user data files... but I can see that not being the case for other environments. Also, at some point, we should try to identify if the accesses to /proc/[0-9]* are to its own pid (or likely for the thunderbird crash reporter), for different pids, and use @{pid} and @{pids} accordingly. Thanks again! -- https://code.launchpad.net/~sdeziel/apparmor-profiles/+git/apparmor-profiles/+merge/330183 Your team AppArmor Developers is subscribed to branch apparmor-profiles:master. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [Merge] ~u-d/apparmor-profiles:thunderbird/links into apparmor-profiles:master
Review: Approve Looks good. I went ahead and merged this, even though it was a subset of Simon's branch, just to get the origins/credit correct. I forget to amend my commit message before pushing, but I also copied the changes to the 17.10 branch (which didn't exist when the merge request was originally made :/ ). Thanks again, and my apologies for the delay! -- https://code.launchpad.net/~u-d/apparmor-profiles/+git/apparmor-profiles/+merge/320285 Your team AppArmor Developers is subscribed to branch apparmor-profiles:master. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] merge updated traceroute profile into 2.10 and 2.9
On Tue, Sep 12, 2017 at 06:03:24PM +0200, Christian Boltz wrote: > the updated traceroute profile (especially the /proc/sys/net/ipv4/... > rule) made it only into 2.11 and trunk, but it's also needed in 2.10.x > which is used in openSUSE Leap 42.x. > > I propose to apply this patch to the 2.10 and 2.9 branch. Acked-by: Steve Beattie <st...@nxnw.org> for both branches. Thanks! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [Merge] ~intrigeri/apparmor-profiles/+git/apparmor-profiles:stricter-totem into apparmor-profiles:master
Review: Approve Thanks for your patience! Looks good, merged. -- https://code.launchpad.net/~intrigeri/apparmor-profiles/+git/apparmor-profiles/+merge/310120 Your team AppArmor Developers is subscribed to branch apparmor-profiles:master. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [Merge] ~intrigeri/apparmor-profiles/+git/apparmor-profiles:drop-obsolete-dev-.udev into apparmor-profiles:master
Review: Approve Sorry about the delay. Looks good, merged. Thanks! -- https://code.launchpad.net/~intrigeri/apparmor-profiles/+git/apparmor-profiles/+merge/326731 Your team AppArmor Developers is subscribed to branch apparmor-profiles:master. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [Merge] ~intrigeri/apparmor-profiles/+git/apparmor-profiles:drop-obsolete-dev-.udev into apparmor-profiles:master
Oh, I should note that the path still exists in Ubuntu 14.04 LTS (trusty), but going forward, removing it is fine. -- https://code.launchpad.net/~intrigeri/apparmor-profiles/+git/apparmor-profiles/+merge/326731 Your team AppArmor Developers is subscribed to branch apparmor-profiles:master. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] One-line addition to the `man` profile
On Fri, Aug 18, 2017 at 06:16:37PM -0700, Seth Arnold wrote: > On Thu, Aug 17, 2017 at 10:47:05PM +0300, artiom wrote: > > $ man select > > man: can't open /usr/share/postgresql/9.6/man/man7/SELECT.7.gz: > > Отказано в доступе > > I slightly modified artiom's suggestion. It might not make a difference > with our DFA walking but it'd help regular regex engines. > > Signed-off-by: Seth Arnold <seth.arn...@canonical.com> > > === modified file 'profiles/apparmor/profiles/extras/usr.lib.man-db.man' > --- profiles/apparmor/profiles/extras/usr.lib.man-db.man 2016-12-03 > 09:59:01 + > +++ profiles/apparmor/profiles/extras/usr.lib.man-db.man 2017-08-19 > 01:14:01 + > @@ -61,6 +61,7 @@ >/usr/share/man/** r, >/usr/share/terminfo/** r, >/usr/share/texmf/teTeX/man/** r, > + /usr/share/postgresql/*/man/** rk, > >/var/cache/man/** rk, Hrm, while I'm not opposed to the patch, I'm curious why both postgresql and teTeX have manpages outside of /usr/share/man/ given http://www.pathname.com/fhs/pub/fhs-2.3.html#USRSHAREMANMANUALPAGES -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] libapparmor: fix ptrace regression test failure
libapparmor: fix ptrace regression test failure In http://bazaar.launchpad.net/~apparmor-dev/apparmor/master/revision/3659, a testcase was added that where the expected output file did not match the input source name, cause libapparmor's regression tests to fail: Output doesn't match expected data: --- ./test_multi/ptrace_no_denied_mask.out2017-08-18 16:35:30.0 -0700 +++ ./test_multi/out/ptrace_no_denied_mask.out 2017-08-18 16:35:38.985863094 -0700 @@ -1,5 +1,5 @@ START -File: ptrace_1.in +File: ptrace_no_denied_mask.in Event type: AA_RECORD_DENIED Audit ID: 1495217772.047:4471 Operation: ptrace FAIL: ptrace_no_denied_mask This patch corrects the issue. Nominated for trunk and 2.11 (r3659 was backported there). Signed-off-by: Steve Beattie <st...@nxnw.org> --- libraries/libapparmor/testsuite/test_multi/ptrace_no_denied_mask.out |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: b/libraries/libapparmor/testsuite/test_multi/ptrace_no_denied_mask.out === --- a/libraries/libapparmor/testsuite/test_multi/ptrace_no_denied_mask.out +++ b/libraries/libapparmor/testsuite/test_multi/ptrace_no_denied_mask.out @@ -1,5 +1,5 @@ START -File: ptrace_1.in +File: ptrace_no_denied_mask.in Event type: AA_RECORD_DENIED Audit ID: 1495217772.047:4471 Operation: ptrace -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] Request to merge two small merge requests
Hi Vincas, On Wed, Aug 09, 2017 at 05:30:17PM +0300, Vincas Dargis wrote: > Two merge requests are reviewed by intrigeri (thanks!) and could potentially > be merged: > > https://code.launchpad.net/~talkless/apparmor/fix_traceroute_tcp/+merge/326260 > > https://code.launchpad.net/~talkless/apparmor/fix_user_download_nonlatin/+merge/326259 Thanks for preparing these (and thanks intrigeri for reviewing them)! I've merged them both into trunk and the 2.11 branch. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [Merge] lp:~cameronnemo/apparmor/gnome-abstraction into lp:apparmor
This wasn't really rejected, just superceded by intregeri's merge proposal above, which was accepted and merged. Thanks for working on this! -- https://code.launchpad.net/~cameronnemo/apparmor/gnome-abstraction/+merge/261320 Your team AppArmor Developers is requested to review the proposed merge of lp:~cameronnemo/apparmor/gnome-abstraction into lp:apparmor. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Carry over all autodep-generated rules in handle_children()
On Sun, Jul 16, 2017 at 09:47:50PM +0200, Christian Boltz wrote: > when creating a new child profile, handle_children() did only copy over > include and path rules. While this was correct in the past, path rules > got changed to FileRule in the meantime and were therefore lost. > (In practise, this means the "$binary mr," rule wasn't added to the new > child profile, causing a "superfluous" question in aa-logprof.) > > This patch changes handle_children() to carry over the complete new > child profile instead of only cherry-picking include and path rules. > > > I propose this patch for trunk and 2.11. > Older versions (with path as hasher) are not affected. > > [ 01-handle_children-use-new-profile.diff ] Acked-by: Steve Beattie <st...@nxnw.org> for both. Thanks! > --- utils/apparmor/aa.py2017-07-16 21:28:03.462623472 +0200 > +++ utils/apparmor/aa.py2017-07-16 21:34:08.093205307 +0200 > @@ -1266,24 +1270,16 @@ > if ynans == 'y': > hat = exec_target > if not aa[profile].get(hat, False): > -aa[profile][hat] = > ProfileStorage(profile, hat, 'handle_children()') > +stub_profile = create_new_profile(hat, > True) > +aa[profile][hat] = stub_profile[hat][hat] > + > aa[profile][hat]['profile'] = True > > if profile != hat: > aa[profile][hat]['flags'] = > aa[profile][profile]['flags'] > > -stub_profile = create_new_profile(hat, True) > - > aa[profile][hat]['flags'] = 'complain' > > -aa[profile][hat]['allow']['path'] = hasher() > -if > stub_profile[hat][hat]['allow'].get('path', False): > -aa[profile][hat]['allow']['path'] = > stub_profile[hat][hat]['allow']['path'] > - > -aa[profile][hat]['include'] = hasher() > -if stub_profile[hat][hat].get('include', > False): > -aa[profile][hat]['include'] = > stub_profile[hat][hat]['include'] > - > file_name = aa[profile][profile]['filename'] > > filelist[file_name]['profiles'][profile][hat] = True > > -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [administrivia] AppArmor 2.11 branch created
Hi, This was discussed on IRC, but I wanted to make sure everyone was aware. John went ahead and split off an AppArmor (userspace tools) 2.11 branch: https://code.launchpad.net/~apparmor-dev/apparmor/2.11 This means fixes intended for the 2.11.1 (and future 2.11 maintenance releases) will need to be nominated like the other supported branches. Thanks! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch v3] tests: readdir - test both getdents() and getdents64() if available
On Wed, Apr 05, 2017 at 04:09:15PM -0500, Tyler Hicks wrote: > > +#if defined(SYS_getdents) && defined(SYS_getdents64) > > + if (rc != rc64) { > > + printf("FAIL - getdents and getdents64 returned different > > values: %d vs %d\n", rc, rc64); > > I feel like this should be ERROR instead of FAIL. If we use FAIL here, > the test will "pass" in an expected fail test case when getdents() > returns something different than getdents64(). I don't see a way around > that other than printing a different (unexpected) output than FAIL. > > swap.sh looks to be the only other test using ERROR. prologue.inc > doesn't know about ERROR and handles it by calling testerror(). > > We're getting into extreme corner case territory here. As before, I'm > prepared to declare "good enough". Let me know your thoughts. Hrm, yeah, you're right on that. Though, in the read access not permitted case, we should be getting rejected on the open() call, not the getdents()/getdents64() calls. Anyway, we're probably into over-engineered testcase area here, but what follows is v3 of the patch. Changes since v2: - Convert to passing the expected return value to the readdir test, and only fail if something unexpected is generated. - Change the readdir.sh test to pass the expected return values, and expect a PASS result for all tests. - Add a testcase that confirms that granting write access to a directory does not allow reading the directory's contents. - Add a debug print function to readdir.c and use it in a few cases when DEBUG is defined at compile time. Subject: tests: readdir - test both getdents() and getdents64() if available In commit 3649, Colin King fixed the readdir test build issue where aarch64 only supports getdetns64(), not getdents(). Realistically, however, we want to ensure mediation occurs on both syscalls where they exist. This patch changes the test to attempt performing both versions of getdents(). Because we want to catch the situation where the result of getdents differs from getdents64, we now pass in the expected result. Also add a test to verify that having write access does not grant the ability to read a directory's contents. Bug: https://bugs.launchpad.net/bugs/1674245 Signed-off-by: Steve Beattie <st...@nxnw.org> --- tests/regression/apparmor/readdir.c | 129 --- tests/regression/apparmor/readdir.sh | 21 - 2 files changed, 120 insertions(+), 30 deletions(-) Index: b/tests/regression/apparmor/readdir.c === --- a/tests/regression/apparmor/readdir.c +++ b/tests/regression/apparmor/readdir.c @@ -1,14 +1,20 @@ /* * Copyright (C) 2002-2006 Novell/SUSE + * Copyright (C) 2017 Canonical, Ltd. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as * published by the Free Software Foundation, version 2 of the * License. + * + * We attempt to test both getdents() and getdents64() here, but + * some architectures like aarch64 only support getdents64(). */ #define _GNU_SOURCE #include +#include +#include #include #include #include @@ -18,45 +24,118 @@ #include #include -int main(int argc, char *argv[]) +/* error if neither SYS_getdents or SYS_getdents64 is defined */ +#if !defined(SYS_getdents) && !defined(SYS_getdents64) + #error "Neither SYS_getdents or SYS_getdents64 is defined, something has gone wrong!" +#endif + +/* define DEBUG to enable debugging statements i.e. gcc -DDEBUG */ +#ifdef DEBUG +void pdebug(const char *format, ...) { - int fd; -#if defined(SYS_getdents64) - struct dirent64 dir; + va_list arg; + + va_start(arg, format); + vprintf(format, arg); + va_end(arg); +} #else - struct dirent dir; +void pdebug(const char *format, ...) { return; } #endif - if (argc != 2){ - fprintf(stderr, "usage: %s dir\n", - argv[0]); - return 1; +#ifdef SYS_getdents +int my_readdir(char *dirname) +{ + int fd; + struct dirent dir; + + fd = open(dirname, O_RDONLY, 0); + if (fd == -1) { + pdebug("open failed: %s\n", strerror(errno)); + return errno; } - fd = open(argv[1], O_RDONLY, 0); - if (fd == -1){ - printf("FAIL - open %s\n", strerror(errno)); - return 1; + /* getdents isn't exported by glibc, so must use syscall() */ + if (syscall(SYS_getdents, fd, , sizeof(dir)) == -1){ + pdebug("getdents failed: %s\n", strerror(errno)); + close(fd); + return errno; } - /* - if (fchdir(fd) == -1){ - printf(&qu
[apparmor] [patch v2] tests: readdir - test both getdents() and getdents64() if available
On Tue, Apr 04, 2017 at 03:41:41PM -0500, Tyler Hicks wrote: > I didn't mean to make this simple test improvement turn into something > complex. I'm willing to ack your original patch if you don't see a quick > and easy solution to this issue. Nah, let's do it right. V2 of the patch follows. Changes since v1: - compile error if neither SYS_getdents or SYS_getdents is defined - verify that getdents() and getdents64() return the same value if both are available. - propagate errno from getdents/getdents64 if failure occurs, instead of synthetic error value. (For reviewing, because so much changes, it might just be easier to review readdir.c after applying, rather than the diff itself.) Subject: tests: readdir - test both getdents() and getdents64() if available In commit 3649, Colin King fixed the readdir test build issue where aarch64 only supports getdetns64(), not getdents(). Realistically, however, we want to ensure mediation occurs on both syscalls where they exist. This patch changes the test to attempt performing both versions of getdents(). Also, if both versions exist, return a failure if the result of each differs from the other. Bug: https://bugs.launchpad.net/bugs/1674245 Signed-off-by: Steve Beattie <st...@nxnw.org> --- tests/regression/apparmor/readdir.c | 101 +++- 1 file changed, 76 insertions(+), 25 deletions(-) Index: b/tests/regression/apparmor/readdir.c === --- a/tests/regression/apparmor/readdir.c +++ b/tests/regression/apparmor/readdir.c @@ -1,10 +1,14 @@ /* * Copyright (C) 2002-2006 Novell/SUSE + * Copyright (C) 2017 Canonical, Ltd. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as * published by the Free Software Foundation, version 2 of the * License. + * + * We attempt to test both getdents() and getdents64() here, but + * some architectures like aarch64 only support getdents64(). */ #define _GNU_SOURCE @@ -18,45 +22,92 @@ #include #include -int main(int argc, char *argv[]) +/* error if neither SYS_getdents or SYS_getdents64 is defined */ +#if !defined(SYS_getdents) && !defined(SYS_getdents64) + #error "Neither SYS_getdents or SYS_getdents64 is defined, something has gone wrong!" +#endif + +#ifdef SYS_getdents +int my_readdir(char *dirname) { int fd; -#if defined(SYS_getdents64) - struct dirent64 dir; -#else struct dirent dir; -#endif - if (argc != 2){ - fprintf(stderr, "usage: %s dir\n", - argv[0]); - return 1; + fd = open(dirname, O_RDONLY, 0); + if (fd == -1) { + printf("FAIL - open %s\n", strerror(errno)); + return errno; } - fd = open(argv[1], O_RDONLY, 0); - if (fd == -1){ - printf("FAIL - open %s\n", strerror(errno)); - return 1; + /* getdents isn't exported by glibc, so must use syscall() */ + if (syscall(SYS_getdents, fd, , sizeof(dir)) == -1){ + printf("FAIL - getdents %s\n", strerror(errno)); + close(fd); + return errno; } - /* - if (fchdir(fd) == -1){ - printf("FAIL - fchdir %s\n", strerror(errno)); - return 1; + close(fd); + return 0; +} +#endif + +#ifdef SYS_getdents64 +int my_readdir64(char *dirname) +{ + int fd; + struct dirent64 dir; + + fd = open(dirname, O_RDONLY, 0); + if (fd == -1) { + printf("FAIL - open %s\n", strerror(errno)); + return errno; } - */ /* getdents isn't exported by glibc, so must use syscall() */ -#if defined(SYS_getdents64) - if (syscall(SYS_getdents64, fd, , sizeof(struct dirent64)) == -1){ -#else - if (syscall(SYS_getdents, fd, , sizeof(struct dirent)) == -1){ + if (syscall(SYS_getdents64, fd, , sizeof(dir)) == -1){ + printf("FAIL - getdents64 %s\n", strerror(errno)); + close(fd); + return errno; + } + + close(fd); + return 0; +} #endif - printf("FAIL - getdents %s\n", strerror(errno)); - return 1; + +int main(int argc, char *argv[]) +{ + int rc = 0, rc64 = 0, err = 0; + + if (argc != 2) { + fprintf(stderr, "usage: %s dir\n", + argv[0]); + err = 1; + goto err; + } + +#ifdef SYS_getdents + rc = my_readdir(argv[1]); + err |= rc; +#endif + +#ifdef SYS_getdents64 + rc64 = my_readdir64(argv[1]); + err |= rc64; +#endif + +#if defined(SYS_getdents) && defined(SYS_getdents64) + if (rc != rc64) {
Re: [apparmor] [patch] tests: readdir - test both getdents() and getdents64() if available
Hey Tyler, On Tue, Apr 04, 2017 at 02:03:53PM -0500, Tyler Hicks wrote: > On 04/04/2017 01:14 PM, Steve Beattie wrote: > > -int main(int argc, char *argv[]) > > +#ifdef SYS_getdents > > +int my_readdir(char *dirname) > > Nitpick... > > Please use my_getdents() and my_getdents64() since these are getdents > wrappers and not readdir wrappers. The reason for calling the function my_readdir() and not my_getdents() is the same reason the whole test is called the same; they're emulating the work that the libc readdir() function does. while ensuring that glibc doesn't do anything underneath us that might invalidate our assumptions about the test. But I'm not too chuffed one or the other about the name. > > + rc = my_readdir(argv[1]); > > + if (rc != 0) > > + goto err; > > + > > + rc = my_readdir64(argv[1]); > > + if (rc != 0) > > + goto err; > > Objection... > > In an expected fail test case, getdents64() will not be tested because > getdents() will have already failed. > > The test script should pass the expected behavior to the test program > and the test program will need to verify that both syscalls match the > expected behavior. > > Alternatively, a shortcut would be to test both syscalls and if the > results are different, raise an ERROR instead of a FAIL. Yes, I thought about this a bit before submitting. The tricky situation is cases like aarch64 (aka arm64) where the getdents() syscall doesn't exist at all and the stub function is used instead. The test would need to take this into account, perhaps by having the stub function return -ENOSYS or something, to make the situation detectable. I'll think about this more. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] tests: readdir - test both getdents() and getdents64() if available
Hey Colin, On Tue, Apr 04, 2017 at 03:16:29PM -, Colin Ian King wrote: > Colin Ian King has proposed merging > lp:~colin-king/apparmor/fix-arm64-test-builds into lp:apparmor. > > Requested reviews: > AppArmor Developers (apparmor-dev) > > For more details, see: > https://code.launchpad.net/~colin-king/apparmor/fix-arm64-test-builds/+merge/321876 > > This fixes build issues for the readdir test for arm64 where getdents(2) > is not wired up as a system call but gendents64(2) is available. > This changes the preference to use the 64 bit system call by default > if it is available on 64 bit systems. Thanks for providing this fix. Realastically, though, where an architecture supports both getdents() and getdents64(), we ought to be ensuring that apparmor mediation occurs on both syscall paths. What follows is a patch that attempts to do that. Subject: tests: readdir - test both getdents() and getdents64() if available In commit 3649, Colin King fixed the readdir test build issue where aarch64 only supports getdetns64(), not getdents(). Realistically, however, we want to ensure mediation occurs on both syscalls where they exist. This patch changes the test to attempt performing both versions of getdents(). Bug: https://bugs.launchpad.net/bugs/1674245 Signed-off-by: Steve Beattie <st...@nxnw.org> --- tests/regression/apparmor/readdir.c | 80 ++-- 1 file changed, 59 insertions(+), 21 deletions(-) Index: b/tests/regression/apparmor/readdir.c === --- a/tests/regression/apparmor/readdir.c +++ b/tests/regression/apparmor/readdir.c @@ -1,10 +1,14 @@ /* * Copyright (C) 2002-2006 Novell/SUSE + * Copyright (C) 2017 Canonical, Ltd. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as * published by the Free Software Foundation, version 2 of the * License. + * + * We attempt to test both getdents() and getdents64() here, but + * some architectures like aarch64 only support getdents64(). */ #define _GNU_SOURCE @@ -18,45 +22,79 @@ #include #include -int main(int argc, char *argv[]) +#ifdef SYS_getdents +int my_readdir(char *dirname) { int fd; -#if defined(SYS_getdents64) - struct dirent64 dir; -#else struct dirent dir; -#endif - if (argc != 2){ - fprintf(stderr, "usage: %s dir\n", - argv[0]); + fd = open(dirname, O_RDONLY, 0); + if (fd == -1) { + printf("FAIL - open %s\n", strerror(errno)); return 1; } - fd = open(argv[1], O_RDONLY, 0); - if (fd == -1){ - printf("FAIL - open %s\n", strerror(errno)); + /* getdents isn't exported by glibc, so must use syscall() */ + if (syscall(SYS_getdents, fd, , sizeof(dir)) == -1){ + printf("FAIL - getdents %s\n", strerror(errno)); + close(fd); return 1; } - /* - if (fchdir(fd) == -1){ - printf("FAIL - fchdir %s\n", strerror(errno)); + close(fd); + return 0; +} +#else +int my_readdir(char *dirname) { return 0; } +#endif + +#ifdef SYS_getdents64 +int my_readdir64(char *dirname) +{ + int fd; + struct dirent64 dir; + + fd = open(dirname, O_RDONLY, 0); + if (fd == -1) { + printf("FAIL - open %s\n", strerror(errno)); return 1; } - */ /* getdents isn't exported by glibc, so must use syscall() */ -#if defined(SYS_getdents64) - if (syscall(SYS_getdents64, fd, , sizeof(struct dirent64)) == -1){ + if (syscall(SYS_getdents64, fd, , sizeof(dir)) == -1){ + printf("FAIL - getdents64 %s\n", strerror(errno)); + close(fd); + return 1; + } + + close(fd); + return 0; +} #else - if (syscall(SYS_getdents, fd, , sizeof(struct dirent)) == -1){ +int my_readdir64(char *dirname) { return 0; } #endif - printf("FAIL - getdents %s\n", strerror(errno)); - return 1; + +int main(int argc, char *argv[]) +{ + int rc = 0; + + if (argc != 2) { + fprintf(stderr, "usage: %s dir\n", + argv[0]); + rc = 1; + goto err; } + rc = my_readdir(argv[1]); + if (rc != 0) + goto err; + + rc = my_readdir64(argv[1]); + if (rc != 0) + goto err; + printf("PASS\n"); - return 0; +err: + return rc; } -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] update dovecot-lda profile
On Sun, Apr 02, 2017 at 01:20:52PM +0200, Christian Boltz wrote: > dovecot-lda needs > - the attach_disconnected flags > - read access to /usr/share/dovecot/protocols.d/ > - rw for /run/dovecot/auth-userdb > > References: https://bugs.launchpad.net/bugs/1650827 > > I propose this patch for 2.9, 2.10 and trunk. Acked-by: Steve Beattie <st...@nxnw.org> for all three, though... > [ dovecot-lda-lp1650827.diff ] > > === modified file 'profiles/apparmor.d/usr.lib.dovecot.dovecot-lda' > --- profiles/apparmor.d/usr.lib.dovecot.dovecot-lda 2016-02-20 00:15:20 > + > +++ profiles/apparmor.d/usr.lib.dovecot.dovecot-lda 2017-04-02 10:46:01 > + > @@ -12,7 +12,7 @@ > #include > #include > > -/usr/lib/dovecot/dovecot-lda { > +/usr/lib/dovecot/dovecot-lda flags=(attach_disconnected) { >#include >#include >#include > @@ -26,9 +26,11 @@ >/proc/*/mounts r, >owner /tmp/dovecot.lda.* rw, >/{var/,}run/dovecot/mounts r, > + /run/dovecot/auth-userdb rw, >/usr/bin/doveconf mrix, >/usr/lib/dovecot/dovecot-lda mrix, >/usr/sbin/sendmail Cx, > + /usr/share/dovecot/protocols.d/ r, I'm surprised that there isn't any need to read files in that directory. Unless in this configuration there's nothing within that directory for dovecot-lda specifically. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Remove re.LOCALE flag
On Thu, Feb 16, 2017 at 10:38:30PM +0100, Christian Boltz wrote: > Hello, > > starting with python 3.6, the re.LOCALE flag can only be used with byte > patterns, and errors out if used with str. This patch removes the flag > in get_translated_hotkey(). > > > References: https://bugs.launchpad.net/apparmor/+bug/1661766 > > I propose this patch for trunk, 2.10 and 2.9. > > > [ 01-re-locale.diff ] > > === modified file 'utils/apparmor/ui.py' > --- utils/apparmor/ui.py2016-10-03 19:01:29 + > +++ utils/apparmor/ui.py2017-02-16 21:29:22 + > @@ -64,8 +64,8 @@ > msg = 'PromptUser: ' + _('Invalid hotkey for') > > # Originally (\S) was used but with translations it would not work :( > -if re.search('\((\S+)\)', translated, re.LOCALE): > -return re.search('\((\S+)\)', translated, re.LOCALE).groups()[0] > +if re.search('\((\S+)\)', translated): > +return re.search('\((\S+)\)', translated).groups()[0] > else: > if cmsg: > raise AppArmorException(cmsg) The re.LOCALE flag was introduced in commit 0.1.89; however, the regex change added at the same time was incorrect (\S*) and was replaced with code seen above. So it's not clear to me that the original addition of re.LOCALE actually fixed what it claimed it fixed. I'm assuming you've tested the utils manually in German with the patch applied? If so, Acked-by: Steve Beattie <st...@nxnw.org> for three branches. Thanks. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] regression tests: include complain mode arg in runtest
Hi, When individual regression tests are run, a pseudo script for reproducing the steps of the test are created in the created temporary directory (but is not actually how the individual tests are run by the harness). When support for complain-mode test profiles added, the extra complain flag option was not added to the code that generates the runtest script. This patch fixes the issue. Nominated for 2.11, 2.10, and 2.9. Signed-off-by: Steve Beattie <st...@nxnw.org> --- tests/regression/apparmor/prologue.inc |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: b/tests/regression/apparmor/prologue.inc === --- a/tests/regression/apparmor/prologue.inc +++ b/tests/regression/apparmor/prologue.inc @@ -146,7 +146,7 @@ genrunscript() if [ "$retaintmpdir" = "true" ] then runfile=$tmpdir/runtest - echo "$subdomain ${parser_args} $profile" > $runfile + echo "$subdomain ${parser_args} ${complainflag} $profile" > $runfile echo -n "$testexec " >> $runfile while [ $# -gt 0 ] ; do echo -n "\"$1\" " >> $runfile -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] regression tests: fix environ fail case
Hi, While researching https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1661030 I discovered that when the exec() of the child process, we don't report FAIL to stdout, so the regression tests consider it an error rather than a failure and abort, short-circuiting the test script. This patch fixes this by emitting the FAIL message when the result from the wait() syscall indicates the child process did not succeed. Nominated for 2.11, 2.10, and 2.9. Signed-off-by: Steve Beattie <st...@nxnw.org> --- tests/regression/apparmor/environ.c |2 ++ 1 file changed, 2 insertions(+) Index: b/tests/regression/apparmor/environ.c === --- a/tests/regression/apparmor/environ.c +++ b/tests/regression/apparmor/environ.c @@ -63,6 +63,8 @@ int main(int argc, char *argv[]) if (retval == RET_CHLD_SUCCESS) { printf("PASS\n"); retval = 0; + } else { + printf("FAIL: Child failed\n"); } } else if (pid == 0) { -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [Merge] ~intrigeri/apparmor-profiles/+git/apparmor-profiles:update-pulseaudio into apparmor-profiles:master
Review: Approve Looks good, merged. -- https://code.launchpad.net/~intrigeri/apparmor-profiles/+git/apparmor-profiles/+merge/315293 Your team AppArmor Developers is subscribed to branch apparmor-profiles:master. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Drop unused global variables in aa.py
On Thu, Jan 19, 2017 at 11:24:13PM +0100, Christian Boltz wrote: > grepping through the code shows that running_under_genprof, > unimplemented_warning, ALL, t, seen and skip are unused, so drop them. > > [ 01-unused-global.diff ] > > --- utils/apparmor/aa.py2017-01-18 22:19:23.971405280 +0100 > +++ utils/apparmor/aa.py2017-01-19 23:13:17.063279610 +0100 > @@ -74,8 +74,6 @@ > debug_logger = DebugLogger('aa') > > CONFDIR = '/etc/apparmor' > -running_under_genprof = False > -unimplemented_warning = False > > # The database for severity > sev_db = None > @@ -99,12 +97,7 @@ > # format: user_globs['/foo*'] = AARE('/foo*') > user_globs = {} > > -# The key for representing bare "file," rules > -ALL = '\0ALL' > - > ## Variables used under logprof > -### Were our > -t = hasher() # dict() > transitions = hasher() > > aa = hasher() # Profiles originally in sd, replace by aa > @@ -96,12 +89,10 @@ > log = [] > pid = dict() > > -seen = hasher() # dir() > profile_changes = hasher() > prelog = hasher() > changed = dict() > created = [] > -skip = hasher() > helpers = dict() # Preserve this between passes # was our > ### logprof ends > > @@ -1900,7 +1891,6 @@ > # set up variables for this pass > #t = hasher() Can you kill the t = hasher() comment here, too? Acked-by: Steve Beattie <st...@nxnw.org> with or without that change. Thanks. > #transitions = hasher() > -#seen = hasher() # XXX global? > global log > log = [] > global existing_profiles > @@ -1909,7 +1899,6 @@ > #profile_changes = hasher() > # prelog = hasher() > # changed = dict() > -#skip = hasher() # XXX global? > #filelist = hasher() > > aaui.UI_Info(_('Reading log entries from %s.') % logfile) > > > Regards, > > Christian Boltz > -- > Bitte sende kein "postconf -n", so wie alles es tun. > Das macht es nur unnötig einfach zu helfen. > [Patrick Ben Koetter in postfixbuch-users] > -- > AppArmor mailing list > AppArmor@lists.ubuntu.com > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/apparmor -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] sshd profile: drop local/ include
On Thu, Jan 12, 2017 at 06:32:07PM +0100, Christian Boltz wrote: > the local/ include in the sshd profile in extras causes some trouble: > - it breaks "make check" because the parser can't find the local/ file > - it results in a broken profile if someone uses this profile as > starting point, but doesn't notice it needs the local include > > > Note: This affects only trunk (including 2.11). > > [ sshd-local.diff ] Acked-by: Steve Beattie <st...@nxnw.org>. That said, fixing things so that the local includes are generated and work for the extras profiles would be nice, too. (Same for the profile repo.) -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [2.10] backport abstractions/wayland changes
On Sun, Jan 08, 2017 at 01:18:57PM +0100, Christian Boltz wrote: > while reading bzr log and writing changelogs, I noticed that we added > the initial version of the wayland abstraction to 2.10, but never > backported another commit that adds more paths. > > I hereby propose to backport trunk r3590 to the 2.10 branch. > > revno: 3590 > fixes bug: https://launchpad.net/bugs/1507469 > committer: Seth Arnold <seth.arn...@canonical.com> > branch nick: apparmor > timestamp: Wed 2016-11-30 15:16:32 -0800 > message: > Add more wayland paths, suggested by Simon McVittie in > https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1507469 Acked-by: Steve Beattie <st...@nxnw.org>, Thanks for reviewing and noticing this. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] aa-unconfined man page vs behavior?
On Fri, Dec 30, 2016 at 12:25:15AM -0800, John Johansen wrote: > On 12/29/2016 11:33 PM, Steve Beattie wrote: > > While editing the man page for aa-unconfined in this patch set, I > > noticed that it's uh pretty inaccurate at describing the behavior > > of aa-unconfined. It described listing processes without apparmor > > policies applied, whereas the tool reports processes with and without > > policies applied. > > > > The question is, which way is the preferred way to fix this? Change > > the documentation to accurately reflect the tool's behavior, or adjust > > the tool to more closely reflect the documentation? > > > Well I think the name is really pushing in the direction of only > unconfined. > > Note that it does only report unconfined processes without --paranoid > but with --paranoid it reports both confined and unconfined. That's not the behavior I see... (tip of trunk, without patchset applied) Ubuntu 16.04 LTS: $ sudo ./aa-unconfined 1300 /sbin/rpcbind not confined 1455 /usr/sbin/NetworkManager not confined 1480 /usr/sbin/avahi-daemon confined by '/usr/sbin/avahi-daemon (complain)' 1664 /usr/sbin/dnsmasq confined by '/usr/sbin/dnsmasq (enforce)' 1711 /usr/sbin/sshd not confined 2153 /usr/sbin/openvpn not confined 3019 /usr/sbin/xinetd not confined 3130 /usr/sbin/tcsd not confined 3437 /usr/lib/postfix/sbin/master confined by 'postfix-master (enforce)' 3933 /usr/bin/ssh not confined 22072 /home/steam/.steam/ubuntu12_32/steam not confined 26822 /usr/sbin/cups-browsed confined by '/usr/sbin/cups-browsed (enforce) and Ubuntu 14.04 LTS: $ sudo ./aa-unconfined 1091 /sbin/rpcbind not confined 1196 /sbin/rpc.statd not confined 1965 /usr/sbin/avahi-daemon confined by '/usr/sbin/avahi-daemon (enforce)' 2014 /usr/bin/perl (/usr/bin/perl -wT /usr/sbin/munin-node) not confined 2113 /usr/sbin/xinetd not confined 2155 /usr/sbin/sshd not confined 2218 /usr/sbin/cups-browsed confined by '/usr/sbin/cups-browsed (enforce)' 2324 /usr/sbin/dnsmasq confined by '/usr/sbin/dnsmasq (enforce)' 2950 /usr/sbin/ntpd confined by '/usr/sbin/ntpd (enforce)' 3094 /usr/sbin/rpc.mountd not confined 3268 /usr/lib/postfix/master not confined 4183 /usr/bin/mpd not confined 4296 /usr/sbin/apache2 confined by '/usr/sbin/apache2 (enforce)' 8540 /usr/bin/Xvnc4 not confined 13728 /usr/sbin/sshd (sshd: user@pts/4) not confined 28374 /usr/sbin/cupsd confined by '/usr/sbin/cupsd (enforce)' Also, --paranoid reports all processes, not just ones with network sockets. > If we want the other behavior we can add a new tool aa-confined, or > aa-netstat, ..? or some such aa-status? :) But I like the current behavior, both from a "it's comforting to see what I do have confined" perspective as well as a potential fear of asking myself "is the tool reporting nothing because I have everything listening on a network socket confined, or because aa-unconfined is buggy?" if we make the behavior consistent with the documentation. That said, I'm mildly inclined to make it match the documentation (and maybe provide an option to get the old behavior back), but I also fear breaking things for people who might have scripts that parse the output of aa-unconfined. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] utils/aa-unconfined: fix netstat invocation regression
On Fri, Dec 30, 2016 at 07:11:33PM +0100, Christian Boltz wrote: > You can earn the other 50% of the Ack by also fixing the python2 branch > 3 lines above ;-)) (no need to resend the patch) Doh, thanks. Fixed. (This is why I pulled out the commands in the original patch set, so I'd only need to remember to change them once.) -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 1/4] utils/aa-unconfined: fix netstat usage, use ss(8) by default
On Fri, Dec 30, 2016 at 02:54:31PM +0100, Christian Boltz wrote: > Am Donnerstag, 29. Dezember 2016, 23:24:55 CET schrieb Steve Beattie: > > [1] Was a bug filed for this? > > No, just a mail to the ML. That's what I thought, I just wanted to make sure I did reference a bug when committing if one existed. > > [2] In fact, the version of ss/iproute2 in Ubuntu 14.04 LTS does not > > restrict the listings to network sockets when 'ss -nlp --family > > inet' is invoked. > > Nice[tm]. Yeah. Oh, there's one other caveat with ss(8), on Ubuntu 12.04 LTS, the format is different once again and in a way that my patch is not able to parse. But 12.04 is only supported for another 4 months or so, and the option to use netstat is still there... > Some testing shows that aa-unconfined gives different results with ss and > netstat (ss lists more processes). Some digging shows that this seems to > be caused by differences in what netstat and ss reports, so it's not an > error in aa-unconfined. > > The differences on my system are (only listed by ss): > - 2749 /usr/sbin/wpa_supplicant not confined > - several apache child processes like > 4049 /usr/sbin/httpd-prefork confined by > '/usr/sbin/httpd{,2}-prefork//HANDLING_UNTRUSTED_INPUT (complain)' > > I wonder if netstat or ss are "more right" ;-) My assumption is that ss is more correct. The apache entry actually is what demonstrated that the parsing of ss output was a little more complicated; in netstat output, it looks like: tcp6 0 0 :::80 :::* LISTEN 4296/apache2 whereas in ss, it looks like so: tcp LISTEN 0 128 :::80 :::* users:(("apache2",pid=22236,fd=4),("apache2",pid=8747,fd=4),("apache2",pid=8746,fd=4),("apache2",pid=8745,fd=4),("apache2",pid=8744,fd=4),("apache2",pid=8743,fd=4),("apache2",pid=4296,fd=4)) > Another difference: > The old aa-unconfined version gives nearly the same result as the > --with-netstat option (good), however the old version doesn't list > NetworkManager. > > The netstat output doesn't explain this difference: > > root@tux:/dev/shm> netstat -nlp |grep Network > raw0 0 :::58 :::*7 > 2286/NetworkManager > raw0 0 :::58 :::*7 > 2286/NetworkManager > unix 2 [ ACC ] STREAM HÖRT 43660 2286/NetworkManager > /run/NetworkManager/private-dhcp > root@tux:/dev/shm> netstat -nlp --protocol inet,inet6 |grep Network > raw0 0 :::58 :::*7 > 2286/NetworkManager > raw0 0 :::58 :::*7 > 2286/NetworkManager > > So - did you accidently fix (or hide?) a parsing bug along the way? > I doubt _less_ netstat output really meant to cause more aa-unconfined > output ;-) This was fixed in http://bazaar.launchpad.net/~apparmor-dev/apparmor/master/revision/3593 per https://lists.ubuntu.com/archives/apparmor/2016-December/010307.html > Another interesting[tm] detail (off-topic here) is: > 4464 /usr/bin/python2.7 (/usr/bin/python) not confined > > Hmm, this python2.7 process is salt-master. Interestingly, > salt-master.service has ExecStart=/usr/bin/salt-master > Any idea why the processes show up as "python2.7" in the processlist? Is this with all the patches in the series applied or just this one? It's possible that's a change due to reading the /proc/PID/cmdline directly rather than using cat. What does the contents of /proc/PID/cmdline look like for the salt process? (Note that it's a series of strings that are null terminated, so you might want to do something like "sed -e 's/\x0/\n/g'" on it). Thanks. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch 3/4] utils/aa-unconfined: allow specifying ss/netstat binary locations
On Fri, Dec 30, 2016 at 03:16:04PM +0100, Christian Boltz wrote: > Am Donnerstag, 29. Dezember 2016, 23:24:57 CET schrieb Steve Beattie: > > This patch allows a user to specify a specific location for ss or > > netstat for use in aa-unconfined, allowing a user to work around a > > tool that's buggy, uninstalled, or installed in an unexpected > > location. Note this option in the manpage. > > How likely is it that someone really needs this? > > I mean, if I have a broken ss or netstat installed, I'd want to fix it > for everything - but I wouldn't think about overriding it just for one > application, and leave everything else broken ;-) Well, I was more thinking in terms of "broken in that aa-unconfined couldn't parse it, and netstat isn't available". > So even without the security concerns, I tend to dislike this patch > simply because I think these options are superfluous IMHO. But yeah, I kind of agree with that. > However, parts of the patch make sense. I'm talking about handing over > the command name as parameter to the function - this will allow to write > tests using a fake_ss and fake_netstat command. > (We might need to move out those functions to a python module when we > write those tests to avoid running the global code in aa-unconfined, but > that can be easily done once someone writes such a test.) I did wonder if get_pids_ss() and get_pids_netstat() ought to move to apparmor/common.py, but didn't see a need to do so for the time being. > To sum it up: > Acked-by: Christian Boltz <appar...@cboltz.de> > for adding the function parameters that make testing possible with a > fake_ss or fake_netstat script > > The Ack does _not_ cover allowing users to specify the ss or netstat to > use via commandline parameters - if you really want that, use John's ack > for this part of the patch ;-) Okay to apply? Subject: utils/aa-unconfined: allow specifying ss/netstat binary locations This patch allows a user to specify a specific location for ss or netstat in the invocations of get_pids_ss() or get_pids_netstat(). Signed-off-by: Steve Beattie <st...@nxnw.org> --- utils/aa-unconfined |8 1 file changed, 4 insertions(+), 4 deletions(-) Index: b/utils/aa-unconfined === --- a/utils/aa-unconfined +++ b/utils/aa-unconfined @@ -50,7 +50,7 @@ def get_all_pids(): return set(filter(lambda x: re.search(r"^\d+$", x), aa.get_subdirectories("/proc"))) -def get_pids_ss(): +def get_pids_ss(ss='ss'): '''Get a set of pids listening on network sockets via ss(8)''' regex_lines = re.compile(r"^(tcp|udp|raw|p_dgr)\s.+\s+users:(?P\(\(.*\)\))$") regex_users_pids = re.compile(r'(\("[^"]+",(pid=)?(\d+),[^)]+\))') @@ -60,7 +60,7 @@ def get_pids_ss(): my_env['LANG'] = 'C' my_env['PATH'] = '/bin:/usr/bin:/sbin:/usr/sbin' for family in ['inet', 'inet6', 'link']: -cmd = ['ss', '-nlp', '--family', family] +cmd = [ss, '-nlp', '--family', family] if sys.version_info < (3, 0): output = subprocess.check_output(cmd, shell=False, env=my_env).split("\n") else: @@ -76,11 +76,11 @@ def get_pids_ss(): return pids -def get_pids_netstat(): +def get_pids_netstat(netstat='netstat'): '''Get a set of pids listening on network sockets via netstat(8)''' regex_tcp_udp = re.compile(r"^(tcp|udp|raw)6?\s+\d+\s+\d+\s+\S+\:(\d+)\s+\S+\:(\*|\d+)\s+(LISTEN|\d+|\s+)\s+(?P\d+)\/(\S+)") -cmd = ['netstat', '-nlp', '--protocol', 'inet,inet6'] +cmd = [netstat, '-nlp', '--protocol', 'inet,inet6'] my_env = os.environ.copy() my_env['LANG'] = 'C' my_env['PATH'] = '/bin:/usr/bin:/sbin:/usr/sbin' -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] utils/aa-unconfined: fix netstat invocation regression
On Fri, Dec 30, 2016 at 02:54:31PM +0100, Christian Boltz wrote: > For 2.10 and 2.9, I'd prefer to have a small patch (using netstat's > --protocol option) instead of a full aa-unconfined rewrite. Okay to apply to 2.10 and 2.9? Subject: utils/aa-unconfined: fix netstat invocation regression It was reported[1] that converting the netstat command to examine processes bound to ipv6 addresses broke on OpenSUSE due to the version of nettools not supporting the short -4 -6 arguments. This patch fixes the invocation of netstat to use the "--protocol inet,inet6" arguments instead, which should return the same results as the short options. Signed-off-by: Steve Beattie <st...@nxnw.org> --- utils/aa-unconfined |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Index: b/utils/aa-unconfined === --- a/utils/aa-unconfined +++ b/utils/aa-unconfined @@ -49,7 +49,7 @@ else: output = subprocess.check_output("LANG=C netstat -nlp46", shell=True).split("\n") else: #Python3 needs to translate a stream of bytes to string with specified encoding -output = str(subprocess.check_output("LANG=C netstat -nlp46", shell=True), encoding='utf8').split("\n") +output = str(subprocess.check_output("LANG=C netstat -nlp --protocol inet,inet6", shell=True), encoding='utf8').split("\n") for line in output: match = regex_tcp_udp.search(line) -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] aa-unconfined man page vs behavior?
While editing the man page for aa-unconfined in this patch set, I noticed that it's uh pretty inaccurate at describing the behavior of aa-unconfined. It described listing processes without apparmor policies applied, whereas the tool reports processes with and without policies applied. The question is, which way is the preferred way to fix this? Change the documentation to accurately reflect the tool's behavior, or adjust the tool to more closely reflect the documentation? For reference, here's the man page, after applying the first patch in the series: A-UNCONFINED(8) AppArmor AA-UNCONFINED(8) NAME aa-unconfined - output a list of processes with tcp or udp ports that do not have AppArmor profiles loaded SYNOPSIS aa-unconfined [--paranoid] [--with-ss | --with-netstat] OPTIONS --paranoid Displays all processes from /proc filesystem with tcp or udp ports that do not have AppArmor profiles loaded. --with-ss Use the ss(8) command to find processes listening on network sockets (the default). --with-netstat Use the netstat(8) command to find processes listening on network sockets. This is also what aa-unconfined will fall back to when ss(8) is not available. DESCRIPTION aa-unconfined will use netstat(8) to determine which processes have open network sockets and do not have AppArmor profiles loaded into the kernel. BUGS aa-unconfined must be run as root to retrieve the process executable link from the /proc filesystem. This program is susceptible to race conditions of several flavours: an unlinked executable will be mishandled; an executable started before an AppArmor profile is loaded will not appear in the output, despite running without confinement; a process that dies between the netstat(8) and further checks will be mishandled. This program only lists processes using TCP and UDP. In short, this program is unsuitable for forensics use and is provided only as an aid to profiling all network- accessible processes in the lab. If you find any bugs, please report them at <https://bugs.launchpad.net/apparmor/+filebug>. SEE ALSO ss(8), netstat(8), apparmor(7), apparmor.d(5), aa_change_hat(2), and <http://wiki.apparmor.net>. AppArmor 2.10.95 2016-12-30 AA-UNCONFINED(8) -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch 0/4] utils: fix aa-unconfined regression
This patch set fixes a regression in the utils/aa-unconfined utility introduced in trunk commit 3592 (and backported to apparmor 2.10 and 2.9) that was intended to add support for processes that listen on ipv6 sockets. The arguments passed to netstat are not supported by the version of netstat provided in OpenSUSE. It does this both by addressing the invocation of netstat as well as parsing ss(8) output and using this by default. The third patch in the series is offered optionally, as it makes aa-unconfined support using alternate binaries for netstat/ss, which may be problematic in restricted sudo environments. Proposed for trunk, 2.10, and 2.9. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch 4/4] utils/aa-unconfined: whitespace cleanups for pep8 consistency.
This is what this patch looks like when diff'ed ignoring spacing changes: $ quilt diff | diffstat aa-unconfined | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) $ quilt diff --diff 'diff -uw' $ Signed-off-by: Steve Beattie <st...@nxnw.org> --- utils/aa-unconfined | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) Index: b/utils/aa-unconfined === --- a/utils/aa-unconfined +++ b/utils/aa-unconfined @@ -114,15 +114,15 @@ else: for pid in sorted(map(int, pids)): try: -prog = os.readlink("/proc/%s/exe"%pid) +prog = os.readlink("/proc/%s/exe" % pid) except OSError: continue attr = None -if os.path.exists("/proc/%s/attr/current"%pid): -with apparmor.common.open_file_read("/proc/%s/attr/current"%pid) as current: +if os.path.exists("/proc/%s/attr/current" % pid): +with apparmor.common.open_file_read("/proc/%s/attr/current" % pid) as current: for line in current: line = line.strip() -if line.endswith(' (complain)', 1) or line.endswith(' (enforce)', 1): # enforce at least one char as profile name +if line.endswith(' (complain)', 1) or line.endswith(' (enforce)', 1): # enforce at least one char as profile name attr = line pname = None @@ -131,7 +131,7 @@ for pid in sorted(map(int, pids)): cmdline = cmd.readlines()[0] pname = cmdline.split("\0")[0] if '/' in pname and pname != prog: -pname = "(%s)"% pname +pname = "(%s)" % pname else: pname = "" regex_interpreter = re.compile(r"^(/usr)?/bin/(python|perl|bash|dash|sh)$") @@ -140,17 +140,17 @@ for pid in sorted(map(int, pids)): cmdline = re.sub(r"\x00", " ", cmdline) cmdline = re.sub(r"\s+$", "", cmdline).strip() -ui.UI_Info(_("%(pid)s %(program)s (%(commandline)s) not confined") % { 'pid': pid, 'program': prog, 'commandline': cmdline }) +ui.UI_Info(_("%(pid)s %(program)s (%(commandline)s) not confined") % {'pid': pid, 'program': prog, 'commandline': cmdline}) else: if pname and pname[-1] == ')': pname = ' ' + pname -ui.UI_Info(_("%(pid)s %(program)s%(pname)s not confined") % { 'pid': pid, 'program': prog, 'pname': pname }) +ui.UI_Info(_("%(pid)s %(program)s%(pname)s not confined") % {'pid': pid, 'program': prog, 'pname': pname}) else: if regex_interpreter.search(prog): cmdline = re.sub(r"\0", " ", cmdline) cmdline = re.sub(r"\s+$", "", cmdline).strip() -ui.UI_Info(_("%(pid)s %(program)s (%(commandline)s) confined by '%(attribute)s'") % { 'pid': pid, 'program': prog, 'commandline': cmdline, 'attribute': attr }) +ui.UI_Info(_("%(pid)s %(program)s (%(commandline)s) confined by '%(attribute)s'") % {'pid': pid, 'program': prog, 'commandline': cmdline, 'attribute': attr}) else: if pname and pname[-1] == ')': pname = ' ' + pname -ui.UI_Info(_("%(pid)s %(program)s%(pname)s confined by '%(attribute)s'") % { 'pid': pid, 'program': prog, 'pname': pname, 'attribute': attr }) +ui.UI_Info(_("%(pid)s %(program)s%(pname)s confined by '%(attribute)s'") % {'pid': pid, 'program': prog, 'pname': pname, 'attribute': attr}) -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch 3/4] utils/aa-unconfined: allow specifying ss/netstat binary locations
This patch allows a user to specify a specific location for ss or netstat for use in aa-unconfined, allowing a user to work around a tool that's buggy, uninstalled, or installed in an unexpected location. Note this option in the manpage. [The downside to this patch is that if an environment uses something like a restrictive sudo policy around aa-unconfined, this would possibly give a user a way to subvert that. So I'm ambivalent about this patch.] Signed-off-by: Steve Beattie <st...@nxnw.org> --- utils/aa-unconfined | 24 +++- utils/aa-unconfined.pod |8 +--- 2 files changed, 20 insertions(+), 12 deletions(-) Index: b/utils/aa-unconfined === --- a/utils/aa-unconfined +++ b/utils/aa-unconfined @@ -34,8 +34,8 @@ _ = init_translation() parser = argparse.ArgumentParser(description=_("Lists unconfined processes having tcp or udp ports")) parser.add_argument("--paranoid", action="store_true", help=_("scan all processes from /proc")) bin_group = parser.add_mutually_exclusive_group() -bin_group.add_argument("--with-ss", action='store_true', help=_("use ss(8) to find listening processes (default)")) -bin_group.add_argument("--with-netstat", action='store_true', help=_("use netstat(8) to find listening processes")) +bin_group.add_argument("--with-ss", nargs='?', const='ss', metavar='SS_PATH', help=_("use ss(8) to find listening processes (default)")) +bin_group.add_argument("--with-netstat", nargs='?', const='netstat', metavar='NETSTAT_PATH', help=_("use netstat(8) to find listening processes")) args = parser.parse_args() paranoid = args.paranoid @@ -50,17 +50,20 @@ def get_all_pids(): return set(filter(lambda x: re.search(r"^\d+$", x), aa.get_subdirectories("/proc"))) -def get_pids_ss(): +def get_pids_ss(ss): '''Get a set of pids listening on network sockets via ss(8)''' regex_lines = re.compile(r"^(tcp|udp|raw|p_dgr)\s.+\s+users:(?P\(\(.*\)\))$") regex_users_pids = re.compile(r'(\("[^"]+",(pid=)?(\d+),[^)]+\))') +if ss is None: +ss = 'ss' + pids = set() my_env = os.environ.copy() my_env['LANG'] = 'C' my_env['PATH'] = '/bin:/usr/bin:/sbin:/usr/sbin' for family in ['inet', 'inet6', 'link']: -cmd = ['ss', '-nlp', '--family', family] +cmd = [ss, '-nlp', '--family', family] if sys.version_info < (3, 0): output = subprocess.check_output(cmd, shell=False, env=my_env).split("\n") else: @@ -76,11 +79,14 @@ def get_pids_ss(): return pids -def get_pids_netstat(): +def get_pids_netstat(netstat): '''Get a set of pids listening on network sockets via netstat(8)''' regex_tcp_udp = re.compile(r"^(tcp|udp|raw)6?\s+\d+\s+\d+\s+\S+\:(\d+)\s+\S+\:(\*|\d+)\s+(LISTEN|\d+|\s+)\s+(?P\d+)\/(\S+)") -cmd = ['netstat', '-nlp', '--protocol', 'inet,inet6'] +if netstat is None: +netstat = 'netstat' + +cmd = [netstat, '-nlp', '--protocol', 'inet,inet6'] my_env = os.environ.copy() my_env['LANG'] = 'C' my_env['PATH'] = '/bin:/usr/bin:/sbin:/usr/sbin' @@ -101,10 +107,10 @@ def get_pids_netstat(): pids = set() if paranoid: pids = get_all_pids() -elif args.with_ss or (not args.with_netstat and (os.path.exists('/bin/ss') or os.path.exists('/usr/bin/ss'))): -pids = get_pids_ss() +elif args.with_ss is not None or (args.with_netstat is None and (os.path.exists('/bin/ss') or os.path.exists('/usr/bin/ss'))): +pids = get_pids_ss(args.with_ss) else: -pids = get_pids_netstat() +pids = get_pids_netstat(args.with_netstat) for pid in sorted(map(int, pids)): try: Index: b/utils/aa-unconfined.pod === --- a/utils/aa-unconfined.pod +++ b/utils/aa-unconfined.pod @@ -27,7 +27,7 @@ not have AppArmor profiles loaded =head1 SYNOPSIS -B] [I<--with-ss> | I<--with-netstat>]> +B] [I<--with-ss> [SS_PATH] | I<--with-netstat> [NETSTAT_PATH]]> =head1 OPTIONS @@ -41,13 +41,15 @@ that do not have AppArmor profiles loade =item B<--with-ss> Use the ss(8) command to find processes listening on network sockets -(the default). +(the default). An alternate ss binary can be specified using the +optional I argument. =item B<--with-netstat> Use the netstat(8) command to find processes listening on network sockets. This is also what aa-unconfined will fall back to when ss(8) -is not available. +is not available. An alternate netstat binary can be specified using +the optional I argument. =back -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch 1/4] utils/aa-unconfined: fix netstat usage, use ss(8) by default
It was reported[1] that converting the netstat command to examine processes bound to ipv6 addresses broke on OpenSUSE due to the version of nettools not supporting the short -4 -6 arguments. This patch switches to use the ss(8) utility from iproute2 by default (if ss is found) as netstat/net-tools is deprecated. Unfortunately, ss's '--family' argument does not accept multiple families, nor does passing '--family' multiple times with different arguments work either[2], so aa-unconfined invokes ss multiple times to gather the different socket families. It also fixes the invocation of netstat to use the "--protocol inet,inet6" arguments instead, which should return the same results as the short options. This patch provides command line arguments to manually switch using one tool or the other, as well as converting the invocations of ss and netstat to not use a shell, and documents these options in the aa-unconfined man page. [1] Was a bug filed for this? [2] In fact, the version of ss/iproute2 in Ubuntu 14.04 LTS does not restrict the listings to network sockets when 'ss -nlp --family inet' is invoked. Signed-off-by: Steve Beattie <st...@nxnw.org> --- utils/aa-unconfined | 74 +++- utils/aa-unconfined.pod | 25 2 files changed, 81 insertions(+), 18 deletions(-) Index: b/utils/aa-unconfined === --- a/utils/aa-unconfined +++ b/utils/aa-unconfined @@ -1,6 +1,7 @@ #! /usr/bin/python3 # -- #Copyright (C) 2013 Kshitij Gupta <kgupta8...@gmail.com> +#Copyright (C) 2016 Canonical, Ltd. # #This program is free software; you can redistribute it and/or #modify it under the terms of version 2 of the GNU General Public @@ -15,6 +16,7 @@ import argparse import os import re +import subprocess import sys import apparmor.aa as aa @@ -31,6 +33,9 @@ _ = init_translation() parser = argparse.ArgumentParser(description=_("Lists unconfined processes having tcp or udp ports")) parser.add_argument("--paranoid", action="store_true", help=_("scan all processes from /proc")) +bin_group = parser.add_mutually_exclusive_group() +bin_group.add_argument("--with-ss", action='store_true', help=_("use ss(8) to find listening processes (default)")) +bin_group.add_argument("--with-netstat", action='store_true', help=_("use netstat(8) to find listening processes")) args = parser.parse_args() paranoid = args.paranoid @@ -39,26 +44,69 @@ aa_mountpoint = aa.check_for_apparmor() if not aa_mountpoint: raise aa.AppArmorException(_("It seems AppArmor was not started. Please enable AppArmor and try again.")) -pids = [] -if paranoid: -pids = list(filter(lambda x: re.search(r"^\d+$", x), aa.get_subdirectories("/proc"))) -else: -regex_tcp_udp = re.compile(r"^(tcp|udp|raw)6?\s+\d+\s+\d+\s+\S+\:(\d+)\s+\S+\:(\*|\d+)\s+(LISTEN|\d+|\s+)\s+(\d+)\/(\S+)") -import subprocess + +def get_all_pids(): +'''Return a set of all pids via walking /proc''' +return set(filter(lambda x: re.search(r"^\d+$", x), aa.get_subdirectories("/proc"))) + + +def get_pids_ss(): +'''Get a set of pids listening on network sockets via ss(8)''' +regex_lines = re.compile(r"^(tcp|udp|raw|p_dgr)\s.+\s+users:(?P\(\(.*\)\))$") +regex_users_pids = re.compile(r'(\("[^"]+",(pid=)?(\d+),[^)]+\))') + +pids = set() +my_env = os.environ.copy() +my_env['LANG'] = 'C' +my_env['PATH'] = '/bin:/usr/bin:/sbin:/usr/sbin' +for family in ['inet', 'inet6', 'link']: +cmd = ['ss', '-nlp', '--family', family] +if sys.version_info < (3, 0): +output = subprocess.check_output(cmd, shell=False, env=my_env).split("\n") +else: +# Python3 needs to translate a stream of bytes to string with specified encoding +output = str(subprocess.check_output(cmd, shell=False, env=my_env), encoding='utf8').split("\n") + +for line in output: +match = regex_lines.search(line.strip()) +if match: +users = match.group('users') +for (_, _, pid) in regex_users_pids.findall(users): +pids.add(pid) +return pids + + +def get_pids_netstat(): +'''Get a set of pids listening on network sockets via netstat(8)''' +regex_tcp_udp = re.compile(r"^(tcp|udp|raw)6?\s+\d+\s+\d+\s+\S+\:(\d+)\s+\S+\:(\*|\d+)\s+(LISTEN|\d+|\s+)\s+(?P\d+)\/(\S+)") + +cmd = ['netstat', '-nlp', '--protocol', 'inet,inet6'] +my_env = os.environ.copy() +my_env['LANG'] = 'C' +my_env['PATH'] = '/bin:/usr/bin:/sbin:/usr/sbin' if sys.version_info < (3, 0): -output = s
Re: [apparmor] [patch] Update nmbd profile and abstractions/samba
On Tue, Dec 13, 2016 at 04:54:43PM +0100, Christian Boltz wrote: > Hello, > > nmbd needs some additional permissions: > - k for /var/cache/samba/lck/* (via abstractions/samba) > - rw for /var/cache/samba/msg/ (the log only mentioned r, but that > directory needs to be created first) > - w for /var/cache/samba/msg/* (the log didn't indicate any read access) > > Reported by FLD on IRC, audit log on https://paste.debian.net/902010/ > > I propose this patch for trunk, 2.10 and 2.9 > > [ nmbd.diff ] Acked-by: Steve Beattie <st...@nxnw.org> for all three. Thanks. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] documentaion: add Makefile to generate pdfs from odt files
On Thu, Dec 01, 2016 at 03:11:07PM -0800, Seth Arnold wrote: > On Thu, Dec 01, 2016 at 02:46:10PM -0800, Steve Beattie wrote: > > Here's what the Makefile would look like after the renaming of the odt > > files occurred (much simpler): > > This is much easier on the eyes :) > > If John doesn't hate the renaming.. > > Acked-by: Seth Arnold <seth.arn...@canonical.com> > > Acked for everything :) Okay, I received no objections to the renaming, so I went ahead with that and applied the Makefile patch as well. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH] make aa-unconfined include ipv6
On Sat, Dec 10, 2016 at 01:31:06PM +0100, Christian Boltz wrote: > Am Freitag, 9. Dezember 2016, 23:09:03 CET schrieb Steve Beattie: > > Is the ss(8) http://manpages.ubuntu.com/manpages/xenial/man8/ss.8.html > > tool available? It's from iproute2 > > https://wiki.linuxfoundation.org/networking/iproute2 and serves a > > similar purpose. > > Yes, it's available (package iproute2) on Tumbleweed and Leap, so it > sounds like a good alternative. Is it merely available or installed by default? > I never used 'ss', so I'd prefer if someone who knows it sends a patch > ;-) I don't think any of us have much in the way of experience with ss, but netstat and net-tools on the whole are considered deprecated and superseded by the iproute2 package. I'll try to get a patch together. Thanks. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patches] avoid building tech doc during build
On Wed, Nov 30, 2016 at 03:31:59PM -0800, Steve Beattie wrote: > Attached are two alternative patches for avoiding the problem of > building the parser's techdoc during the build, which introduces > the latex stack as build dependencies for downstreams, and is also > a source of unrepeatability in the build. > > The first patch adds building the documentation (including man pages) > to the tarball generation process, and thus includes the generated > files in the tarball. Assuming the time stamps work out okay (local > tests showed they did), building the documentation would be (mostly) > avoided during the build -- the exception to this is the libapparmor > man pages, as the Makefile to generate its man pages is created > as part of the configure process, which is not done as part of the > release process. An additional step that I didn't do here would be to > remove the pdf from the default build path, to ensure that it doesn't > get triggered to build. > > The alternative patch removes the techdoc and the steps to build it > entirely. It has not been updated meaningfully since 2007, and no one > seems to have time to take on bringing it up to date. Furthermore, > John has created his own (unfinished) set of documentation as > {open,libre}office documents in the toplevel documentation directory. > > Either patch would improve the situation for downstreams. Based on feedback received, I committed the first patch, which keeps the techdoc, and builds it and most of the man pages during the release process rather than during the build, to make things easier for downstream consumption. I also included in the commit the followup patch to take the techdoc out of the default build path. Local changes to the man pages will still cause them to be regenerated during the build, so downstreams can still do that if necessary. All that said, at this point I consider the techdoc deprecated. I would like to see it updated to more accurately reflect the current capabilities of apparmor. And, while latex is a fine documentation language, if someone wants to update it but would rather work with a different technology than latex, be it libreoffice, markdown, or some other tool, I have no problem with that and am willing to help with such a conversion. Thanks. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patches] avoid building tech doc during build
On Thu, Dec 01, 2016 at 10:31:18PM +0100, Christian Boltz wrote: > I vote for keeping techdoc and creating the PDF at tarball creation, so > > apparmor-build_docs_w_tarball.patch > Acked-by: Christian Boltz <appar...@cboltz.de> > > I didn't test your patch, so please create a test tarball to make sure > the PDF really gets packaged (and isn't the victim of some "make clean" > that might be part of the tarball build process ;-) Yep, I verified that. There is a 'clean' stage of the tarball build process, but it's on the working tree, and honestly, it's a bit superfluous, because the tarball/snapshot build process goes out of its way to avoid capturing local changes by checking out a clean copy of the branch and using that as the basis for the tarball. Thanks. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patches] avoid building tech doc during build
On Wed, Nov 30, 2016 at 03:31:59PM -0800, Steve Beattie wrote: > An additional step that I didn't do here would be to > remove the pdf from the default build path, to ensure that it doesn't > get triggered to build. Here's the patch to do that as well, by creating an extra_docs target and using it as part of the tarball generation: Signed-off-by: Steve Beattie <st...@nxnw.org> --- Makefile|2 ++ parser/Makefile |3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) Index: b/Makefile === --- a/Makefile +++ b/Makefile @@ -75,6 +75,8 @@ clean: .PHONY: setup setup: cd $(__SETUP_DIR)/libraries/libapparmor && ./autogen.sh + # parser has an extra doc to build + make -C $(__SETUP_DIR)/parser extra_docs # libraries/libapparmor needs configure to have run before # building docs $(foreach dir, $(filter-out libraries/libapparmor tests, $(DIRS)), \ Index: b/parser/Makefile === --- a/parser/Makefile +++ b/parser/Makefile @@ -161,7 +161,8 @@ htmlmanpages: $(HTMLMANPAGES) pdf: techdoc.pdf -docs: manpages htmlmanpages pdf +docs: manpages htmlmanpages +extra_docs: pdf indep: docs $(Q)$(MAKE) -C po all -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] build: stop tarball builds on error
One thing I noticed while testing the patch that creates the tech doc at build time is that the snapshot/tarball builds use some shell constructs that end up causing failures at various stages to be ignored. The following patch addresses that. Signed-off-by: Steve Beattie <st...@nxnw.org> --- Makefile | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) Index: b/Makefile === --- a/Makefile +++ b/Makefile @@ -39,18 +39,18 @@ TAR_EXCLUSIONS= .PHONY: tarball tarball: clean - REPO_VERSION=`$(value REPO_VERSION_CMD)` ; \ - make export_dir __EXPORT_DIR=${RELEASE_DIR} __REPO_VERSION=$${REPO_VERSION} ; \ - make setup __SETUP_DIR=${RELEASE_DIR} ; \ + REPO_VERSION=`$(value REPO_VERSION_CMD)` && \ + make export_dir __EXPORT_DIR=${RELEASE_DIR} __REPO_VERSION=$${REPO_VERSION} && \ + make setup __SETUP_DIR=${RELEASE_DIR} && \ tar ${TAR_EXCLUSIONS} -cvzf ${RELEASE_DIR}.tar.gz ${RELEASE_DIR} .PHONY: snapshot snapshot: clean $(eval REPO_VERSION:=$(shell $(value REPO_VERSION_CMD))) $(eval SNAPSHOT_NAME=apparmor-$(VERSION)~$(REPO_VERSION)) - make export_dir __EXPORT_DIR=${SNAPSHOT_NAME} __REPO_VERSION=${REPO_VERSION} ; \ - make setup __SETUP_DIR=${SNAPSHOT_NAME} ; \ - tar ${TAR_EXCLUSIONS} -cvzf ${SNAPSHOT_NAME}.tar.gz ${SNAPSHOT_NAME} ; + make export_dir __EXPORT_DIR=${SNAPSHOT_NAME} __REPO_VERSION=${REPO_VERSION} && \ + make setup __SETUP_DIR=${SNAPSHOT_NAME} && \ + tar ${TAR_EXCLUSIONS} -cvzf ${SNAPSHOT_NAME}.tar.gz ${SNAPSHOT_NAME} .PHONY: coverity coverity: snapshot -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH] make aa-unconfined include ipv6
On Sat, Dec 10, 2016 at 12:21:06AM +0100, Christian Boltz wrote: > Am Donnerstag, 1. Dezember 2016, 16:13:26 CET schrieb John Johansen: > > aa-unconfined currently does not check/display ipv6 fix this > > > > Signed-off-by: John Johansen <john.johan...@canonical.com> > > > > === modified file 'utils/aa-unconfined' > > --- utils/aa-unconfined 2016-10-01 18:57:09 + > > +++ utils/aa-unconfined 2016-12-02 00:09:27 + > ... > > -output = subprocess.check_output("LANG=C netstat -nlp", > > shell=True).split("\n") > > +output = > > subprocess.check_output("LANG=C netstat -nlp46", > > Unfortunately this breaks aa-unconfined on openSUSE: > netstat: invalid option -- '4' > > (netstat is from net-tools-deprecated-1.60-770.1.x86_64) > > Looks like we'll need to find another solution... Is the ss(8) http://manpages.ubuntu.com/manpages/xenial/man8/ss.8.html tool available? It's from iproute2 https://wiki.linuxfoundation.org/networking/iproute2 and serves a similar purpose. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] documentaion: add Makefile to generate pdfs from odt files
On Thu, Dec 01, 2016 at 10:42:30PM +0100, Christian Boltz wrote: > Am Mittwoch, 30. November 2016, 16:17:49 CET schrieb Steve Beattie: > > Speaking of the odt files in the documentation directory... > > > > documentaion: add Makefile to generate pdfs from odt files > > > > The odt files in the documentation directory are hard to consume > > in that form. This adds a Makefile that generates pdfs from the > > odt files, using the unoconv tool, based on the idea/github tree > > https://github.com/jessfraz/apparmor-docs from > > Jessica Frazelle <m...@jessfraz.com>. > > Good idea! > > > That said, renaming the odt files to not have spaces would simplify > > this a lot. > > Indeed. Please do this first instead of commiting a Makefile with ugly > workarounds ;-) Does the following work for people? (John, since you named and created these docs originally, I've been expecting hatorade from you.) $ for file in *.odt ; do new_file=$(echo "${file}" | sed -e 's/ - /-/' -e 's/ /_/g') ; bzr mv "${file}" "${new_file}" ; done It results in: documentation/AppArmor Developer 1 - Kernel Notes.odt => documentation/AppArmor_Developer_1-Kernel_Notes.odt documentation/AppArmor Developer 2 - policy layout and encoding.odt => documentation/AppArmor_Developer_2-policy_layout_and_encoding.odt documentation/AppArmor Developer 3 - HFA.odt => documentation/AppArmor_Developer_3-HFA.odt documentation/AppArmor Developer 4 - Policy compilation.odt => documentation/AppArmor_Developer_4-Policy_compilation.odt documentation/AppArmor Developer 5 - extending apparmor to userspace.odt => documentation/AppArmor_Developer_5-extending_apparmor_to_userspace.odt documentation/AppArmor Policy.odt => documentation/AppArmor_Policy.odt documentation/Techdoc - eHFA.odt => documentation/Techdoc-eHFA.odt > For renaming all *.odt files to names without spaces: > Acked-by: Christian Boltz <appar...@cboltz.de> > > > Signed-off-by: Steve Beattie <st...@nxnw.org> > > --- > ... > > +++ b/documentation/Makefile > > @@ -0,0 +1,59 @@ > ... > > +# cannot figure out how to get make to honor the %.odt dependency > > correctly > > +# so need to manually enter the list below > > +%.pdf: > > + unoconv -v -f pdf --output "$@" $(call us,$(@:.pdf=.odt)) > > common/Make.rules has > > %.1 %.2 %.3 %.4 %.5 %.6 %.7 %.8: %.pod > $(POD2MAN) $< --release=$(MAN_RELEASE) --center=AppArmor --stderr > --section=$(subst .,,$(suffix $@)) > $@ > > So I'd guess you need something like (untested!) > > %.pdf: %.odt > unoconv -v -f pdf --output "$@" "$<" Yes, that's exactly what it would be. It just doesn't work if the odt filenames contain spaces. Here's what the Makefile would look like after the renaming of the odt files occurred (much simpler): --- documentation/Makefile | 36 1 file changed, 36 insertions(+) Index: b/documentation/Makefile === --- /dev/null +++ b/documentation/Makefile @@ -0,0 +1,36 @@ +# -- +#Copyright (c) 2016 Canonical Ltd. +# +#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. +# -- +NAME = documentation +all: +COMMONDIR=../common/ + +include $(COMMONDIR)/Make.rules + +all: docs + +SOURCES:= $(wildcard *.odt) +DOCS:=$(SOURCES:.odt=.pdf) + +.PHONY: docs +docs: $(DOCS) + +%.pdf: %.odt + unoconv -v -f pdf --output "$@" "$<" + +.PHONY: clean +ifndef VERBOSE +.SILENT: clean +endif +clean: + rm -f *.pdf + Thanks. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patches] avoid building tech doc during build
Hi, Attached are two alternative patches for avoiding the problem of building the parser's techdoc during the build, which introduces the latex stack as build dependencies for downstreams, and is also a source of unrepeatability in the build. The first patch adds building the documentation (including man pages) to the tarball generation process, and thus includes the generated files in the tarball. Assuming the time stamps work out okay (local tests showed they did), building the documentation would be (mostly) avoided during the build -- the exception to this is the libapparmor man pages, as the Makefile to generate its man pages is created as part of the configure process, which is not done as part of the release process. An additional step that I didn't do here would be to remove the pdf from the default build path, to ensure that it doesn't get triggered to build. The alternative patch removes the techdoc and the steps to build it entirely. It has not been updated meaningfully since 2007, and no one seems to have time to take on bringing it up to date. Furthermore, John has created his own (unfinished) set of documentation as {open,libre}office documents in the toplevel documentation directory. Either patch would improve the situation for downstreams. Opinions? Thanks. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ build: make documentation at tarball creation time, rather than during build The latex based techdoc in the parser/ tree adds a number of build dependencies for downstreams to create it; it also is the primary element to make the builds unrepeatable. Creating the techdoc and other documentation when generating a tarball for distribution avoids all that. * Makefile: build documentation as part of the tarball creation. Skip the libraries/libapparmor directory as it needs to have configure run before the manpages can be made. * changehat/mod_apparmor/Makefile, changehat/mod_apparmor/Makefile, utils/Makefile, profiles/Makefile: create separate docs target, some of them dummies. Signed-off-by: Steve Beattie <st...@nxnw.org> --- Makefile|4 changehat/mod_apparmor/Makefile |6 +- changehat/pam_apparmor/Makefile |7 ++- profiles/Makefile |8 ++-- utils/Makefile |7 +-- 5 files changed, 26 insertions(+), 6 deletions(-) Index: b/Makefile === --- a/Makefile +++ b/Makefile @@ -75,6 +75,10 @@ clean: .PHONY: setup setup: cd $(__SETUP_DIR)/libraries/libapparmor && ./autogen.sh + # libraries/libapparmor needs configure to have run before + # building docs + $(foreach dir, $(filter-out libraries/libapparmor tests, $(DIRS)), \ + make -C $(__SETUP_DIR)/$(dir) docs;) .PHONY: tag tag: Index: b/utils/Makefile === --- a/utils/Makefile +++ b/utils/Makefile @@ -1,6 +1,6 @@ # -- #Copyright (c) 1999, 2004-2009 NOVELL (All rights reserved) -#Copyright (c) 2010-2011 Canonical Ltd. +#Copyright (c) 2010-2016 Canonical Ltd. # #This program is free software; you can redistribute it and/or #modify it under the terms of version 2 of the GNU General Public @@ -30,10 +30,13 @@ PYMODULES = $(wildcard apparmor/*.py app MANPAGES = ${TOOLS:=.8} logprof.conf.5 -all: ${MANPAGES} ${HTMLMANPAGES} +all: docs $(MAKE) -C po all $(MAKE) -C vim all +.PHONY: docs +docs: ${MANPAGES} ${HTMLMANPAGES} + # need some better way of determining this DESTDIR=/ BINDIR=${DESTDIR}/usr/sbin Index: b/changehat/mod_apparmor/Makefile === --- a/changehat/mod_apparmor/Makefile +++ b/changehat/mod_apparmor/Makefile @@ -1,5 +1,6 @@ # -- #Copyright (c) 2004, 2005 NOVELL (All rights reserved) +#Copyright (c) 2016 Canonical, Ltd. # #This program is free software; you can redistribute it and/or #modify it under the terms of version 2 of the GNU General Public @@ -73,7 +74,10 @@ endif .SILENT: libapparmor_check libapparmor_check: ; $(ERROR_MESSAGE) -all: libapparmor_check $(TARGET) ${MANPAGES} ${HTMLMANPAGES} +all: libapparmor_check $(TARGET) docs + +.PHONY: docs +docs: ${MANPAGES} ${HTMLMANPAGES} %.so: %.c ${APXS} ${LIBAPPARMOR_FLAGS} -c $< ${LDLIBS} Index: b/changehat/pam_apparmor/Makefile === --- a/changehat/pam_apparmor/Makefile +++ b/changehat/pam_apparmor/Makefile @@ -1,5 +1,6 @@ # -- #Copyright (c) 1999, 2004, 2005 NOVELL (All rights reserved) +#Copyright (c) 2016 Canonical, Ltd. # #This program is free software; you can redistribute it and/or #modify it und
Re: [apparmor] wayland paths
On Wed, Nov 30, 2016 at 11:40:21AM -0800, Seth Arnold wrote: > Simon recommends some new paths for Wayland support in comment 8 at > https://bugs.launchpad.net/ubuntu/+source/apparmor/+bug/1507469 > > I propose this patch for trunk: > > > === modified file 'profiles/apparmor.d/abstractions/wayland' > --- profiles/apparmor.d/abstractions/wayland 2016-09-11 21:03:01 + > +++ profiles/apparmor.d/abstractions/wayland 2016-11-30 19:37:17 + > @@ -10,3 +10,5 @@ > # -- > >owner /{,var/}run/user/*/weston-shared-* rw, > + owner /run/user/*/wayland-[0-9]* rw, > + owner /run/user/*/{mesa,mutter,sdl,weston,xwayland}-shared-* rw, > > Signed-off-by: Seth Arnold <seth.arn...@canonical.com> Can we kill the first rule? Or at least only have the /var/ path, since the non-var path is covered by the last rule? Otherwise, Acked-by: Steve Beattie <st...@nxnw.org> Thanks! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] dovecot profile: allow capability sys_resource
On Tue, Nov 29, 2016 at 01:49:05PM +0100, Christian Boltz wrote: > On servers with not too much memory ("only" 16 GB), dovecot logins fail: > > Nov 25 21:35:15 server dovecot[28737]: master: Fatal: setrlimit(RLIMIT_DATA, > 268435456): Permission denied > Nov 25 21:35:15 server dovecot[28731]: master: Error: service(auth): command > startup failed, throttling for 2 secs > Nov 25 21:35:15 server dovecot[28737]: auth: Fatal: master: service(auth): > child 25976 returned error 89 (Fatal failure) > audit.log messages are: > ... apparmor="DENIED" operation="capable" profile="/usr/sbin/dovecot" > pid=25000 comm="dovecot" capability=24 capname="sys_resource" > ... apparmor="DENIED" operation="setrlimit" profile="/usr/sbin/dovecot" > pid=25000 comm="dovecot" rlimit=data value=268435456 Interesting. This should only be needed if raising RLIMIT_DATA ('ulimit -H -d' in bash/dash) over an existing hard limit. Is there some setting elsewhere that's lowering it before dovecot runs? > After allowing capability sys_resource, dovecot can increase the limit > and works again. > > I propose this patch for trunk, 2.10 and 2.9 That question aside, I think it's okay. Acked-by: Steve Beattie <st...@nxnw.org> for all three branches. > [ dovecot-cap-sys_resource.diff ] > > === modified file 'profiles/apparmor.d/usr.sbin.dovecot' > --- profiles/apparmor.d/usr.sbin.dovecot2014-12-22 16:49:28 + > +++ profiles/apparmor.d/usr.sbin.dovecot2016-11-29 11:46:32 + > @@ -28,6 +28,7 @@ > capability net_bind_service, >capability setuid, >capability sys_chroot, > + capability sys_resource, > >/etc/dovecot/** r, >/etc/mtab r, -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Allow "network unspec dgram, " in ntpd profile
On Mon, Nov 14, 2016 at 10:26:40PM +0100, Christian Boltz wrote: > a while ago, support for "network unspec" was added. However, nobody > updated the ntpd profile (at least not the profile in upstream bzr) > which was the main reason for adding "unspec". > > References: https://bugs.launchpad.net/ubuntu/+source/ntp/+bug/1546455 > (the original bugreport about "unspec") > > References: https://bugzilla.opensuse.org/show_bug.cgi?id=1009964 > (about the ntpd profile) > > > I propose this patch for trunk and 2.10 (the openSUSE bugreport is about > Tumbleweed, which uses 2.10.1). > > I'm not sure about 2.9 because I don't know if the "unspec" keyword was > backported to 2.9. > [ ntpd-network-unspec-dgram.diff ] Acked-by: Steve Beattie <st...@nxnw.org> for all three branches. The 2.9 branch contains 'parser/tst/simple_tests/network/network_ok_7.sd' which checks for the validity of the 'unspec' network keyword. Thanks! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [Branch ~apparmor-dev/apparmor/master] Rev 3582: libapparmor python bindings: use __init__.py to import from LibAppArmor.py
FYI, On Mon, Nov 14, 2016 at 10:08:23PM -, nore...@launchpad.net wrote: > > revno: 3582 > committer: Steve Beattie <sbeat...@ubuntu.com> > branch nick: apparmor > timestamp: Mon 2016-11-14 14:06:41 -0800 > message: > libapparmor python bindings: use __init__.py to import from LibAppArmor.py > > Fix import errors with swig > 3.0.8 with the libapparmor python > bindings. Do this by removing the code to rename the generated > LibAppArmor.py, and instead use a stub __init__.py that automatically > imports everything from LibAppArmor.py. Also adjust bzrignore to > compensate for the autogenerated file name changing. > > Bug: https://bugzilla.opensuse.org/show_bug.cgi?id=987607 > > Signed-off-by: Steve Beattie <st...@nxnw.org> > Acked-by: Christian Boltz <appar...@cboltz.de> > added: > libraries/libapparmor/swig/python/__init__.py > modified: > .bzrignore > libraries/libapparmor/swig/python/Makefile.am Because this commit (and the corresponding 2.10 commit) adds a file that previously may have existed as the result of being autogenerated in a build, when doing a bzr up/bzr pull, you may end up with a conflict out of this. It's safe to just remove the copy that bzr makes of the generated file and mark it resolved via 'bzr resolved libraries/libapparmor/swig/python/__init__.py'. Sorry for the minor hiccup. Thanks. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Move str_to_mode() tests to test-aamode.py
On Tue, Oct 11, 2016 at 12:39:58PM -0700, Seth Arnold wrote: > On Sun, Oct 09, 2016 at 08:32:48PM +0200, Christian Boltz wrote: > > +class AamodeTest_str_to_mode(AATest): > > +tests = [ > > +('x', apparmor.aamode.AA_MAY_EXEC), > > +('w', apparmor.aamode.AA_MAY_WRITE), > > +('r', apparmor.aamode.AA_MAY_READ), > > +('a', apparmor.aamode.AA_MAY_APPEND), > > +('l', apparmor.aamode.AA_MAY_LINK), > > +('k', apparmor.aamode.AA_MAY_LOCK), > > +('m', apparmor.aamode.AA_EXEC_MMAP), > > +('i', apparmor.aamode.AA_EXEC_INHERIT), > > +('u', apparmor.aamode.AA_EXEC_UNCONFINED | > > apparmor.aamode.AA_EXEC_UNSAFE), > > +('U', apparmor.aamode.AA_EXEC_UNCONFINED), > > +('p', apparmor.aamode.AA_EXEC_PROFILE | > > apparmor.aamode.AA_EXEC_UNSAFE), > > +('P', apparmor.aamode.AA_EXEC_PROFILE), > > +('c', apparmor.aamode.AA_EXEC_CHILD | > > apparmor.aamode.AA_EXEC_UNSAFE), > > +('C', apparmor.aamode.AA_EXEC_CHILD), > > +] > > I didn't find where this 'tests' array is actually used in the patch. > > Also, 'tests' is a bit generic -- is there a better description of what > this array mapping actually provides? > > A dictionary would allow quicker lookups from key to value; an array > makes more sense if it's only ever iterated, or if lookups can go both > directions. But since I didn't find any use of this array it's hard to > guess which would be better. :) The AATest class in the common_test.py that AamodeTest_str_to_mode is subsclassing from causes it to be used. Basically, the way it works is you define a 'tests' array, a _run_test() method, and make a call to setup_all_loops() in your test script, and test methods will be generated based on the array entries being passed to _run_test(). The idea here is to avoid code duplication when running many similar tests, as well as trying to make adding tests as simple as adding an additional entry in array (c.f. my followup to the original patch). -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Rename config_test.py to test-config.py
On Sun, Oct 09, 2016 at 09:50:12PM +0200, Christian Boltz wrote: > $subject. > > This little change means that the tests will run as part of > 'make check'. > > [ 05-rename-config_test.diff ] > > [ imagine 'bzr mv utils/test/config_test.py utils/test/test-config.py' result > here ;-) - the file content won't change ] Acked-by: Steve Beattie <st...@nxnw.org>. Thanks! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] complete test coverage for FileRule
On Sun, Oct 09, 2016 at 04:06:50PM +0200, Christian Boltz wrote: > this patch adds a testcase with exec-only permissions (which get ignored by > get_perms_for_path()) to increase FileRule test coverage to 100%. > > [ 01-test-file-coverage.diff ] Acked-by: Steve Beattie <st...@nxnw.org>. Thanks! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] [06/38] Add FileRule and FileRuleset
On Thu, Sep 29, 2016 at 09:08:36PM +0200, Christian Boltz wrote: > Hello, > > Am Freitag, 12. August 2016, 22:47:07 CEST schrieb Christian Boltz: > > +def split_perms(perm_string, deny): > > +'''parse permission string > > + - perm_string: the permission string to parse > > + - deny: True if this is a deny rule > > + ''' > > +perms = set() > > +exec_mode = None > > + > > +while perm_string: > > +if perm_string[0] in file_permissions: > > +perms.add(perm_string[0]) > > +perm_string = perm_string[1:] > > +elif perm_string[0] == 'x': > > +if not deny: > > +raise AppArmorException(_("'x' must be preceded by an exec > > qualifier (i, P, C or U)")) > > +exec_mode = 'x' > > +perm_string = perm_string[1:] > > +elif perm_string.startswith(allow_exec_transitions): > > +if exec_mode: > > +raise AppArmorException(_('conflicting execute permissions > > found: %s and %s' % (exec_mode, perm_string[0:2]))) > > +exec_mode = perm_string[0:2] > > +perm_string = perm_string[2:] > > +elif perm_string.startswith(allow_exec_fallback_transitions) and > > not deny: > > I'd like to change this to > > +elif perm_string.startswith(allow_exec_fallback_transitions): > > (= drop the "and not deny" part) for two reasons: > - to get it in sync with the allow_exec_transtions check > - to get a better error message - with the "and not deny" in place, a > "deny /foo pix," rule will result in hitting the else branch ("unknown > character"). Without the "and not deny" check, __init__ will do the > error checking and come up with a more helpful error message. > > > Opinions? Acks? Objections? ;-) Sounds reasonable to me, Acked-by: Steve Beattie <st...@nxnw.org>. Thanks. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] [42/38] Drop more unused functions from aa.py
On Thu, Sep 29, 2016 at 08:52:44PM +0200, Christian Boltz wrote: > after looking at matchliteral(), I found out that it's only user is > rematchfrag(), which is only called in a) an "if False:" block and > b) match_include_to_path() - and that is only called by the also unused > match_prof_incs_to_path() function. > > This patch drops some dead code (like the mentioned "if False:" block) > and the now unused functions > - matchliteral() > - rematchfrag() > - match_include_to_path() > - match_prof_incs_to_path() > > This patch is also THE ANSWER to the question when I'll finally consider > this patch series complete. > > 42. It can't become better than that! ;-) > > [ 42-cleanup-aa.py.diff ] Woo, more code deletion! Acked-by: Steve Beattie <st...@nxnw.org>. Thanks! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] [28/38] AARE: let match() handle plain path regexes as non-regex
On Thu, Sep 29, 2016 at 08:48:21PM +0200, Christian Boltz wrote: > Hello, > > Am Montag, 26. September 2016, 14:45:34 CEST schrieb Steve Beattie: > > On Fri, Aug 12, 2016 at 11:03:09PM +0200, Christian Boltz wrote: > > > when matching an AARE against another AARE, most AARE objects don't > > > contain orig_regex (only AARE instances originating from a log event > > > contain orig_regex). > > > > > > In this case, match() will use is_equal() to error out on the safe > > > side. Unfortunately this also means that there are lots of false > > > negative cases where match() returns False errornously. > > > > > > With this patch, match() checks the given AARE regex and, if it > > > doesn't contain any special characters (wildcards, alternations or > > > variables), handles it as plain path. This avoids most of the false > > > negatives. > > > > > > Also extend the AARE tests to check a bunch of plain path regexes > > > using AARE matching instead of only str matching. > > > > > > [ 28-aare-plain-path.diff ] > > > > Acked-by: Steve Beattie <st...@nxnw.org>, though I'm not crazy about > > commingling the plain checks with the regex checks in the same > > function, as I suspect it will make figuring out what's failing when > > something goes wrong more difficult (in answering "What's being > > tested and why?"). > > Please allow me to disagree ;-) Sorry, I wrote my comment particularly poorly. My complaint was about commingling the regex and non-regex checks in the *testcases* in test/test-aare.py, not the implementation itself in match(). My apologies for the confusion. Thanks. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] [41/38] let aa-mergeprof ask about new hats and subprofiles
On Wed, Sep 28, 2016 at 11:08:40PM +0200, Christian Boltz wrote: > if a merged profile contains additional hats or subprofiles, the "old" > aa-mergeprof silently created them as additional hasher elements (partly > buggy, because subprofiles would end up as '^/subprofile' instead of > 'profile subprofile'). After switching to FileRule, aa-mergeprof crashes > on new hats or subprofiles. > > This patch adds code to ask the user if the new hat or subprofile should > be added - which means this patch replaces two bugs (crash + silently > adding subprofiles and hats) with a new feature ;-) > > > The new questions also add a new text CMD_ADDSUBPROFILE in ui.py. > > Finally, the new "button" combinations get added to test-translations.py. > > > > If you want to test, try to aa-mergeprof this profile (the subprofile > and hat are dummies, nothing ping would really require): > > > #include > /{usr/,}bin/ping { > #include > #include > #include > > capability net_raw, > capability setuid, > network inet raw, > network inet6 raw, > > /{,usr/}bin/ping mixr, > /etc/modules.conf r, > > ^hat { > /bin/hat r, > /bin/bash px, > } > > profile /subprofile { > /bin/subprofile r, > /bin/bash px, > } > > # Site-specific additions and overrides. See local/README for details. > #include > } > > Note that this patch is not covered by unittests, but it passed all my > manual tests. > > [ 41-mergeprof-new-subprofiles.diff ] Acked-by: Steve Beattie <st...@nxnw.org>. Thanks! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] [40/38] Load all includes in aa-mergeprof ask_the_questions()
On Tue, Sep 27, 2016 at 11:17:05PM +0200, Christian Boltz wrote: > aa-mergeprof empties 'includes' when running reset_aa(). The result is > KeyError: 'abstractions/newly_added_abstraction' > if an include file gets added because it isn't part of 'includes' at > this time. Note that you'll need to add another rule after adding the > include to trigger checking the includes for superfluous rules. > > This fixes the regression found by Steve - which isn't really a > regression, "just" one more thing that got more visible with the new > code. Before, it was just an ill-addressed hasher that didn't complain ;-) > > [ 40-mergeprof-load-includes.diff ] Acked-by: Steve Beattie <st...@nxnw.org>. Thanks for addressing it quickly! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] regression introduced by this series
On Mon, Sep 26, 2016 at 05:40:37PM -0700, Steve Beattie wrote: > On Fri, Aug 12, 2016 at 10:40:39PM +0200, Christian Boltz wrote: > > this patch series introduces the FileRule and FileRuleset classes and > > changes several code sections to use these classes instead of the old > > 'path' hasher. > > > > the switch to FileRule made some bugs visible that survived unnoticed > > with hasher for years. > > I've found a regression that occurs once patch > 14-switch-to-FileRule.diff is applied (but still occurs with the entire > sequence applied), and occurs under either python2 or python3. > > When merging two profiles that contain the following: > > == PROFILE A == > #include > > /usr/lib/postfix/smtpd { > #include > #include > > capability dac_override, > capability dac_read_search, > > /usr/lib/postfix/smtpd rix, > /usr/sbin/postdrop rpx, > > } > == PROFILE B == > #include > > /usr/lib/postfix/smtpd { > #include > > capability dac_override, > capability dac_read_search, > > /usr/lib/postfix/smtpd rmix, > /usr/sbin/postdrop rPx, > > } > == END PROFILES == > > i.e. their differences are: > > --- usr.lib.postfix.smtpd 2016-09-26 17:30:35.848884709 -0700 > +++ ../usr.lib.postfix.smtpd 2016-09-26 17:30:44.620874325 -0700 > @@ -12,13 +12,12 @@ > #include > > /usr/lib/postfix/smtpd { > - #include > - #include > + #include > >capability dac_override, >capability dac_read_search, > > - /usr/lib/postfix/smtpd rix, > - /usr/sbin/postdrop rpx, > + /usr/lib/postfix/smtpd rmix, > + /usr/sbin/postdrop rPx, > > } > > Running aa-mergeprof fails, if the added include for > abstractions/openssl is allowed, followed by adjusting the postdrop > permissions, like so: > > == BEGIN OUTPUT == > > Merging profile for /usr/lib/postfix/smtpd > > File includes: Select the ones you wish to add > > [1 - #include ] > [(A)llow] / (I)gnore / Abo(r)t / (F)inish > Adding #include to the file. > > Path:/usr/sbin/postdrop > Select the appropriate mode: > > [1 - /usr/sbin/postdrop rpx,] > 2 - /usr/sbin/postdrop rPx, > (A)llow / Abo(r)t > > Path:/usr/sbin/postdrop > Select the appropriate mode: > > 1 - /usr/sbin/postdrop rpx, > [2 - /usr/sbin/postdrop rPx,] > (A)llow / Abo(r)t To clarify, after selecting option 2, I've now selected (A)llow, and then the traceback occurs when aa-mergeprof moves on to handle the difference on the /usr/lib/postfix/smtpd permissions. Dropping the openssl include difference, or even simply responding to the openssl prompt with (I)gnore also causes it not to be triggered. > Traceback (most recent call last): > File "./aa-mergeprof", line 457, in > main() > File "./aa-mergeprof", line 126, in main > act([user_file, base_file, None], 2, profile_name) > File "./aa-mergeprof", line 144, in act > mergeprofiles.ask_the_questions('base', merging_profile) > File "./aa-mergeprof", line 331, in ask_the_questions > if is_known_rule(aa[profile][hat], ruletype, rule_obj): > File "${HOME}/bzr/apparmor-master/utils/apparmor/aa.py", line 3498, in > is_known_rule > if include[incname][incname].get(rule_type, False): > KeyError: 'abstractions/openssl' > > > An unexpected error occoured! > > For details, see /tmp/apparmor-bugreport-03m_vh3s.txt > Please consider reporting a bug at https://bugs.launchpad.net/apparmor/ > and attach this file. > > == END OUTPUT == > > and here's the contents of the bugreport file. Hope this all helps track > down what's gone wrong: > > KeyError > Python 3.5.2: /usr/bin/python3 > Mon Sep 26 17:32:12 2016 > > A problem occurred in a Python script. Here is the sequence of > function calls leading up to the error, in the order they occurred. > > ${HOME}/bzr/apparmor-master/utils/aa-mergeprof in () > 448 > 449 > edit_rule_obj.store_edit(newpath) > 450 options, default_option = > add_to_options(options, edit_rule_obj.get_raw()) > 451 apparmor.aa.user_globs[newpath] > = AARE(newpath, True) > 452 > 453 else: > 454 done = False > 455 > 456 if __name__ == '__main__': >
Re: [apparmor] [patch] [34/38] logprof, mergeprof: cleanup superfluous rules when user adds a new rule
On Fri, Aug 12, 2016 at 11:06:33PM +0200, Christian Boltz wrote: > when an user adds a new rule to a profile, cleanup / delete existing > rules that are covered by the new rule, and report the number of deleted > rules. > > [ 34-logprof-cleanup-duplicates-on-add.diff ] Acked-by: Steve Beattie <st...@nxnw.org>. Thanks! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] [31/38] FileRule: add get_exec_rules_for_path() and get_exec_conflict_rules()
On Fri, Aug 12, 2016 at 11:05:05PM +0200, Christian Boltz wrote: > get_exec_rules_for_path() returns a FileRuleset with all rules matching > the given path. > > get_exec_conflict_rules() returns a FileRuleset with all exec rules that > conflict with the given oldrule. This will be used by aa-mergeprof to > ask the user which rule he wants to keep. > > Also add tests for both functions. > > [ 31-filerule-exec-conflicts.diff ] Acked-by: Steve Beattie <st...@nxnw.org>. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] [30/38] Re-enable clear_common() call in aa-mergeprof
On Fri, Aug 12, 2016 at 11:04:40PM +0200, Christian Boltz wrote: > the clear_common() call was disabled because it crashed in > delete_path_duplicates(). With the switch to FileRule, this function > no longer exists and therefore it can't crash ;-) > > This patch re-enables the clear_common() call to avoid asking > superfluous questions. > > References: https://bugs.launchpad.net/apparmor/+bug/1382236 > > [ 30-cleanprof-re-enable-clear_common.diff ] Acked-by: Steve Beattie <st...@nxnw.org>. Thanks! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] [29/38] let _is_covered_aare() check against the AARE instead of the (str) regex
On Fri, Aug 12, 2016 at 11:03:33PM +0200, Christian Boltz wrote: > This is the correct way of doing AARE matches. However, this check is > more strict when matching against an AARE containing wildcards etc. > (which can "by luck" match when doing str matching) > > To avoid breaking DbusRule, PtraceRule and SignalRule (especially their > tests), introduce _is_covered_aare_compat() which keeps the previous > behaviour of doing str matching, and use it in these classes. > > On the long term, _is_covered_aare_compat() needs to go away, but doing > the changes needed in DbusRule, PtraceRule and SignalRule (or ideally > just in AARE) are out of scope for the FileRule patch series. (Refactorings or other cleanups that occur because adding a rule type exposes issues with other types of rules are certainly what I would consider in scope for a patch series.) > [ 29-aare-covered-regex.diff ] Acked-by: Steve Beattie <st...@nxnw.org>. Thanks! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] [17/38] Implement globbing in FileRule
On Fri, Aug 12, 2016 at 10:57:08PM +0200, Christian Boltz wrote: > this patch adds the glob() and glob_ext() functions to FileRule, and sets > self.can_glob and self.can_glob_ext. Also add some tests (just enough to > make sure the FileRule integration works - the globbing is handled > inside AARE,and the AARE tests contain more testcases). Can you add a comment to this effect in the FileGlobTest class, that it's purpose is to ensure the FileRule integration works, and a pointer to the more comprehensive regex tests in test-aare.py? > Note that the implementation differs from the original plan (which was > to have globbing in *Ruleset). Therefore add can_glob and can_glob_ext > to BaseRule (both default to False), and add a comment to BaseRuleset > that globbing needs to be removed from all *Ruleset classes. Okay. > [ 17-FileRule-implement-globbing.diff ] Otherwise, looks fine. Acked-by: Steve Beattie <st...@nxnw.org>. Thanks! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] [16/38] move glob_path() and glob_path_ext() to AARE
On Fri, Aug 12, 2016 at 10:56:14PM +0200, Christian Boltz wrote: > Hello, > > [patch] [16/38] move glob_path() and glob_path_ext() to AARE > glob_path() and glob_path_ext() modify a (path) regex, so move them to > AARE. Also change them to use self.regex instead of the newpath > parameter, and to return a new AARE object. > > While on it, also add several tests to test-aare.py. > > > Note: There are still glob_path() and glob_path_ext() calls in aa.py, > but those calls are in a (since the middle of this patch series) dead > code section. As one would hope, pyflakes also complains about this. > [ 16-move_glob_path_and_glob_path_ext_to_AARE.diff ] Acked-by: Steve Beattie <st...@nxnw.org>. Thanks. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] README: deprecate python2.7 support with swig > 3.0.8
On Tue, Sep 20, 2016 at 11:45:23AM -0700, Steve Beattie wrote: > Once we kill python2.7 support, then we can go with the "simple" form of > the __init__.py file. Which reminds me, we ought to note that we're going to kill python 2.7 support in the README file. I moved the required versions section nearer to the beginning and dropped the bit about aa-exec depending on perl, since it's now a compiled binary. Subject: REAMDE: deprecate python2.7 support --- README | 33 + 1 file changed, 17 insertions(+), 16 deletions(-) Index: b/README === --- a/README +++ b/README @@ -52,6 +52,23 @@ kernel (2.6.37 patches should apply clea Without these patches applied to the kernel, the AppArmor userspace will not function correctly. +- +Required versions +- + +The AppArmor userspace utilities are written with some assumptions about +installed and available versions of other tools. This is a (possibly +incomplete) list of known version dependencies: + +The Python utilities require a minimum of Python 2.7 or Python 3.3. +PLEASE NOTE: Python 2.7 support is now deprecated, this is the last +AppArmor utilities release that will support Python 2.7. + +Some utilities (aa-notify and aa-decode) require Perl 5.10.1 or newer. + +Most shell scripts are written for POSIX-compatible sh. aa-decode expects +bash, probably version 3.2 and higher. + -- Building and Installing AppArmor Userspace -- @@ -222,19 +239,3 @@ Building and Installing AppArmor Kernel --- TODO - - -- -Required versions -- - -The AppArmor userspace utilities are written with some assumptions about -installed and available versions of other tools. This is a (possibly -incomplete) list of known version dependencies: - -The Python utilities require a minimum of Python 2.7 or Python 3.3. - -Some utilities (aa-exec, aa-notify and aa-decode) require Perl 5.10.1 or newer. - -Most shell scripts are written for POSIX-compatible sh. aa-decode expects -bash, probably version 3.2 and higher. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] fix python LibAppArmor import failures with swig > 3.0.8
On Wed, Sep 14, 2016 at 02:12:35PM -0500, Tyler Hicks wrote: > On 09/14/2016 01:52 PM, Christian Boltz wrote: > > renaming LibAppArmor.py to __init__.py breaks the import path > > calculation in swig (> 3.0.8)-generated python code, leading to an error > > message saying > > No module named '_LibAppArmor' > > > > Therefore this patch drops renaming the file. To stay compatible with the > > import LibAppArmor.$function_name > > syntax, add an __init__.py that does > > from LibAppArmor.LibAppArmor import * > > > > References: https://bugzilla.opensuse.org/show_bug.cgi?id=987607 > > > > > > Also adjust .bzrignore for this change. > > > > > > > > I propose this patch for trunk and 2.10. > > I'm undecided about 2.9 - technically it shares this bug, but I'd expect > > that 2.9 users don't use the latest swig ;-) - opinions? Ugh, I *really* don't like the 'from LibAppArmor.LibAppArmor import *' bit. > Acked-by: Tyler Hicks <tyhi...@canonical.com> > > Please apply to 2.9, as well. IIRC, this is one of the errors I hit when > I run `make check` on the utils/ dir from Ubuntu 16.04 (with swig > 3.0.8-0ubuntu3) development machine so it'd be nice if that's fixed in > all of our stable branches. Thanks for fixing it! Bleah, swig 3.0.8 is seriously broken. It's requiring that the _LibAppArmor.so be a globally visible module/shared library, rather than being hidden in LibAppArmor._LibAppArmor. > > Note: you'll need to cleanup your libraries/libapparmor/swig/python/ > > directory manually before applying this patch ("make clean" isn't enough, > > so check "bzr ignored"), and regenerate the autogenerated files with > > autogen.sh and configure afterwards. > > > > If there's a "superclean" make target I missed, please tell me ;-) make maintainer-clean, I believe (but it's only supported by the libapparmor subdirectory). -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH] A new profile for weechat
Hi Tomasz, On Thu, Sep 08, 2016 at 12:20:14PM +0200, Tomasz Miąsko wrote: > I prepared a profile for weechat IRC client, you can find a patch attached. > Could you include it in apparmor-profiles? > > Note-1: Tested on Debian. > > Note-2: The permissions within .weechat directory are not exactly > fine-grained, mostly because I wanted to support the case when weechat is > run for the first time and creates all the directories and files within. > > Thanks, For the mailing list record, Seth committed your profile to the apparmor-profiles git tree in: https://git.launchpad.net/apparmor-profiles/commit/?id=510a926cb0a8f0202e596cbe5bfb5224b3b34147 https://git.launchpad.net/apparmor-profiles/commit/?id=b008e835dfd66fe41971e988e79126d78de26080 Thanks for the profile! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH/apparmor-profiles] Add profile for /usr/share/update-notifier/notify-reboot-required
On Sat, Jul 30, 2016 at 02:48:23PM +0200, intrigeri wrote: > Steve Beattie: > > Entirely untested with reboot-notifier, but the following should work: > > Confirmed! > > > (I'd apply the same changes to the copy in the 16.10/ directory > > as well.) > > Please go ahead, thanks :) Thanks for the feedback, I've gone ahead and pushed the changes to master. [There's a bug right now with launchpad's git implementation that despite telling it to email all changes to the branch, launchpad doesn't actually do so. I understand that addressing it is a near-term item on the roadmap, though.] -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Fix aa-logprof "add hat" endless loop
On Sun, Aug 14, 2016 at 10:02:45PM +0200, Christian Boltz wrote: > This turned out to be a simple case of misinterpreting the promptUser() > result - it returns the answer and the selected option, and > "surprisingly" something like > ('CMD_ADDHAT', 0) > never matched > 'CMD_ADDHAT' > ;-) > > I also noticed that the new hat doesn't get initialized as > profile_storage(), and that the changed profile doesn't get marked as > changed. This is also fixed by this patch. > > > References: https://bugs.launchpad.net/apparmor/+bug/1538306 > > > I propose this patch for trunk, 2.10 and 2.9. > Note that 2.9 doesn't have profile_storage(), therefore I won't add > that line there. Acked-by: Steve Beattie <st...@nxnw.org> for all three branches. Thanks. > [ fix-add-hat.diff ] -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] winbindd profile: allow dac_override
On Wed, Aug 03, 2016 at 01:57:53PM +0200, Christian Boltz wrote: > This is needed to delete kerberos ccache files, for details see > https://bugzilla.opensuse.org/show_bug.cgi?id=990006#c5 > > I propose this patch for trunk, 2.10 and 2.9. Acked-by: Steve Beattie <st...@nxnw.org> for all 3 branches. Thanks! > [ winbind-dac_override.diff ] > > === modified file 'profiles/apparmor.d/usr.sbin.winbindd' > --- profiles/apparmor.d/usr.sbin.winbindd 2015-07-30 20:03:02 + > +++ profiles/apparmor.d/usr.sbin.winbindd 2016-07-26 19:15:17 + > @@ -7,6 +7,7 @@ > >deny capability block_suspend, > > + capability dac_override, >capability ipc_lock, >capability setuid, > -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [Merge] ~sdeziel/apparmor-profiles/+git/apparmor-profiles:debian-833184 into apparmor-profiles:master
Review: Approve No worries, thanks for fixing! -- https://code.launchpad.net/~sdeziel/apparmor-profiles/+git/apparmor-profiles/+merge/301764 Your team AppArmor Developers is subscribed to branch apparmor-profiles:master. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [Merge] ~sdeziel/apparmor-profiles/+git/apparmor-profiles:thunderbird-45-refresh into apparmor-profiles:master
Review: Approve Looks good, thanks. Merged. (And also thanks for pushing me to figure out how to reuqest merges via launchpad). Do you expect enigmail to ever provide more than the single jar file? -- https://code.launchpad.net/~sdeziel/apparmor-profiles/+git/apparmor-profiles/+merge/301697 Your team AppArmor Developers is subscribed to branch apparmor-profiles:master. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [PATCH/apparmor-profiles] Add profile for /usr/share/update-notifier/notify-reboot-required
Hi, On Fri, Jul 01, 2016 at 09:46:10AM +0200, intrigeri wrote: > Steve Beattie wrote (30 Jun 2016 19:00:59 GMT) : > > +profile notify-reboot-required > > /usr/share/update-notifier/notify-reboot-required { > > On Debian Jessie and newer, this file is not provided by the > update-notifier package anymore: that binary package is now built from > src:gnome-packagekit. The update-notifier package there is a transitional empty package. > Instead, we've introduced a tiny package called reboot-notifier (in > testing/sid and in jessie-backports) that provides the same interface > as the old update-notifier's. > > I'm not sure how this works in Ubuntu, so I'd like to ask: was this > tested on a system where > /usr/share/update-notifier/notify-reboot-required is provided by the > reboot-notifier package, e.g. Debian testing/sid? Or only with > Ubuntu's update-notifier? I was unaware of the above. The profile was tested only with Ubuntu's update-notifier. That said, I pulled down the source package for reboot-notifier, and it's even more stripped down than the Ubuntu update-notifier script. Entirely untested with reboot-notifier, but the following should work: diff --git a/ubuntu/16.04/usr.share.update-notifier.notify-reboot-required b/ubuntu/16.04/usr.share.update-notifier.notify-reboot-required index 5649d0d..9e97035 100644 --- a/ubuntu/16.04/usr.share.update-notifier.notify-reboot-required +++ b/ubuntu/16.04/usr.share.update-notifier.notify-reboot-required @@ -4,13 +4,13 @@ #include -profile notify-reboot-required /usr/share/update-notifier/notify-reboot-required { +profile notify-reboot-required /usr/share/{update,reboot}-notifier/notify-reboot-required { #include /usr/bin/gettext Pix, - /usr/share/update-notifier/notify-reboot-required r, + /usr/share/{update,reboot}-notifier/notify-reboot-required r, /{var/,}run/reboot-required rw, /{var/,}run/reboot-required.pkgs rw, Unless you'd rather they be distinct profiles? (I'd apply the same changes to the copy in the 16.10/ directory as well.) Thanks fr the feedback! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [PATCH/apparmor-profiles] Add profile for /usr/share/update-notifier/notify-reboot-required
Signed-off-by: Steve Beattie <st...@nxnw.org> --- .../usr.share.update-notifier.notify-reboot-required| 17 + .../usr.share.update-notifier.notify-reboot-required| 17 + 2 files changed, 34 insertions(+) create mode 100644 ubuntu/16.04/usr.share.update-notifier.notify-reboot-required create mode 100644 ubuntu/16.10/usr.share.update-notifier.notify-reboot-required diff --git a/ubuntu/16.04/usr.share.update-notifier.notify-reboot-required b/ubuntu/16.04/usr.share.update-notifier.notify-reboot-required new file mode 100644 index 000..5649d0d --- /dev/null +++ b/ubuntu/16.04/usr.share.update-notifier.notify-reboot-required @@ -0,0 +1,17 @@ +# vim:syntax=apparmor +# Last Modified: Thu Jun 30 11:40:45 2016 +# Author: Steve Beattie <st...@nxnw.org> + +#include + +profile notify-reboot-required /usr/share/update-notifier/notify-reboot-required { + + #include + + /usr/bin/gettext Pix, + + /usr/share/update-notifier/notify-reboot-required r, + + /{var/,}run/reboot-required rw, + /{var/,}run/reboot-required.pkgs rw, +} diff --git a/ubuntu/16.10/usr.share.update-notifier.notify-reboot-required b/ubuntu/16.10/usr.share.update-notifier.notify-reboot-required new file mode 100644 index 000..5649d0d --- /dev/null +++ b/ubuntu/16.10/usr.share.update-notifier.notify-reboot-required @@ -0,0 +1,17 @@ +# vim:syntax=apparmor +# Last Modified: Thu Jun 30 11:40:45 2016 +# Author: Steve Beattie <st...@nxnw.org> + +#include + +profile notify-reboot-required /usr/share/update-notifier/notify-reboot-required { + + #include + + /usr/bin/gettext Pix, + + /usr/share/update-notifier/notify-reboot-required r, + + /{var/,}run/reboot-required rw, + /{var/,}run/reboot-required.pkgs rw, +} -- 2.7.4 -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [Merge] lp:~serge-hallyn/apparmor-profiles/apparmor-profiles into lp:apparmor-profiles
The proposal to merge lp:~serge-hallyn/apparmor-profiles/apparmor-profiles into lp:apparmor-profiles has been updated. Status: Approved => Merged For more details, see: https://code.launchpad.net/~serge-hallyn/apparmor-profiles/apparmor-profiles/+merge/291919 -- Your team AppArmor Developers is subscribed to branch lp:apparmor-profiles. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [announce] apparmor-profiles git tree now live
Hi, As promised, the apparmor-profiles git repo is now live, and the bzr repo will no longer be maintained (though are still available from launchpad). The git repo is available at: https://code.launchpad.net/~apparmor-dev/apparmor-profiles/+git/apparmor-profiles Please let me know if you have any questions. Thanks! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [RFC] test git tree for the apparmor-profiles repo
On Sat, Jun 25, 2016 at 08:39:45PM +0200, intrigeri wrote: > Hi, > > Steve Beattie wrote (24 Jun 2016 17:28:37 GMT) : > > I've created an updated git repo based on the current bzr repo at > > https://code.launchpad.net/~apparmor-dev/apparmor/+git/apparmor-profiles-test > > Thanks! > > I've had a look, diff'ed the tree with a clone made with > git-remote-bzr and they're identical. I also checked that merges seem > to be reflected correctly. > > > If we're happy with this tree, then we can promote it to/as the official > > profiles git tree. > > I say please go ahead! :) > > … and then we can work on reworking the repository layout as discussed > last year at DebConf (spoiler: using branches rather than > directories), and importing profiles from non-Ubuntu distros. > As announced earlier, I want to spend some time on that during the > (currently ongoing) DebCamp. Sorry for the delay, https://code.launchpad.net/~apparmor-dev/apparmor-profiles/+git/apparmor-profiles is now live. I'll kill the other git repos I've pushed and send out a formal announcement to the list. Thanks for your patience! -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [RFC] test git tree for the apparmor-profiles repo
On Fri, Jun 24, 2016 at 09:25:58AM +0200, intrigeri wrote: > Hi, > > Steve Beattie wrote (23 Feb 2016 19:14:22 GMT) : > > On Mon, Feb 22, 2016 at 06:10:25PM -0600, Tyler Hicks wrote: > >> On 2016-02-22 09:53:50, Steve Beattie wrote: > >> > I've created a test git repo of the apparmor-profiles tree at > >> > > >> > https://code.launchpad.net/~sbeattie/+git/apparmor-profiles-test > >> > > >> > based on a simple conversion of the bzr tree. > [...] > >> > Any feedback or advice is welcome. Thanks! > >> > >> Nice job! What conversion tool did you end up using? > > > For this tree, I used bzr fast-export | git fast-import. Sadly, I had > > to do it on an Ubuntu 14.04 host, as bzr-fastexport has been dropped > > from both debian and 16.04, due to RC bugs and a lack of support. :( > > I may have time during DebCamp (that just started) to look at > this tree. Do you want to update the test Git repo, or should I? > > If I get to it I'll also report back about the repo layout design we > came up with during DebConf15 with Christian and a bunch of Debian > folks. I might even try to convert the newly created Git repo to that > layout if I have time :) I've created an updated git repo based on the current bzr repo at https://code.launchpad.net/~apparmor-dev/apparmor/+git/apparmor-profiles-test For the record, the steps were: - mkdir apparmor-profiles-test && cd apparmor-profiles-test - git init . - bzr fast-export --export-marks=../apparmor-profiles.bzr ../../bzr/apparmor-profiles/ | git fast-import --export-marks=../apparmor-profiles.git and then git push to launchpad. If we're happy with this tree, then we can promote it to/as the official profiles git tree. -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [Merge] lp:~intrigeri/apparmor/add-firefox-esr-to-ubuntu-browsers into lp:apparmor
On Thu, Jun 23, 2016 at 06:51:14PM -, intrigeri wrote: > Two months later: ping? Sorry about that. > === modified file 'profiles/apparmor.d/abstractions/ubuntu-browsers' > --- profiles/apparmor.d/abstractions/ubuntu-browsers 2012-04-25 19:13:15 > + > +++ profiles/apparmor.d/abstractions/ubuntu-browsers 2016-04-24 14:26:52 > + > @@ -30,7 +30,7 @@ ># this should cover all firefox browsers and versions (including shiretoko ># and abrowser) >/usr/bin/firefox Cxr -> sanitized_helper, > - /usr/lib/firefox*/firefox*.sh Cx -> sanitized_helper, > + /usr/lib/firefox*/firefox*{,.sh} Cx -> sanitized_helper, The problem with this is that firefox*{,.sh} is equivalent to firefox*. Furthermore it matches the firefox binary /usr/lib/firefox/firefox as shipped in ubuntu, which the original pattern did not. But (and this is what prevented me from replying when the original merge request was proposed), I'm not sure what the implications of that change are, if any. The shipped firefox profile in ubuntu (16.04 LTS at least) has "/usr/lib/firefox/firefox{,*[^s][^h]}" as it's profile match, so potentially this could cause interference. Is there a more tightly bound pattern for the esr firefoxes that debian is shipping? -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ https://code.launchpad.net/~intrigeri/apparmor/add-firefox-esr-to-ubuntu-browsers/+merge/292725 Your team AppArmor Developers is requested to review the proposed merge of lp:~intrigeri/apparmor/add-firefox-esr-to-ubuntu-browsers into lp:apparmor. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [patch] Drop unused escape() function from aa.py
On Thu, Jun 16, 2016 at 10:32:53PM +0200, Christian Boltz wrote: > Besides being unused, this function contains a broken regex. > > References: https://bugs.launchpad.net/bugs/1593324 > > [ 01-drop-unused-escape-function.diff ] LGTM. Acked-by: Steve Beattie <st...@nxnw.org>. Thanks! > === modified file 'utils/apparmor/aa.py' > --- utils/apparmor/aa.py2016-06-01 19:04:13 + > +++ utils/apparmor/aa.py2016-06-16 20:28:09 + > @@ -3135,14 +3135,6 @@ > else: > raise AppArmorException(_('Unknown variable operation %(operation)s > for variable %(variable)s in %(file)s') % { 'operation': var_operation, > 'variable': list_var, 'file': filename }) > > - > -def escape(escape): > -escape = strip_quotes(escape) > -escape = re.sub('((? -if re.search('(\s|^$|")', escape): > -return '"%s"' % escape > -return escape > - > def write_header(prof_data, depth, name, embedded_hat, write_flags): > pre = ' ' * int(depth * 2) > data = [] -- Steve Beattie <sbeat...@ubuntu.com> http://NxNW.org/~steve/ signature.asc Description: PGP signature -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
[apparmor] [patch] translations: fix up msgfmt warnings
The following patch touches up the .po files that generate warnings when msgfmt processes them to create .mo files, at least with gettext 0.19.7-2ubuntu3 in Ubuntu 16.04 LTS. Example warning types cleaned up include: ce.po:7: warning: header field 'Last-Translator' still has the initial default value ce.po:7: warning: header field 'Language' missing in header de.po:6: warning: header field 'Language-Team' still has the initial default value This patch also fixes up po files where the Report-Msgid-Bugs-To: field had not been updated. Signed-off-by: Steve Beattie <st...@nxnw.org> --- binutils/po/de.po|3 ++- binutils/po/en_GB.po |3 ++- binutils/po/id.po|3 ++- binutils/po/pt.po|3 ++- binutils/po/ru.po|3 ++- parser/po/ce.po |5 +++-- parser/po/en_AU.po |2 +- parser/po/en_CA.po |2 +- parser/po/ug.po |2 +- utils/po/de.po |2 +- utils/po/en_GB.po|3 ++- utils/po/id.po |3 ++- utils/po/it.po |3 ++- utils/po/ko.po |2 +- utils/po/pl.po |3 ++- utils/po/pt.po |3 ++- utils/po/sv.po |3 ++- utils/po/tr.po |3 ++- utils/po/ug.po |5 +++-- utils/po/uk.po |2 +- 20 files changed, 36 insertions(+), 22 deletions(-) Index: b/parser/po/ce.po === --- a/parser/po/ce.po +++ b/parser/po/ce.po @@ -6,16 +6,17 @@ msgid "" msgstr "" "Project-Id-Version: apparmor\n" -"Report-Msgid-Bugs-To: FULL NAME <EMAIL@ADDRESS>\n" +"Report-Msgid-Bugs-To: apparmor list <apparmor@lists.ubuntu.com>\n" "POT-Creation-Date: 2014-09-13 00:11-0700\n" "PO-Revision-Date: 2014-08-13 08:08+\n" -"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" +"Last-Translator: apparmor list <apparmor@lists.ubuntu.com>\n" "Language-Team: Chechen <c...@li.org>\n" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n" "Content-Transfer-Encoding: 8bit\n" "X-Launchpad-Export-Date: 2016-02-02 05:10+\n" "X-Generator: Launchpad (build 17908)\n" +"Language: ce\n" #: ../parser_include.c:113 msgid "Error: Out of memory.\n" Index: b/parser/po/en_AU.po === --- a/parser/po/en_AU.po +++ b/parser/po/en_AU.po @@ -9,7 +9,7 @@ msgstr "" "Report-Msgid-Bugs-To: <apparmor@lists.ubuntu.com>\n" "POT-Creation-Date: 2014-09-13 00:11-0700\n" "PO-Revision-Date: 2013-11-14 22:00+\n" -"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" +"Last-Translator: apparmor list <apparmor@lists.ubuntu.com>\n" "Language-Team: English (Australia) <en...@li.org>\n" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n" Index: b/parser/po/en_CA.po === --- a/parser/po/en_CA.po +++ b/parser/po/en_CA.po @@ -9,7 +9,7 @@ msgstr "" "Report-Msgid-Bugs-To: <apparmor@lists.ubuntu.com>\n" "POT-Creation-Date: 2014-09-13 00:11-0700\n" "PO-Revision-Date: 2013-11-14 22:00+\n" -"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" +"Last-Translator: apparmor list <apparmor@lists.ubuntu.com>\n" "Language-Team: English (Canada) <en...@li.org>\n" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n" Index: b/parser/po/ug.po === --- a/parser/po/ug.po +++ b/parser/po/ug.po @@ -9,7 +9,7 @@ msgstr "" "Report-Msgid-Bugs-To: <apparmor@lists.ubuntu.com>\n" "POT-Creation-Date: 2014-09-13 00:11-0700\n" "PO-Revision-Date: 2013-11-14 22:00+\n" -"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" +"Last-Translator: apparmor list <apparmor@lists.ubuntu.com>\n" "Language-Team: Uyghur <u...@li.org>\n" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n" Index: b/utils/po/ko.po === --- a/utils/po/ko.po +++ b/utils/po/ko.po @@ -9,7 +9,7 @@ msgstr "" "Report-Msgid-Bugs-To: <apparmor@lists.ubuntu.com>\n" "POT-Creation-Date: 2014-09-14 19:29+0530\n" "PO-Revision-Date: 2013-11-20 22:28+\n" -"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n" +"Last-Translator: apparmor list <apparmor@lists.ubuntu.com>\n" "Language-Team: Korean <k...@li.org>\n" "MIME-Version: 1.0\n" "Content-Type: text/plain; charset=UTF-8\n&q
Re: [apparmor] [Merge] lp:~sdeziel/apparmor-profiles/apt-cacher-ng-systemd into lp:apparmor-profiles
Review: Approve Looks good, merged. Thanks! -- https://code.launchpad.net/~sdeziel/apparmor-profiles/apt-cacher-ng-systemd/+merge/293481 Your team AppArmor Developers is subscribed to branch lp:apparmor-profiles. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor
Re: [apparmor] [Merge] lp:~xfactor973/apparmor-profiles/ceph-apparmor-profiles into lp:apparmor-profiles
Review: Approve Hi, thanks for fixing up the pid/pids variables. I've gone ahead and merged this after also copying the profiles to the 16.04 and 16.10 trees -- I poked around very briefly at a 16.04 ceph install and didn't see anything radically different in FS layout that would give cause for concern (we'll obivously take updates if there things that need to be updated). While reviewing, I did have one question; both profiles have: owner /etc/ceph/* rw, Is it expected that ceph will need to write to arbitrary files under /etc/ceph/? What's the usage here? Thanks for the contribution! -- https://code.launchpad.net/~xfactor973/apparmor-profiles/ceph-apparmor-profiles/+merge/289844 Your team AppArmor Developers is subscribed to branch lp:apparmor-profiles. -- AppArmor mailing list AppArmor@lists.ubuntu.com Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/apparmor