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

debian: Add rule to allow usage of /var/tmp directory (QEMU) #944

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

stefanberger
Copy link
Owner

@stefanberger stefanberger commented Nov 5, 2024

QEMU's functional tests need access to /var/tmp/. To avoid the following
type of AppArmor permission failures add a rule that allows access to
/var/tmp/
.

type=AVC msg=audit(1730829888.863:260): apparmor="DENIED"
operation="mknod" class="file" profile="swtpm"
name="/var/tmp/qemu_3r9txw7z/swtpm-socket" pid=3925 comm="swtpm"
requested_mask="c" denied_mask="c" fsuid=1000 ouid=1000FSUID="stefanb"
OUID="stefanb"

[ To run the QEMU's functional tests use the following command:
make check-functional ]

@stefanberger
Copy link
Owner Author

FYI, @lvoytek . Let me know if you have a comment...

@lvoytek
Copy link
Contributor

lvoytek commented Nov 5, 2024

FYI, @lvoytek . Let me know if you have a comment...

Looks good to me, tmp access should be fine and the user-tmp abstraction is a good way to add it. The commit title says /var/lib directory though

@stefanberger stefanberger changed the title debian: Add rule to allow usage of /var/lib directory (QEMU) debian: Add rule to allow usage of /var/tmp directory (QEMU) Nov 5, 2024
@stefanberger
Copy link
Owner Author

FYI, @lvoytek . Let me know if you have a comment...

Looks good to me, tmp access should be fine and the user-tmp abstraction is a good way to add it. The commit title says /var/lib directory though

Thanks for the feedback. I fixed the title.

@stefanberger
Copy link
Owner Author

stefanberger commented Nov 5, 2024

@lvoytek I now filed this bug for Ubuntu: https://bugs.launchpad.net/ubuntu/+source/swtpm/+bug/2086736

@stefanberger
Copy link
Owner Author

stefanberger commented Nov 5, 2024

@lvoytek Actually, the original bug report shows this audit entry:

audit(1730822474.384:446): apparmor="DENIED" operation="mknod"
profile="swtpm"
name="/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/tests/functional/arm/test_arm_aspeed.AST2x00Machine.test_arm_ast2600_evb_buildroot_tpm/qemu-machine-hhuvwytc/.lock"
pid=2820156 comm="swtpm" requested_mask="c" denied_mask="c" fsuid=1000
ouid=1000

