apparmor profiles are not recognized

[lacygoill]

Steps to reproduce

Run this shell command:

vim -Nu NONE --cmd 'filetype on' +'echomsg &filetype' /etc/apparmor.d/usr.bin.man

man is echo'ed (and the file is highlighted as a manpage).

That's wrong: /etc/apparmor.d/usr.bin.man is an apparmor profile, not a manpage.

Expected behavior

apparmor is echo'ed.

Operating system

Ubuntu 20.04.3 LTS

Version of Vim

8.2 Included patches: 1-3720

Patch

Patch fixing the issue:

diff --git a/runtime/filetype.vim b/runtime/filetype.vim
index f687cc77a..024a977d7 100644
--- a/runtime/filetype.vim
+++ b/runtime/filetype.vim
@@ -97,6 +97,10 @@ au BufNewFile,BufRead *.run			setf ampl
 " Ant
 au BufNewFile,BufRead build.xml			setf ant
 
+" Apparmor
+au BufNewFile,BufRead /etc/apparmor.d/*				setf apparmor
+au BufNewFile,BufRead /usr/share/apparmor/extra-profiles/*	setf apparmor
+
 " Arduino
 au BufNewFile,BufRead *.ino,*.pde		setf arduino
 

[brammool]

A pattern ending in a star is probably not a good idea. E.g. what if there is a LICENSE or README file in that directory?
We should make the pattern much stricter. Can we match with *.man perhaps?

[lacygoill]

A pattern ending in a star is probably not a good idea.

Maybe, but if that's an issue, then there are already existing patterns in $VIMRUNTIME/filetype.vim which suffer from it. For example:

vim -Nu NONE --cmd 'filetype on' +'echomsg &filetype' /tmp/etc/apt/apt.conf.d/README

aptconf is echo'ed. But since the file is named README, shouldn't it be text?

---

Also, on Ubuntu, we can install the apparmor-utils package:

$ sudo apt-get install apparmor-utils

The latter installs a syntax file for Vim:

/usr/share/vim/addons/syntax/apparmor.vim

The latter contains this comment:

" stick this file into ~/.vim/syntax/ and add these commands into your .vimrc
" to have vim automagically use this syntax file for these directories:
"
" autocmd BufNewFile,BufRead /etc/apparmor.d/*                      set syntax=apparmor
" autocmd BufNewFile,BufRead /usr/share/apparmor/extra-profiles/*   set syntax=apparmor

That's where I copied the patterns from.

Can we match with *.man perhaps?

No, it would not be enough. Here, the name of the file is the path to the man(1) binary after replacing the slashes with dots. But there could be apparmor profiles for other binaries, like firefox, in which case the filename would not end with man but with firefox (usr.bin.firefox).

Maybe we could assert the presence of a dot:

au BufNewFile,BufRead /etc/apparmor.d/*.*                      setf apparmor
au BufNewFile,BufRead /usr/share/apparmor/extra-profiles/*.*   setf apparmor

But that would leave files such as /etc/apparmor.d/lightdm-guest-session.
We could try to inspect the contents of the file and look for an #include directive.
But that would leave files such as /etc/apparmor.d/abstractions/aspell, where there is no #include, and the file does not contain any dot.

It's not perfect, but how about this patch:

diff --git a/runtime/filetype.vim b/runtime/filetype.vim
index f687cc77a..f2f55f1d5 100644
--- a/runtime/filetype.vim
+++ b/runtime/filetype.vim
@@ -97,6 +97,12 @@ au BufNewFile,BufRead *.run			setf ampl
 " Ant
 au BufNewFile,BufRead build.xml			setf ant
 
+" Apparmor
+au BufNewFile,BufRead /etc/apparmor.d/*{README,LICENSE}		setf text
+au BufNewFile,BufRead /usr/share/apparmor/extra-profiles/*{README,LICENSE}	setf text
+au BufNewFile,BufRead /etc/apparmor.d/*				setf apparmor
+au BufNewFile,BufRead /usr/share/apparmor/extra-profiles/*	setf apparmor
+
 " Arduino
 au BufNewFile,BufRead *.ino,*.pde		setf arduino
 

[lacygoill]

E.g. what if there is a LICENSE or README file in that directory?

Actually, I think apparmor(7) reads all the files under /etc/apparmor.d/, including one named README. That would explain why – on my machine – all the lines in /etc/apparmor.d/local/README start with the comment leader #.

This means that there is no way for Vim to know in advance whether a file is meant to be read by a human or by apparmor(7). Vim would have to read all the lines, and check whether at least one is not commented. And even then, what if the file contains code which is commented, but meant to be uncommented on-demand by a sysadmin?

IOW, a README file is technically an apparmor file, not a text file, because it's read by apparmor(7). The original patch is OK.

[brammool]

> A pattern ending in a star is probably not a good idea. Maybe, but if that's an issue, then there are already existing patterns in `$VIMRUNTIME/filetype.vim` which suffer from it. For example: vim -Nu NONE --cmd 'filetype on' +'echomsg &filetype' /tmp/etc/apt/apt.conf.d/README `aptconf` is echo'ed. But since the file is named `README`, shouldn't it be `text`?

Yes. Although the pattern for aptconf doesn't end in a star, it does match many file names. Moving it down to the section with patterns ending in a star isn't sufficient... Ah, the pattern matching README was changed to use "setf" and now it's priority is too low. I'll move it above the patterns ending in star.

Also, on Ubuntu, we can install the `apparmor-utils` package: $ sudo apt-get install apparmor-utils The latter installs a syntax file for Vim: /usr/share/vim/addons/syntax/apparmor.vim The latter contains this comment: " stick this file into ~/.vim/syntax/ and add these commands into your .vimrc " to have vim automagically use this syntax file for these directories: " " autocmd BufNewFile,BufRead /etc/apparmor.d/* set syntax=apparmor " autocmd BufNewFile,BufRead /usr/share/apparmor/extra-profiles/* set syntax=apparmor That's where I copied the patterns from.

Often a package for a specificy type of file gives priority to that type over many other things. Other users that just install Vim have other priorities.

> Can we match with *.man perhaps? No, it would not be enough. Here, the name of the file is the path to the `man(1)` binary after replacing the slashes with dots. But there could be apparmor profiles for other binaries, like firefox, in which case the filename would not end with `man` but with `firefox` (`usr.bin.firefox`). Maybe we could assert the presence of a dot: au BufNewFile,BufRead /etc/apparmor.d/*.* setf apparmor au BufNewFile,BufRead /usr/share/apparmor/extra-profiles/*.* setf apparmor But that would leave files such as `/etc/apparmor.d/lightdm-guest-session`. We could try to inspect the contents of the file and look for an `#include` directive. But that would leave files such as `/etc/apparmor.d/abstractions/aspell`, where there is no `#include`, and the file does not contain any dot. It's not perfect, but how about this patch: ```diff diff --git a/runtime/filetype.vim b/runtime/filetype.vim index f687cc77a..f2f55f1d5 100644 --- a/runtime/filetype.vim +++ b/runtime/filetype.vim @@ -97,6 +97,12 @@ au BufNewFile,BufRead *.run setf ampl " Ant au BufNewFile,BufRead build.xml setf ant +" Apparmor +au BufNewFile,BufRead /etc/apparmor.d/*{README,LICENSE} setf text +au BufNewFile,BufRead /usr/share/apparmor/extra-profiles/*{README,LICENSE} setf text +au BufNewFile,BufRead /etc/apparmor.d/* setf apparmor +au BufNewFile,BufRead /usr/share/apparmor/extra-profiles/* setf apparmor + " Arduino au BufNewFile,BufRead *.ino,*.pde setf arduino ```

I'll make a patch for aptconf, that should give a hint for apparmor. I'll also match AUTHORS, COPYING and LICENSE next to README, since these are often used.

…

-- ALL: A witch! A witch! WITCH: It's a fair cop. ALL: Burn her! Burn her! Let's make her into a ladder. "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD /// Bram Moolenaar -- ***@***.*** -- http://www.Moolenaar.net \\\ /// \\\ \\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[brammool]

> E.g. what if there is a LICENSE or README file in that directory? Actually, I think `apparmor(7)` reads all the files under `/etc/apparmor.d/`, including one named `README`. That would explain why – on my machine – all the lines in `/etc/apparmor.d/local/README` start with the comment leader `#`. This means that there is no way for Vim to know in advance whether a file is meant to be read by a human or by `apparmor(7)`. Vim would have to read all the lines, and check whether at least one is not commented. And even then, what if the file contains code which is commented, but meant to be uncommented on-demand by a sysadmin? IOW, a `README` file *is* technically an apparmor file, not a text file, because it's read by `apparmor(7)`. The original patch is OK.

OK, that's different then. How about the extra-profiles directory?

…

-- BEDEVERE: And that, my lord, is how we know the Earth to be banana-shaped. "Monty Python and the Holy Grail" PYTHON (MONTY) PICTURES LTD /// Bram Moolenaar -- ***@***.*** -- http://www.Moolenaar.net \\\ /// \\\ \\\ sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ /// \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

[lacygoill]

How about the extra-profiles directory?

It's full of apparmor profiles, which are meant to be copied by the sysadmin under /etc/apparmor.d/. But there is one README file (/usr/share/apparmor/README) where the lines are not commented. Not sure it's worth making our code more complicated just to support this file. I think whoever wrote it should have commented the lines.

[lacygoill]

I'll make a patch for aptconf, that should give a hint for apparmor.

muttrc is also affected by the issue. This shell command:

vim -Nu NONE --cmd 'filetype on' +'echomsg &filetype' /tmp/etc/Muttrc.d/README

Outputs muttrc. It should output text.

Patch fixing the issue:

diff --git a/runtime/filetype.vim b/runtime/filetype.vim
index 00711d7b5..7cd072b00 100644
--- a/runtime/filetype.vim
+++ b/runtime/filetype.vim
@@ -1123,9 +1123,6 @@ au BufNewFile,BufRead *.msql			setf msql
 " Mysql
 au BufNewFile,BufRead *.mysql			setf mysql
 
-" Mutt setup files (must be before catch *.rc)
-au BufNewFile,BufRead */etc/Muttrc.d/*		call s:StarSetf('muttrc')
-
 " Tcl Shell RC file
 au BufNewFile,BufRead tclsh.rc			setf tcl
 
@@ -2185,6 +2182,9 @@ au BufNewFile,BufRead */etc/apache2/*.conf*,*/etc/apache2/conf.*/*,*/etc/apache2
 " APT config file
 au BufNewFile,BufRead */etc/apt/apt.conf.d/{[-_[:alnum:]]\+,[-_.[:alnum:]]\+.conf} call s:StarSetf('aptconf')
 
