Re: [libvirt] [PATCH v3 12/14] security: Label the external swtpm with SELinux labels

2018-05-09 Thread Stefan Berger

On 05/08/2018 05:28 PM, John Ferlan wrote:


On 05/04/2018 04:21 PM, Stefan Berger wrote:

In this patch we label the swtpm process with SELinux labels. We give it the
same label as the QEMU process has. We label its state directory and files
as well.

The file and process labels now look as follows:

Directory: /var/lib/libvirt/swtpm

[root@localhost swtpm]# ls -lZ
total 4
rwx--. 2 tss  tss  system_u:object_r:svirt_image_t:s0:c254,c932 4096 Apr  5 
16:46 testvm

[root@localhost testvm]# ls -lZ
total 8
-rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 3648 Apr  5 
16:46 tpm-00.permall

The log in /var/log/swtpm/libvirt/qemu is labeled as follows:

-rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 2237 Apr  5 
16:46 vtpm.log

[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | 
grep ctrl | grep -v grep
system_u:system_r:svirt_t:s0:c254,c932 tss 25664 0.0  0.0 28172  3892 ?
Ss   16:57   0:00 /usr/bin/swtpm socket --daemon --ctrl 
type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 
--tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm1.2 --log 
file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log

[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | 
grep tpm | grep -v grep
system_u:system_r:svirt_t:s0:c254,c932 qemu 25669 99.0  0.0 3096704 48500 ?
Sl   16:57   3:28 /bin/qemu-system-x86_64 [..]

Signed-off-by: Stefan Berger 
---
  src/libvirt_private.syms|  1 +
  src/qemu/qemu_extdevice.c   | 22 ++-
  src/security/security_driver.h  |  4 ++
  src/security/security_manager.c | 17 +
  src/security/security_manager.h |  3 ++
  src/security/security_selinux.c | 82 +
  src/security/security_stack.c   | 19 ++
  7 files changed, 147 insertions(+), 1 deletion(-)


I think this looks OK - not my specialty 0-) though.  I see
security_manager, selinux, etc. and my eyes start glazing over!

Anyway, I assume the reason there's no Restore processing is because
everything is deleted at shutdown, right?


No, the restore functions were missing. Added them now.

  Stefan

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH v3 12/14] security: Label the external swtpm with SELinux labels

2018-05-08 Thread John Ferlan