So they are using /mnt, for which we would probably need a rule like owner /mnt/** rwkl or even owner /** rwkl.

@lvoytek
Copy link
Contributor

lvoytek commented Nov 5, 2024

@lvoytek Actually, the original bug report show this audit entry:

audit(1730822474.384:446): apparmor="DENIED" operation="mknod"
profile="swtpm"
name="/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/tests/functional/arm/test_arm_aspeed.AST2x00Machine.test_arm_ast2600_evb_buildroot_tpm/qemu-machine-hhuvwytc/.lock"
pid=2820156 comm="swtpm" requested_mask="c" denied_mask="c" fsuid=1000
ouid=1000

So they are using /mnt, for which we would probably need a rule like owner /mnt/** rwkl or even owner /** rwkl.

That's fair, I guess it would be reasonable to allow swtpm full access to any file that it specifically owns. We would also be able to clean up the apparmor profile a lot by removing all the lines tagged with owner, and it would likely prevent a lot of future unnecessary apparmor denials. I'll check with my team to make sure that would be fine to add into Ubuntu.

@stefanberger
Copy link
Owner Author

We would also be able to clean up the apparmor profile a lot by removing all the lines tagged with owner, and it would likely prevent a lot of future unnecessary apparmor denials. I'll check with my team to make sure that would be fine to add into Ubuntu.

Yes, if this is indeed considered a plus. All file access restrictions that swtpm once had due to the profile will be gone...

@stefanberger stefanberger marked this pull request as draft November 5, 2024 22:11
@stefanberger
Copy link
Owner Author

We would also be able to clean up the apparmor profile a lot by removing all the lines tagged with owner, and it would likely prevent a lot of future unnecessary apparmor denials. I'll check with my team to make sure that would be fine to add into Ubuntu.

Yes, if this is indeed considered a plus. All file access restrictions that swtpm once had due to the profile will be gone...

... though we may not have another choice. Next time root could mount to /mymount and we'd have the same problem again. :-(

@lvoytek
Copy link
Contributor

lvoytek commented Nov 6, 2024

We would also be able to clean up the apparmor profile a lot by removing all the lines tagged with owner, and it would likely prevent a lot of future unnecessary apparmor denials. I'll check with my team to make sure that would be fine to add into Ubuntu.

Yes, if this is indeed considered a plus. All file access restrictions that swtpm once had due to the profile will be gone...

... though we may not have another choice. Next time root could mount to /mymount and we'd have the same problem again. :-(

Going over it with my team, we ultimately decided that /** with even owner files would still provide too much access, since a change of file/directory uid to swtpm would ultimately remove any intended restrictions. For mount directory use cases we would recommend adding a local override of the folder name if it's known. I still think having all tmp directories available by default would be reasonable, but I know it doesn't solve this specific issue :(

@stefanberger
Copy link
Owner Author

Going over it with my team, we ultimately decided that /** with even owner files would still provide too much access, since a change of file/directory uid to swtpm would ultimately remove any intended restrictions. For mount directory use cases we would recommend adding a local override of the folder name if it's known. I still think having all tmp directories available by default would be reasonable, but I know it doesn't solve this specific issue :(

I am going to try to adjust the test case and send a patch to the QEMU mailing list. The path in the test case is not ideal. However, we will still need this patch here, since it will be using /var/tmp then.

@stefanberger
Copy link
Owner Author

Proposed QEMU patch: https://lore.kernel.org/qemu-devel/20241106180751.6859-1-stefanb@linux.vnet.ibm.com/T/#u

Still need this one here.

QEMU's functional tests need access to /var/tmp/**. To avoid the following
type of AppArmor permission failures add a rule that allows access to
/var/tmp/**.

 type=AVC msg=audit(1730829888.863:260): apparmor="DENIED" \
   operation="mknod" class="file" profile="swtpm" \
   name="/var/tmp/qemu_3r9txw7z/swtpm-socket" pid=3925 comm="swtpm" \
   requested_mask="c" denied_mask="c" fsuid=1000 ouid=1000FSUID="stefanb" \
   OUID="stefanb"

[ To run the QEMU's functional tests use the following command:
    make check-functional ]

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
pbo-linaro pushed a commit to pbo-linaro/qemu-ci that referenced this pull request Nov 6, 2024
To avoid AppArmor-related test failures when functional test are run from
somewhere under /mnt, adjust the path to swtpm's state to use an AppArmor-
supported path, such as /var/tmp, which is provided by the python function
tempfile.TemporaryDirectory().

An update to swtpm's AppArmor profile is also being done to support /var/tmp.

Link: https://lore.kernel.org/qemu-devel/CAFEAcA8A=kWLtTZ+nua-MpzqkaEjW5srOYZruZnE2tB6vmoMig@mail.gmail.com/
Link: stefanberger/swtpm#944
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
pbo-linaro pushed a commit to pbo-linaro/qemu-ci that referenced this pull request Nov 6, 2024
https://lore.kernel.org/qemu-devel/20241106180751.6859-1-stefanb@linux.vnet.ibm.com

---

From: Stefan Berger <stefanb@linux.vnet.ibm.com>
To: peter.maydell@linaro.org, qemu-devel@nongnu.org
Cc: marcandre.lureau@gmail.com, clg@redhat.com, lena.voytek@canonical.com,
 Stefan Berger <stefanb@linux.ibm.com>
Subject: [PATCH] tests: Adjust path for swtpm state to use path under /var/tmp/
Date: Wed,  6 Nov 2024 13:07:51 -0500
Message-ID: <20241106180751.6859-1-stefanb@linux.vnet.ibm.com>
X-Mailer: git-send-email 2.47.0
X-TM-AS-GCONF: 00
X-Proofpoint-GUID: jQY8kr9-L3a--Ft4HpIau6cZby3TRSjl
X-Proofpoint-ORIG-GUID: mijDPM4hfiXsLZy04kVjHHhv1za7mCLJ
Content-Transfer-Encoding: 8bit
X-Proofpoint-UnRewURL: 0 URL was un-rewritten
MIME-Version: 1.0
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1051,Hydra:6.0.680,FMLib:17.12.62.30
 definitions=2024-10-15_01,2024-10-11_01,2024-09-30_01
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0
 mlxscore=0 clxscore=1011
 adultscore=0 lowpriorityscore=0 impostorscore=0 malwarescore=0
 mlxlogscore=839 spamscore=0 bulkscore=0 priorityscore=1501 phishscore=0
 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.19.0-2409260000 definitions=main-2411060141
Received-SPF: none client-ip=148.163.156.1;
 envelope-from=stefanb@linux.vnet.ibm.com; helo=mx0a-001b2d01.pphosted.com
X-Spam_score_int: -26
X-Spam_score: -2.7
X-Spam_bar: --
X-Spam_report: (-2.7 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1,
 DKIM_VALID=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7,
 RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001,
 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001,
 SPF_HELO_NONE=0.001, SPF_NONE=0.001 autolearn=ham autolearn_force=no
X-Spam_action: no action
X-BeenThere: qemu-devel@nongnu.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
 <mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <https://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
 <mailto:qemu-devel-request@nongnu.org?subject=subscribe>
Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org
Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org

From: Stefan Berger <stefanb@linux.ibm.com>

To avoid AppArmor-related test failures when functional test are run from
somewhere under /mnt, adjust the path to swtpm's state to use an AppArmor-
supported path, such as /var/tmp, which is provided by the python function
tempfile.TemporaryDirectory().

An update to swtpm's AppArmor profile is also being done to support /var/tmp.

Link: https://lore.kernel.org/qemu-devel/CAFEAcA8A=kWLtTZ+nua-MpzqkaEjW5srOYZruZnE2tB6vmoMig@mail.gmail.com/
Link: stefanberger/swtpm#944
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
 tests/functional/test_arm_aspeed.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/functional/test_arm_aspeed.py b/tests/functional/test_arm_aspeed.py
index 9761fc06a4..a574b1e521 100644
--- a/tests/functional/test_arm_aspeed.py
+++ b/tests/functional/test_arm_aspeed.py
@@ -227,11 +227,11 @@ def test_arm_ast2600_evb_buildroot_tpm(self):

         image_path = self.ASSET_BR2_202302_AST2600_TPM_FLASH.fetch()

-        socket_dir = tempfile.TemporaryDirectory(prefix="qemu_")
-        socket = os.path.join(socket_dir.name, 'swtpm-socket')
+        tpmstate_dir = tempfile.TemporaryDirectory(prefix="qemu_")
+        socket = os.path.join(tpmstate_dir.name, 'swtpm-socket')

         subprocess.run(['swtpm', 'socket', '-d', '--tpm2',
-                        '--tpmstate', f'dir={self.vm.temp_dir}',
+                        '--tpmstate', f'dir={tpmstate_dir.name}',
                         '--ctrl', f'type=unixio,path={socket}'])

         self.vm.add_args('-chardev', f'socket,id=chrtpm,path={socket}')
--
2.34.1

Signed-off-by: GitHub Actions Bot <bot@github.com>
stefanberger added a commit to stefanberger/qemu-tpm that referenced this pull request Nov 6, 2024
To avoid AppArmor-related test failures when functional test are run from
somewhere under /mnt, adjust the path to swtpm's state to use an AppArmor-
supported path, such as /var/tmp, which is provided by the python function
tempfile.TemporaryDirectory().

An update to swtpm's AppArmor profile is also being done to support /var/tmp.

Link: https://lore.kernel.org/qemu-devel/CAFEAcA8A=kWLtTZ+nua-MpzqkaEjW5srOYZruZnE2tB6vmoMig@mail.gmail.com/
Link: stefanberger/swtpm#944
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
@stefanberger stefanberger marked this pull request as ready for review November 6, 2024 18:35
@stefanberger stefanberger merged commit cc52b20 into master Nov 6, 2024
4 of 5 checks passed
legoater pushed a commit to legoater/qemu that referenced this pull request Nov 7, 2024
To avoid AppArmor-related test failures when functional test are run from
somewhere under /mnt, adjust the path to swtpm's state to use an AppArmor-
supported path, such as /var/tmp, which is provided by the python function
tempfile.TemporaryDirectory().

An update to swtpm's AppArmor profile is also being done to support /var/tmp.

Link: https://lore.kernel.org/qemu-devel/CAFEAcA8A=kWLtTZ+nua-MpzqkaEjW5srOYZruZnE2tB6vmoMig@mail.gmail.com/
Link: stefanberger/swtpm#944
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Fixes: f04cb2d ("tests/functional: Convert most Aspeed machine tests")
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Peter Maydell <peter.maydell@linaro.org>
stefanberger added a commit to stefanberger/qemu-tpm that referenced this pull request Nov 7, 2024
To avoid AppArmor-related test failures when functional test are run from
somewhere under /mnt, adjust the path to swtpm's state to use an AppArmor-
supported path, such as /var/tmp, which is provided by the python function
tempfile.TemporaryDirectory().

An update to swtpm's AppArmor profile is also being done to support /var/tmp.

Link: https://lore.kernel.org/qemu-devel/CAFEAcA8A=kWLtTZ+nua-MpzqkaEjW5srOYZruZnE2tB6vmoMig@mail.gmail.com/
Link: stefanberger/swtpm#944
Tested-by: Peter Maydell <peter.maydell@linaro.org>
Fixes: f04cb2d ("tests/functional: Convert most Aspeed machine tests")
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
pbo-linaro pushed a commit to pbo-linaro/qemu-ci that referenced this pull request Nov 7, 2024
To avoid AppArmor-related test failures when functional test are run from
somewhere under /mnt, adjust the path to swtpm's state to use an AppArmor-
supported path, such as /var/tmp, which is provided by the python function
tempfile.TemporaryDirectory().

An update to swtpm's AppArmor profile is also being done to support /var/tmp.

Link: https://lore.kernel.org/qemu-devel/CAFEAcA8A=kWLtTZ+nua-MpzqkaEjW5srOYZruZnE2tB6vmoMig@mail.gmail.com/
Link: stefanberger/swtpm#944
Tested-by: Peter Maydell <peter.maydell@linaro.org>
Fixes: f04cb2d ("tests/functional: Convert most Aspeed machine tests")
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
stefanberger added a commit to stefanberger/qemu-tpm that referenced this pull request Nov 7, 2024
To avoid AppArmor-related test failures when functional test are run from
somewhere under /mnt, adjust the path to swtpm's state to use an AppArmor-
supported path, such as /var/tmp, which is provided by the python function
tempfile.TemporaryDirectory().

An update to swtpm's AppArmor profile is also being done to support /var/tmp.

Link: https://lore.kernel.org/qemu-devel/CAFEAcA8A=kWLtTZ+nua-MpzqkaEjW5srOYZruZnE2tB6vmoMig@mail.gmail.com/
Link: stefanberger/swtpm#944
Tested-by: Peter Maydell <peter.maydell@linaro.org>
Fixes: f04cb2d ("tests/functional: Convert most Aspeed machine tests")
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
pbo-linaro pushed a commit to pbo-linaro/qemu-ci that referenced this pull request Nov 7, 2024
To avoid AppArmor-related test failures when functional test are run from
somewhere under /mnt, adjust the path to swtpm's state to use an AppArmor-
supported path, such as /var/tmp, which is provided by the python function
tempfile.TemporaryDirectory().

An update to swtpm's AppArmor profile is also being done to support /var/tmp.

Link: https://lore.kernel.org/qemu-devel/CAFEAcA8A=kWLtTZ+nua-MpzqkaEjW5srOYZruZnE2tB6vmoMig@mail.gmail.com/
Link: stefanberger/swtpm#944
Tested-by: Peter Maydell <peter.maydell@linaro.org>
Fixes: f04cb2d ("tests/functional: Convert most Aspeed machine tests")
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
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

Successfully merging this pull request may close these issues.

2 participants