Re: [libvirt] [PATCH v3 07/11] Add SELinux and DAC labeling support for TPM passthrough

2013-04-02 Thread Stefan Berger 

On 04/01/2013 05:06 PM, Corey Bryant wrote:



On 03/21/2013 11:42 AM, Stefan Berger wrote:

Signed-off-by: Stefan Bergerstef...@linux.vnet.ibm.com

---
  src/security/security_dac.c |   53 ++
  src/security/security_selinux.c |   96 


  2 files changed, 149 insertions(+)

Index: libvirt/src/security/security_selinux.c
===
--- libvirt.orig/src/security/security_selinux.c
+++ libvirt/src/security/security_selinux.c
@@ -45,6 +45,7 @@
  #include virrandom.h
  #include virutil.h
  #include virconf.h
+#include virtpm.h

  #define VIR_FROM_THIS VIR_FROM_SECURITY

@@ -76,6 +77,12 @@ struct _virSecuritySELinuxCallbackData {
  #define SECURITY_SELINUX_VOID_DOI   0
  #define SECURITY_SELINUX_NAME selinux

+static int
+virSecuritySELinuxRestoreSecurityTPMFileLabelInt(virSecurityManagerPtr 
mgr,

+ virDomainDefPtr def,
+ virDomainTPMDefPtr tpm);
+
+
  /*
   * Returns 0 on success, 1 if already reserved, or -1 on fatal error
   */
@@ -1062,6 +1069,84 @@ err:
  return rc;
  }