On 05/04/2018 04:21 PM, Stefan Berger wrote:
> In this patch we label the swtpm process with SELinux labels. We give it the
> same label as the QEMU process has. We label its state directory and files
> as well.
> 
> The file and process labels now look as follows:
> 
> Directory: /var/lib/libvirt/swtpm
> 
> [root@localhost swtpm]# ls -lZ
> total 4
> rwx--. 2 tss  tss  system_u:object_r:svirt_image_t:s0:c254,c932 4096 Apr  
> 5 16:46 testvm
> 
> [root@localhost testvm]# ls -lZ
> total 8
> -rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 3648 Apr  
> 5 16:46 tpm-00.permall
> 
> The log in /var/log/swtpm/libvirt/qemu is labeled as follows:
> 
> -rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 2237 Apr  
> 5 16:46 vtpm.log
> 
> [root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | 
> grep ctrl | grep -v grep
> system_u:system_r:svirt_t:s0:c254,c932 tss 25664 0.0  0.0 28172  3892 ?   
>  Ss   16:57   0:00 /usr/bin/swtpm socket --daemon --ctrl 
> type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 
> --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm1.2 --log 
> file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log
> 
> [root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | 
> grep tpm | grep -v grep
> system_u:system_r:svirt_t:s0:c254,c932 qemu 25669 99.0  0.0 3096704 48500 ?   
>  Sl   16:57   3:28 /bin/qemu-system-x86_64 [..]
> 
> Signed-off-by: Stefan Berger 
> ---
>  src/libvirt_private.syms|  1 +
>  src/qemu/qemu_extdevice.c   | 22 ++-
>  src/security/security_driver.h  |  4 ++
>  src/security/security_manager.c | 17 +
>  src/security/security_manager.h |  3 ++
>  src/security/security_selinux.c | 82 
> +
>  src/security/security_stack.c   | 19 ++
>  7 files changed, 147 insertions(+), 1 deletion(-)
> 

I think this looks OK - not my specialty 0-) though.  I see
security_manager, selinux, etc. and my eyes start glazing over!

Anyway, I assume the reason there's no Restore processing is because
everything is deleted at shutdown, right?

> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index e533b95..79b8afa 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1334,6 +1334,7 @@ virSecurityManagerSetProcessLabel;
>  virSecurityManagerSetSavedStateLabel;
>  virSecurityManagerSetSocketLabel;
>  virSecurityManagerSetTapFDLabel;
> +virSecurityManagerSetTPMLabels;
>  virSecurityManagerStackAddNested;
>  virSecurityManagerTransactionAbort;
>  virSecurityManagerTransactionCommit;
> diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
> index f3f337d..eb7220d 100644
> --- a/src/qemu/qemu_extdevice.c
> +++ b/src/qemu/qemu_extdevice.c
> @@ -166,12 +166,32 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
>  
>  virCommandSetErrorBuffer(cmd, );
>  
> -if (virCommandRun(cmd, ) < 0 || exitstatus != 0) {
> +if (virSecurityManagerSetTPMLabels(driver->securityManager,
> +   def) < 0)
> +goto error;
> +
> +if (virSecurityManagerSetChildProcessLabel(driver->securityManager,
> +   def, cmd) < 0)
> +goto error;
> +
> +if (virSecurityManagerPreFork(driver->securityManager) < 0)
> +goto error;
> +
> +/* make sure we run this with the appropriate user */
> +virCommandSetUID(cmd, cfg->swtpm_user);
> +virCommandSetGID(cmd, cfg->swtpm_group);
> +
> +ret = virCommandRun(cmd, );
> +
> +virSecurityManagerPostFork(driver->securityManager);
> +
> +if (ret < 0 || exitstatus != 0) {
>  VIR_ERROR("Could not start 'swtpm'. exitstatus: %d\n"
>"stderr: %s\n", exitstatus, errbuf);
>  virReportError(VIR_ERR_INTERNAL_ERROR,
> _("Could not start 'swtpm'. exitstatus: %d, "
> "error: %s"), exitstatus, errbuf);
> +ret = -1;
>  goto error;
>  }
>  
> diff --git a/src/security/security_driver.h b/src/security/security_driver.h
> index 95e7c4d..4aa415f 100644
> --- a/src/security/security_driver.h
> +++ b/src/security/security_driver.h
> @@ -149,6 +149,8 @@ typedef int (*virSecurityDomainRestoreChardevLabel) 
> (virSecurityManagerPtr mgr,
>   virDomainDefPtr def,
>   
> virDomainChrSourceDefPtr dev_source,
>   bool chardevStdioLogd);
> +typedef int (*virSecurityDomainSetTPMLabels) (virSecurityManagerPtr mgr,
> +  virDomainDefPtr def);
>  
>  
>  struct _virSecurityDriver {
> @@ -213,6 +215,8 @@ struct _virSecurityDriver {
>  
>  virSecurityDomainSetChardevLabel domainSetSecurityChardevLabel;
>  virSecurityDomainRestoreChardevLabel 

[libvirt] [PATCH v3 12/14] security: Label the external swtpm with SELinux labels

2018-05-04 Thread Stefan Berger
In this patch we label the swtpm process with SELinux labels. We give it the
same label as the QEMU process has. We label its state directory and files
as well.

The file and process labels now look as follows:

Directory: /var/lib/libvirt/swtpm

[root@localhost swtpm]# ls -lZ
total 4
rwx--. 2 tss  tss  system_u:object_r:svirt_image_t:s0:c254,c932 4096 Apr  5 
16:46 testvm

[root@localhost testvm]# ls -lZ
total 8
-rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 3648 Apr  5 
16:46 tpm-00.permall

The log in /var/log/swtpm/libvirt/qemu is labeled as follows:

-rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 2237 Apr  5 
16:46 vtpm.log

[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | 
grep ctrl | grep -v grep
system_u:system_r:svirt_t:s0:c254,c932 tss 25664 0.0  0.0 28172  3892 ?
Ss   16:57   0:00 /usr/bin/swtpm socket --daemon --ctrl 
type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 
--tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm1.2 --log 
file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log

[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | 
grep tpm | grep -v grep
system_u:system_r:svirt_t:s0:c254,c932 qemu 25669 99.0  0.0 3096704 48500 ?
Sl   16:57   3:28 /bin/qemu-system-x86_64 [..]

Signed-off-by: Stefan Berger 
---
 src/libvirt_private.syms|  1 +
 src/qemu/qemu_extdevice.c   | 22 ++-
 src/security/security_driver.h  |  4 ++
 src/security/security_manager.c | 17 +
 src/security/security_manager.h |  3 ++
 src/security/security_selinux.c | 82 +
 src/security/security_stack.c   | 19 ++
 7 files changed, 147 insertions(+), 1 deletion(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e533b95..79b8afa 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1334,6 +1334,7 @@ virSecurityManagerSetProcessLabel;
 virSecurityManagerSetSavedStateLabel;
 virSecurityManagerSetSocketLabel;
 virSecurityManagerSetTapFDLabel;
+virSecurityManagerSetTPMLabels;
 virSecurityManagerStackAddNested;
 virSecurityManagerTransactionAbort;
 virSecurityManagerTransactionCommit;
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index f3f337d..eb7220d 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -166,12 +166,32 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
 
 virCommandSetErrorBuffer(cmd, );
 
-if (virCommandRun(cmd, ) < 0 || exitstatus != 0) {
+if (virSecurityManagerSetTPMLabels(driver->securityManager,
+   def) < 0)
+goto error;
+
+if (virSecurityManagerSetChildProcessLabel(driver->securityManager,
+   def, cmd) < 0)
+goto error;
+
+if (virSecurityManagerPreFork(driver->securityManager) < 0)
+goto error;
+
+/* make sure we run this with the appropriate user */
+virCommandSetUID(cmd, cfg->swtpm_user);
+virCommandSetGID(cmd, cfg->swtpm_group);
+
+ret = virCommandRun(cmd, );
+
+virSecurityManagerPostFork(driver->securityManager);
+
+if (ret < 0 || exitstatus != 0) {
 VIR_ERROR("Could not start 'swtpm'. exitstatus: %d\n"
   "stderr: %s\n", exitstatus, errbuf);
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Could not start 'swtpm'. exitstatus: %d, "
"error: %s"), exitstatus, errbuf);
+ret = -1;
 goto error;
 }
 
diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index 95e7c4d..4aa415f 100644
--- a/src/security/security_driver.h
+++ b/src/security/security_driver.h
@@ -149,6 +149,8 @@ typedef int (*virSecurityDomainRestoreChardevLabel) 
(virSecurityManagerPtr mgr,
  virDomainDefPtr def,
  virDomainChrSourceDefPtr 
dev_source,
  bool chardevStdioLogd);
+typedef int (*virSecurityDomainSetTPMLabels) (virSecurityManagerPtr mgr,
+  virDomainDefPtr def);
 
 
 struct _virSecurityDriver {
@@ -213,6 +215,8 @@ struct _virSecurityDriver {
 
 virSecurityDomainSetChardevLabel domainSetSecurityChardevLabel;
 virSecurityDomainRestoreChardevLabel domainRestoreSecurityChardevLabel;
+
+virSecurityDomainSetTPMLabels domainSetSecurityTPMLabels;
 };
 
 virSecurityDriverPtr virSecurityDriverLookup(const char *name,
diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index 71f7f59..48777bb 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -1204,3 +1204,20 @@ 
virSecurityManagerRestoreChardevLabel(virSecurityManagerPtr mgr,
 virReportUnsupportedError();
 return -1;
 }
+
+
+int