Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tunables for file formats? #525

Open
odomingao opened this issue Sep 24, 2024 · 11 comments
Open

Tunables for file formats? #525

odomingao opened this issue Sep 24, 2024 · 11 comments

Comments

@odomingao
Copy link
Contributor

A way to further confine a few tools, I think, would be to add tunables for file extensions. For instance: @{document_formats} would contain doc docx odt pdf etc., and then we could restrict e.g. the libreoffice profile to only read and write to files that have the whitelisted extensions, rather than every file.

@roddhjav
Copy link
Owner

roddhjav commented Sep 25, 2024

No, this is not a good solution: a program is not legitimate to open a file based on its extension but based on its location. It is why all locations should be labelled.

Such a solution would also be a security issue as it is quite obvious for some other programs to simply rename a file to have the right extension. Whereas, when you give access to a directory, they cannot escape it at all.
That would be different on SELinux as it is not path based. However, on AppArmor, setting a policy according to a path that can be changed by a program is not good.

It would also be a maintainability hell:

  • By itself is a red flag (as I am the one who will have to maintain it)
  • A lot of programs (like LibreOffice) need full access to some cache/lock file on top of the file in use.
  • It is also a security issue as it would tend to broken program. And people don't use broken program.

The solution to improve the current setup that is not perfect, is prompting: https://discourse.ubuntu.com/t/ubuntu-desktop-s-24-10-dev-cycle-part-5-introducing-permissions-prompting/47963

@odomingao
Copy link
Contributor Author

odomingao commented Sep 25, 2024

The reason for my suggestion is that a lot of profiles only have write access to user directories that a lot of other profiles also have write access to, like Downloads or Documents. So, if a user wants to download a sensitive file from a web browser or from a different app, they would normally have to resort to saving that file in a weird directory if they don't want that file to be readable by other programs (like, in .config/ or .local/share/ which usually are only read/writable to one profile).

So, how about adding a private user directory for each app profile, in e.g. ~/Private/Program?

Such a solution would also be a security issue as it is quite obvious for some other programs to simply rename a file to have the right extension.

Could you clarify? Do you mean that a program can only read/write to /**/*.jpg could they rename ~/.zshrc to something.jpg to be able to read it? I must be misunderstanding

@beroal
Copy link
Contributor

beroal commented Sep 25, 2024

So, how about adding a private user directory for each app profile, in e.g. ~/Private/Program?

I do something similar. I don't use the standard document directories and have a separate directory for every program. For example, if I write a document and send it in an email, I move the document from LibreOffice's directory to ThunderBird's directory. It's PITA.

IMHO, multiplying directories isn't a good solution. The best solution is the so called PowerBox GUI. In principle, it's possible to create a privileged service which updates AppArmor profiles and is driven by such a GUI.

@odomingao
Copy link
Contributor Author

I wish this was a part of the XDG spec.