+
+static int
+virSecuritySELinuxSetSecurityTPMFileLabel(virSecurityManagerPtr mgr,
+  virDomainDefPtr def,
+  virDomainTPMDefPtr tpm)
+{
+int rc;
+virSecurityLabelDefPtr seclabel;
+char *cancel_path;
+
+seclabel = virDomainDefGetSecurityLabelDef(def, 
SECURITY_SELINUX_NAME);

+if (seclabel == NULL)
+return -1;
+
+switch (tpm-type) {
+case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+rc = virSecuritySELinuxSetFilecon(
+ tpm-data.passthrough.source.data.file.path,
+   seclabel-imagelabel);
+if (rc  0)
+return -1;
+
+if ((cancel_path = virTPMFindCancelPath()) != NULL) {
+rc = virSecuritySELinuxSetFilecon(cancel_path,
+ seclabel-imagelabel);
+VIR_FREE(cancel_path);
+if (rc  0) {
+ virSecuritySELinuxRestoreSecurityTPMFileLabelInt(mgr, def,
+ tpm);
+return -1;
+}
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Cannot determine TPM command cancel 
path));

+return -1;


This makes me wonder if cancel-path should be specifiable at the 
libvirt level rather than just using the default sysfs entry.  If I've 
read the code correctly I don't think it can currently be specified.  
However QEMU is capable of taking a cancel-path string in case it is 
different from the default sysfs path.





I am not sure whether to allow users to specify the path and 
misconfigure it and to have QEMU write a letter into the wrong file. I 
wonder whether we could have libvirt determine the path and display it 
in the XML as read-only, though.


   Stefan

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

Re: [libvirt] [PATCH v3 07/11] Add SELinux and DAC labeling support for TPM passthrough

2013-04-02 Thread Corey Bryant 



On 04/02/2013 07:15 AM, Stefan Berger wrote:

On 04/01/2013 05:06 PM, Corey Bryant wrote:



On 03/21/2013 11:42 AM, Stefan Berger wrote:

Signed-off-by: Stefan Bergerstef...@linux.vnet.ibm.com

---
  src/security/security_dac.c |   53 ++
  src/security/security_selinux.c |   96

  2 files changed, 149 insertions(+)

Index: libvirt/src/security/security_selinux.c
===
--- libvirt.orig/src/security/security_selinux.c
+++ libvirt/src/security/security_selinux.c
@@ -45,6 +45,7 @@
  #include virrandom.h
  #include virutil.h
  #include virconf.h
+#include virtpm.h

  #define VIR_FROM_THIS VIR_FROM_SECURITY

@@ -76,6 +77,12 @@ struct _virSecuritySELinuxCallbackData {
  #define SECURITY_SELINUX_VOID_DOI   0
  #define SECURITY_SELINUX_NAME selinux

+static int
+virSecuritySELinuxRestoreSecurityTPMFileLabelInt(virSecurityManagerPtr
mgr,
+ virDomainDefPtr def,
+ virDomainTPMDefPtr tpm);
+
+
  /*
   * Returns 0 on success, 1 if already reserved, or -1 on fatal error
   */
@@ -1062,6 +1069,84 @@ err:
  return rc;
  }

+
+static int
+virSecuritySELinuxSetSecurityTPMFileLabel(virSecurityManagerPtr mgr,
+  virDomainDefPtr def,
+  virDomainTPMDefPtr tpm)
+{
+int rc;
+virSecurityLabelDefPtr seclabel;
+char *cancel_path;
+
+seclabel = virDomainDefGetSecurityLabelDef(def,
SECURITY_SELINUX_NAME);
+if (seclabel == NULL)
+return -1;
+
+switch (tpm-type) {
+case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+rc = virSecuritySELinuxSetFilecon(
+ tpm-data.passthrough.source.data.file.path,
+   seclabel-imagelabel);
+if (rc  0)
+return -1;
+
+if ((cancel_path = virTPMFindCancelPath()) != NULL) {
+rc = virSecuritySELinuxSetFilecon(cancel_path,
+ seclabel-imagelabel);
+VIR_FREE(cancel_path);
+if (rc  0) {
+ virSecuritySELinuxRestoreSecurityTPMFileLabelInt(mgr, def,
+ tpm);
+return -1;
+}
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Cannot determine TPM command cancel
path));
+return -1;


This makes me wonder if cancel-path should be specifiable at the
libvirt level rather than just using the default sysfs entry.  If I've
read the code correctly I don't think it can currently be specified.
However QEMU is capable of taking a cancel-path string in case it is
different from the default sysfs path.




I am not sure whether to allow users to specify the path and
misconfigure it and to have QEMU write a letter into the wrong file. I
wonder whether we could have libvirt determine the path and display it
in the XML as read-only, though.

Stefan



After discussing with Stefan some more, I think just using the default 
path is enough.  I don't know why the sysfs path would not be the 
default anyway.  And as far as I know we've decided not to support fd 
passing for vTPM, at least at this point, so that is not a concern.


--
Regards,
Corey Bryant

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

Re: [libvirt] [PATCH v3 07/11] Add SELinux and DAC labeling support for TPM passthrough

2013-04-01 Thread Corey Bryant 



On 03/21/2013 11:42 AM, Stefan Berger wrote:

Signed-off-by: Stefan Bergerstef...@linux.vnet.ibm.com

---
  src/security/security_dac.c |   53 ++
  src/security/security_selinux.c |   96 

  2 files changed, 149 insertions(+)

Index: libvirt/src/security/security_selinux.c
===
--- libvirt.orig/src/security/security_selinux.c
+++ libvirt/src/security/security_selinux.c
@@ -45,6 +45,7 @@
  #include virrandom.h
  #include virutil.h
  #include virconf.h
+#include virtpm.h

  #define VIR_FROM_THIS VIR_FROM_SECURITY

@@ -76,6 +77,12 @@ struct _virSecuritySELinuxCallbackData {
  #define SECURITY_SELINUX_VOID_DOI   0
  #define SECURITY_SELINUX_NAME selinux

+static int
+virSecuritySELinuxRestoreSecurityTPMFileLabelInt(virSecurityManagerPtr mgr,
+ virDomainDefPtr def,
+ virDomainTPMDefPtr tpm);
+
+
  /*
   * Returns 0 on success, 1 if already reserved, or -1 on fatal error
   */
@@ -1062,6 +1069,84 @@ err:
  return rc;
  }

+
+static int
+virSecuritySELinuxSetSecurityTPMFileLabel(virSecurityManagerPtr mgr,
+  virDomainDefPtr def,
+  virDomainTPMDefPtr tpm)
+{
+int rc;
+virSecurityLabelDefPtr seclabel;
+char *cancel_path;
+
+seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME);
+if (seclabel == NULL)
+return -1;
+
+switch (tpm-type) {
+case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
+rc = virSecuritySELinuxSetFilecon(
+   tpm-data.passthrough.source.data.file.path,
+   seclabel-imagelabel);
+if (rc  0)
+return -1;
+
+if ((cancel_path = virTPMFindCancelPath()) != NULL) {
+rc = virSecuritySELinuxSetFilecon(cancel_path,
+  seclabel-imagelabel);
+VIR_FREE(cancel_path);
+if (rc  0) {
+virSecuritySELinuxRestoreSecurityTPMFileLabelInt(mgr, def,
+ tpm);
+return -1;
+}
+} else {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+   _(Cannot determine TPM command cancel path));
+return -1;


This makes me wonder if cancel-path should be specifiable at the libvirt 
level rather than just using the default sysfs entry.  If I've read the 
code correctly I don't think it can currently be specified.  However 
QEMU is capable of taking a cancel-path string in case it is different 
from the default sysfs path.



--
Regards,
Corey Bryant

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