+" Mutt setup files (must be before catch *.rc)
+au BufNewFile,BufRead */etc/Muttrc.d/*		call s:StarSetf('muttrc')
+
 " Asterisk config file
 au BufNewFile,BufRead *asterisk/*.conf*		call s:StarSetf('asterisk')
 au BufNewFile,BufRead *asterisk*/*voicemail.conf* call s:StarSetf('asteriskvm')

[lacygoill]

Patch fixing the issue:

Ah, the patch might be wrong. I didn't take into account the "(must be before catch *.rc)" comment.

[brammool]

See patch 8.2.3730

[lacygoill]

But there is one README file (/usr/share/apparmor/README) where the lines are not commented. Not sure it's worth making our code more complicated just to support this file. I think whoever wrote it should have commented the lines.

Alternatively, how about this patch:

diff --git a/runtime/filetype.vim b/runtime/filetype.vim
index 705c2dce4..1a25f2e7a 100644
--- a/runtime/filetype.vim
+++ b/runtime/filetype.vim
@@ -2183,6 +2183,10 @@ au BufNewFile,BufRead proftpd.conf*					call s:StarSetf('apachestyle')
 au BufNewFile,BufRead access.conf*,apache.conf*,apache2.conf*,httpd.conf*,srm.conf*	call s:StarSetf('apache')
 au BufNewFile,BufRead */etc/apache2/*.conf*,*/etc/apache2/conf.*/*,*/etc/apache2/mods-*/*,*/etc/apache2/sites-*/*,*/etc/httpd/conf.*/*,*/etc/httpd/mods-*/*,*/etc/httpd/sites-*/*,*/etc/httpd/conf.d/*.conf*		call s:StarSetf('apache')
 
+" Apparmor
+au BufNewFile,BufRead /etc/apparmor.d/*				setf apparmor
+au BufNewFile,BufRead /usr/share/apparmor/extra-profiles/*	setf apparmor
+
 " APT config file
 au BufNewFile,BufRead */etc/apt/apt.conf.d/{[-_[:alnum:]]\+,[-_.[:alnum:]]\+.conf} call s:StarSetf('aptconf')
 

It detects the README file as text, and all the profiles as apparmor.