Firejail does this with the private folders but IIRC (and please correct me if I'm wrong), it clutters the home directory by creating the folders directly there rather than in a proper sub directory. This should really be standardized somehow.

I'm also now doing this manually with overrides, in ~/AppData rather than ~/Private because I think that the application's private user files should still be accessible to the file manager.

Prompting is great but it doesn't really solve the problem, in my opinion.

@curiosityseeker
Copy link
Contributor

No, this is not a good solution: a program is not legitimate to open a file based on its extension but based on its location. It is why all locations should be labelled.

Yes, but adding permissions to only open/write files based on their extensions enhances security. For example, the okular profile contains include <abstractions/user-write-strict> but that abstraction includes, e.g., owner @{user_projects_dirs}/{,**} wl, , owner @{user_work_dirs}/{,**} wl, and others. Hence, okular could theoretically delete/tamper with files in those directories which it should not be able to manipulate. Restricting this to PDF files would considerably reduce this risk.

Such a solution would also be a security issue as it is quite obvious for some other programs to simply rename a file to have the right extension.

Yes, true. But if I have the choice between

  • a situation where the application could delete/manipulate any files in the allowed user directories
  • and a situation where a file could be renamed to have the "right" extension but would not be able to seriously harm our system as the respective application is confined with AppArmor (that's why we're doing this, don't we?)

I would opt for the second option.

It would also be a maintainability hell:

* By itself is a red flag (as I am the one who will have to maintain it)

I think the the list of extensions as used by Morfikov for, e.g., mpv, vlc, okular is a very good start. This lists should hardly change, the maintenance burden should therefore be minimal.

* A lot of programs (like LibreOffice) need full access to some cache/lock file on top of the file in use.

The Libreoffice profile from Morfikov (which originally comes from Canonical) has been in use in Ubuntu for years. Access to cache/lock files is still possible.

* It is also a security issue as it would tend to broken program. And people don't use broken program.

Again, the LibreOffice profile on Ubuntu has been in use for years. It seems that it hasn't caused problems. Besides, security issues by deleting/manipulating files for which an application should not have any access is even more a security issue.

@roddhjav
Copy link
Owner

roddhjav commented Oct 1, 2024

Hum, I may change my mind on this. Can you open a PR that would create a file in apparmor.d/tunables/multiarch.d/extensions and propose these extensions list.

We should however document how these extension name should be used. Something like:

  • No: /**@{ext}, or even @{HOME}/**@{ext}
  • Yes: @{user_work_dirs}/**@{ext} rwl,
  • Can be used to restrict access even more, not less

Also, I would like to note: even if the sandboxing tool (flatpak/snap) are far to be perfect, correctly handled they can provide more security (in more separation) than confinement that is more suitable for app that you can't confine easily (ie: that have too many interaction with other programs).

@odomingao
Copy link
Contributor Author

I created it on #543.

Also, I would like to note: even if the sandboxing tool (flatpak/snap) are far to be perfect, correctly handled they can provide more security (in more separation) than confinement that is more suitable for app that you can't confine easily (ie: that have too many interaction with other programs).

Even better should be to use apparmor alongside a namespaces sandbox manager, as both have their shortcomings and having extra protections is always good

@curiosityseeker
Copy link
Contributor

Hum, I may change my mind on this. Can you open a PR that would create a file in apparmor.d/tunables/multiarch.d/extensions and propose these extensions list.

I'd love to do this, and I have a file that is nearly finished. However ...

We should however document how these extension name should be used. Something like:

* No:  `/**@{ext}`, or even `@{HOME}/**@{ext}`

* Yes: `@{user_work_dirs}/**@{ext} rwl,`

* Can be used to restrict access even more, not less

... @{user_work_dirs}/**@{ext} rwl, would not achieve anything as long as the respective profiles contain the user-write-strict abstraction which allows write access to all files in those directories. Hence, such a rule would not restrict anything more. I don't know how we should solve this problem under the current profile structure.

@roddhjav
Copy link
Owner

roddhjav commented Oct 3, 2024

@{user_work_dirs}/**@{ext} rwl, would not achieve anything as long as the respective profiles contain the user-write-strict

You are right. This structure need to fully be revisited. However, I am still looking for a solution that would please everyone and that would be technically compatible with all security objectives.

Currently we have the following positions (that are all legitimate tbh):

  • @{user_work_dirs} is not strict enough, we need: @{user_work_dirs}/**@{ext}
  • @{user_work_dirs} is too strict, we need user-read-strict (ie all labelled directories)
  • Labelled directories are too strict, we need user-read (ie all non hidden files)
  • Just @{HOME}/**@{ext}
  • Just prompt over non hidden files

@curiosityseeker
Copy link
Contributor

Currently we have the following positions (that are all legitimate tbh):

* `@{user_work_dirs}` is not strict enough, we need: `@{user_work_dirs}/**@{ext}`

* `@{user_work_dirs}` is too strict, we need `user-read-strict` (ie all labelled directories)

* Labelled directories are too strict, we need `user-read` (ie all non hidden files)

* Just `@{HOME}/**@{ext}`

* Just `prompt` over non hidden files

Yes, it's difficult. But whichever solution comes out of this, I think that removing the user-read-strict and user-write-strict abstractions and adding an appropriate variable (containing the directories in those abstarctions) to tunables would be a much more flexible solution. The distinction between "read" and "write" would no longer be necessary as the proper permissions (with or without **@{ext}) would be defined in the profiles themselves.

A similar logic would apply to user-read and user-write as well although the paths therein are not identical unlike in the 2 abs above.

@curiosityseeker
Copy link
Contributor

curiosityseeker commented Oct 3, 2024

I created it on #543.

Thanks! However, it's not comprehensive enough as capital letters are not taken into account. The (still half-baked) solution I had mind (which is largely copied from the Canonical profile for LibreOffice and from Morfikow) is:

# apparmor.d - Full set of apparmor profiles
# Copyright (C) 2024 curiosityseeker
# Copyright (C) 2024 Alexandre Pujol <[email protected]>
# SPDX-License-Identifier: GPL-2.0-only


# Office suites extensions
# Defines all common supported file formats

#Generic
#.txt
@{office_ext} = [tT][xX][tT]
#All the open document format
@{office_ext} += {,f,F}[oO][dDtT][tTsSpPbBgGfF]
#.xml and xsl
@{office_ext} += [xX][mMsS][lL]
#.pdf
@{office_ext} += [pP][dD][fF]
#Unified office format
@{office_ext} += [uU][oO][fFtTsSpP]
#(x)htm(l)
@{office_ext} += {,x,X}[hH][tT][mM]{,l,L}
#.epub
@{office_ext} += [eE][pP][uU][bB]
#.ps (printing to file)
@{office_ext} += [pP][sS]

#Images
@{office_ext} += [jJ][pP][gG]
@{office_ext} += [jJ][pP][eE][gG]
@{office_ext} += [pP][nN][gG]
@{office_ext} += [sS][vV][gG]
@{office_ext} += [sS][vV][gG][zZ]99251
@{office_ext} += [tT][iI][fF]
@{office_ext} += [tT][iI][fF][fF]

#Writer
@{office_ext} += [dD][oO][cCtT]{,x,X}
@{office_ext} += [rR][tT][fF]

#Calc
@{office_ext} += [xX][lL][sStT]{,x,X,m,M}
@{office_ext} += [xX][lL][wW]
#.dif dbf
@{office_ext} += [dD][iIbB][fF]
#.tsv .csv
@{office_ext} += [cCtT][sS][vV]
@{office_ext} += [sS][lL][kK]

#Impress/Draw
@{office_ext} += [pP][pP][tTsS]{,x,X}
@{office_ext} += [pP][oO][tT]{,m,M}
#Photoshop
@{office_ext} += [pP][sS][dD]

#Math
@{office_ext} += [mM][mM][lL]


# Video/audio extensions:
# a52, aac, ac3, mka, flac, mp1, mp2, mp3, mpc, oga, oma, wav, wv, wm, wma, 3g2, 3gp, 3gp2, 3gpp,
# asf, avi, divx, m1v, m2v, m4v, mkv, mov, mp4, mpa, mpe, mpg, mpeg, mpeg1, mpeg2, mpeg4, ogg, ogm,
# ogx, ogv, rm, rmvb, webm, wmv, wtv, mp2t
@{audiovideo_ext}  = [aA]{52,[aA][cC],[cC]3}
@{audiovideo_ext} += [mM][kK][aA]
@{audiovideo_ext} += [fF][lL][aA][cC]
@{audiovideo_ext} += [mM][pP][123cC]
@{audiovideo_ext} += [oO][gGmM][aA]
@{audiovideo_ext} += [wW]{,[aA]}[vV]
@{audiovideo_ext} += [wW][mM]{,[aA]}
@{audiovideo_ext} += 3[gG]{[2pP],[pP][2pP]}
@{audiovideo_ext} += [aA][sS][fF]
@{audiovideo_ext} += [aA][vV][iI]
@{audiovideo_ext} += [dD][iI][vV][xX]
@{audiovideo_ext} += [mM][124][vV]
@{audiovideo_ext} += [mM][kKoO][vV]
@{audiovideo_ext} += [mM][pP][4aAeEgG]
@{audiovideo_ext} += [mM][pP][eE][gG]{,[124]}
@{audiovideo_ext} += [oO][gG][gGmMxXvV]
@{audiovideo_ext} += [rR][mM]{,[vV][bB]}
@{audiovideo_ext} += [wW][eE][bB][mM]
@{audiovideo_ext} += [wW][mMtT][vV]
@{audiovideo_ext} += [mM][pP]2[tT]

# Subtitle extensions:
# srt, txt, sub
@{subtitle_ext} += [sS][rR][tT]
@{subtitle_ext} += [tT][xX][tT]
@{subtitle_ext} += [sS][uU][bB]

# Playlist extensions:
# m3u, m3u8, pls
@{playlist_ext} += [mM]3[uU]{,8}
@{playlist_ext} += [pP][lL][sS]

# Extensions for PDF readers
# epub, pdf
@{pdf_ext} = [eE][pP][uU][bB] [pP][dD][fF]

# Image extensions:
# avi, bmp, dng, gif, heic, jpg, jpeg, mov, mp4, mts, png, rw2, svg, thm, tif, tiff, webp, xcf
@{image_ext}  = [aA][vV][iI]
@{image_ext} += [bB][mM][pP]
@{image_ext} += [dD][nN][gG]
@{image_ext} += [gG][iI][fF]
@{image_ext} += [hH][eE][iI][cC]
@{image_ext} += [jJ][pP][eE][gG] [jJ][pP][gG]
@{image_ext} += [mM][oO][vV]
@{image_ext} += [mM][pP][4]
@{image_ext} += [mM][tT][sS]
@{image_ext} += [pP][nN][gG]
@{image_ext} += [rR][wW][2]
@{image_ext} += [sS][vV][gG] [sS][vV][gG][zZ]
@{image_ext} += [tT][hH][mM]
@{image_ext} += [tT][iI][fF] [tT][iI][fF][fF]
@{image_ext} += [wW][eE][bB][pP]
@{image_ext} += [xX][cC][fF]

Even better should be to use apparmor alongside a namespaces sandbox manager, as both have their shortcomings and having extra protections is always good

Yes, but this can lead to new problems (example).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants