Hello,
In order to enable the performance analyzer OpenSearch plugin, the /dev/shm/performanceanalyzer directory must be read / written by OpenSearch.
I would like to request approval and auto-connect of the system-files: performance-analyzer plug defined here.
Thank you
Hey @medib,
Any chance this can be adjusted to be /{dev,run}/shm/snap.@{SNAP_INSTANCE_NAME}? As per the base template: https://github.com/snapcore/snapd/blob/master/interfaces/apparmor/template.go#L386 this is allowed without requiring any interface at all.
Hey @emitorino,
Thanks for following up on this! Iām not sure how i missed the previous messages (sorry!).
I am working on testing your suggestion, will report back as soon as I get some results.
Thanks!
So it seems that the path can be configurable, so /dev/shm/snap.${SNAP_INSTANCE_NAME}/performanceanalyzer/ would work (I assume having an additional directory level below snap.${SNAP_INSTANCE_NAME} is ok?).
However, the plugin requires the said directory to be already created, and I canāt seem to be able to do it from the install hook (in --jailmode at least).
mkdir -p "/dev/shm/snap.${SNAP_INSTANCE_NAME}/performanceanalyzer/" throws a ācannot create directory ā/dev/shm/snap.opensearchā: Permission deniedā error.
Do I perhaps need some system-files -> write plug on the install hook for this?
I think you might be able to use the private option with the shared-memory interface in that case (since you then get a private /dev/shm which you can do what you want with, but only within your snap, it cannot be shared with other snaps etc). https://snapcraft.io/docs/shared-memory-interface
Thanks @alexmurray! having a dedicated /dev/shm would be fine in this case.
It does seem however that iām still unable to create a directory under /dev/shm.
plugs:
shmem-perf-analyzer:
interface: shared-memory
private: true
apps:
daemon:
daemon: simple
plugs:
- shmem-perf-analyzer
The service attempts to create the said directory: (also tried to manually do it on the install hook)
==> apparmor="DENIED" operation="mkdir" class="file" profile="snap.opensearch.daemon" name="/dev/shm/performanceanalyzer/" pid=71584 comm="java" requested_mask="c" denied_mask="c" fsuid=584788 ouid=584788
Can you try running the following (hopefully have the right name of the profile):
grep /dev/shm /var/lib/snapd/apparmor/profiles/snap.opensearch.daemon
Also what is the output of: snap connections opensearch?
Here is the output:
> grep /dev/shm /var/lib/snapd/apparmor/profiles/snap.opensearch.daemon
# App-specific access to files and directories in /dev/shm. We allow file
# access in /dev/shm for shm_open() and files in subdirectories for open()
# Description: Allow access to everything in private /dev/shm
"/dev/shm/*" mrwlkix,
And the list of connections:
> snap connections opensearch
Interface Plug Slot Notes
log-observe opensearch:log-observe :log-observe manual
mount-observe opensearch:mount-observe :mount-observe manual
network opensearch:network :network -
network-bind opensearch:network-bind :network-bind -
process-control opensearch:process-control :process-control manual
shared-memory opensearch:shmem-perf-analyzer :shared-memory -
system-files opensearch:sys-fs-cgroup-service :system-files manual
system-observe opensearch:system-observe :system-observe manual
Ah apologies, I see now that I was confused about how snapd was preparing the AppArmor profile in this case - and so the profile does not contain all the permissions that I had thought it would.
However, what is there is correct for shared-memory with private: true - I wonder if perhaps snapd should be updated to use a more permissive rule for the Allow access to everything in private /dev/shm case, ie:
"/dev/shm/**" mrwlkix,
which would allow to create directories under this path (rather than just files) - it would be worth testing this by editing the profile by hand to make this change (ie. make the single asterisk in this rule a double asterisk) and then running apparmor_parser -r /var/lib/snapd/apparmor/profiles/snap.opensearch.daemon and then re-start the opensearch daemon and see if this is then allowed.
If so we could look at making an update to this interface in snapd to support this use-case.
@jamesh since you did the original work on the private shared memory interface I would be interested to get your thoughts on this proposed change. Thanks.
I donāt see any obvious security problems with allowing a snap with private shared-memory access to create and access subdirectories under /dev/shm.
The application use case seems a bit weird though, since /dev/shm is really only there to support the POSIX shared memory and semaphore APIs. If they want to store other temporary files, /tmp is the obvious choice. But if modifying the upstream is not on the cards, the AppArmor change seems reasonable.
Thank you! the double asterisks ** fixes it. I can make a PR - it seems straightfoward. (PR)
Now (after this test), It seems I will also need read access to /proc/p_id/task/task_id/schedstat, do you think it would make sense I patch the system-observe interface by adding @{PROC}/*/{,task/*/}schedstat r,?
audit[121170]: AVC apparmor="DENIED" operation="open" class="file" profile="snap.opensearch.daemon" name="/proc/121170/task/121647/schedstat" pid=121170 comm="pa-collectors-t" requested_mask="r" denied_mask="r" fsuid=584788 ouid=584788
Hi @medib,
I see this PR has been merged. Itās been one month since we last discussed and I see opensearch successfully published in the store, so whatās the status of this request?
Hi @emitorino - both PRs were merged - Iām waiting for the new version of snapd 2.60.1 to be released on stable, so those changes can be picked up.
Once thatās done, I can enable the performance analyzer on the OpenSearch snap (which currently doesnāt support the said plugin)
Please feel free to mark it as done - thanks
Thanks for the confirmation @medib. I am then removing this request from our review queue. Feel free to write here again if for some reasons you experience issues with the new snapd version.
Thanks!