diff mbox series

[v5,1/3] crypto: add reload for QCryptoTLSCredsClass

Message ID 20210311105424.1370-2-changzihao1@huawei.com (mailing list archive)
State New, archived
Headers show
Series vnc: support reload x509 certificates | expand

Commit Message

Zihao Chang March 11, 2021, 10:54 a.m. UTC 
This patch adds reload interface for QCryptoTLSCredsClass and implements
the interface for QCryptoTLSCredsX509.

Signed-off-by: Zihao Chang <changzihao1@huawei.com>
---
 crypto/tlscredsx509.c     | 117 ++++++++++++++++++++++++++++++++++++++
 include/crypto/tlscreds.h |   8 ++-
 2 files changed, 122 insertions(+), 3 deletions(-)

Comments

Daniel P. Berrangé March 11, 2021, 11:12 a.m. UTC | #1 
On Thu, Mar 11, 2021 at 06:54:22PM +0800, Zihao Chang wrote:
> This patch adds reload interface for QCryptoTLSCredsClass and implements
> the interface for QCryptoTLSCredsX509.
> 
> Signed-off-by: Zihao Chang <changzihao1@huawei.com>
> ---
>  crypto/tlscredsx509.c     | 117 ++++++++++++++++++++++++++++++++++++++
>  include/crypto/tlscreds.h |   8 ++-
>  2 files changed, 122 insertions(+), 3 deletions(-)
> 
> diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
> index dbadad4df28e..6b57f14f2df1 100644
> --- a/crypto/tlscredsx509.c
> +++ b/crypto/tlscredsx509.c
> @@ -23,8 +23,10 @@
>  #include "tlscredspriv.h"
>  #include "crypto/secret.h"
>  #include "qapi/error.h"
> +#include "qapi/qmp/qstring.h"
>  #include "qemu/module.h"
>  #include "qom/object_interfaces.h"
> +#include "qom/qom-qobject.h"
>  #include "trace.h"
>  
>  
> @@ -770,6 +772,118 @@ qcrypto_tls_creds_x509_prop_get_sanity(Object *obj,
>  }
>  
>  
> +#ifdef CONFIG_GNUTLS
> +
> +
> +/*
> + * object_property_get_qobject() return "" for NULL QString,
> + * set NULL QString prop as "" is unsafe.
> + */
> +static bool
> +qcrypto_tls_creds_x509_need_set_prop(QObject *qobj)
> +{
> +    QString *qstring = qobject_to(QString, qobj);
> +    /* prop type is not QString. */
> +    if (!qstring) {
> +        return true;
> +    }
> +
> +    return strlen(qstring_get_str(qstring)) > 0;
> +}
> +
> +
> +static bool
> +qcrypto_tls_creds_x509_copy_propertites(Object *new,
> +                                        Object *old,
> +                                        Error **errp)
> +{
> +    ObjectProperty *prop = NULL;
> +    ObjectPropertyIterator iter;
> +    Error *local_err = NULL;
> +
> +    object_property_iter_init(&iter, old);
> +    while ((prop = object_property_iter_next(&iter))) {
> +        QObject *value = NULL;
> +        /* "loaded" depends on other props, copy it finally. */
> +        if (g_strcmp0(prop->name, "loaded") == 0) {
> +            continue;
> +        }
> +
> +        /* prop do not support set. */
> +        if (!prop->set) {
> +            continue;
> +        }
> +
> +        value = object_property_get_qobject(old, prop->name, &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err);
> +            return false;
> +        }
> +
> +        if (qcrypto_tls_creds_x509_need_set_prop(value)) {
> +            object_property_set_qobject(new, prop->name, value, &local_err);
> +            if (local_err) {
> +                error_propagate(errp, local_err);
> +                qobject_unref(value);
> +                return false;
> +            }
> +        }
> +        qobject_unref(value);
> +    }
> +
> +    return true;
> +}
> +
> +
> +static bool
> +qcrypto_tls_creds_x509_reload(QCryptoTLSCreds *creds, Error **errp)
> +{
> +    Object *old = OBJECT(creds);
> +    QCryptoTLSCredsX509 *old_x509 = QCRYPTO_TLS_CREDS_X509(creds);
> +    Object *new = object_new(TYPE_QCRYPTO_TLS_CREDS_X509);
> +    QCryptoTLSCredsX509 *new_x509 = QCRYPTO_TLS_CREDS_X509(new);
> +    Error *local_err = NULL;
> +    bool ret = false;
> +
> +    if (!qcrypto_tls_creds_x509_copy_propertites(new, old, &local_err)) {
> +        error_propagate(errp, local_err);
> +        goto out;
> +    }

IMHO creating a parallel object with duplicate properties is a rather
too complex way to solve this.

> +    qcrypto_tls_creds_x509_load(new_x509, &local_err);

IMHO you should just be changing this method so that it
doesn't replace the existing 'data' and 'dh_params' fields
until the very end when it knows it was successful.

> +    if (local_err) {
> +        error_propagate(errp, local_err);
> +        goto out;
> +    }
> +
> +    /* load new cert successfully, release old data and update */
> +    qcrypto_tls_creds_x509_unload(old_x509);
> +    old_x509->data = new_x509->data;
> +    old_x509->parent_obj.dh_params = new_x509->parent_obj.dh_params;
> +    new_x509->data = NULL;
> +    new_x509->parent_obj.dh_params = NULL;
> +
> +    ret = true;
> +
> + out:
> +    object_unref(new);
> +    return ret;
> +}
> +
> +
> +#else /* ! CONFIG_GNUTLS */
> +
> +
> +static bool
> +qcrypto_tls_creds_x509_reload(QCryptoTLSCreds *creds, Error **errp)
> +{
> +    return false;
> +}
> +
> +
> +#endif /* ! CONFIG_GNUTLS */
> +
> +
>  static void
>  qcrypto_tls_creds_x509_complete(UserCreatable *uc, Error **errp)
>  {
> @@ -800,6 +914,9 @@ static void
>  qcrypto_tls_creds_x509_class_init(ObjectClass *oc, void *data)
>  {
>      UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
> +    QCryptoTLSCredsClass *ctcc = QCRYPTO_TLS_CREDS_CLASS(oc);
> +
> +    ctcc->reload = qcrypto_tls_creds_x509_reload;
>  
>      ucc->complete = qcrypto_tls_creds_x509_complete;
>  
> diff --git a/include/crypto/tlscreds.h b/include/crypto/tlscreds.h
> index 079e37604784..d0808e391e91 100644
> --- a/include/crypto/tlscreds.h
> +++ b/include/crypto/tlscreds.h
> @@ -30,14 +30,15 @@
>  
>  #define TYPE_QCRYPTO_TLS_CREDS "tls-creds"
>  typedef struct QCryptoTLSCreds QCryptoTLSCreds;
> -DECLARE_INSTANCE_CHECKER(QCryptoTLSCreds, QCRYPTO_TLS_CREDS,
> -                         TYPE_QCRYPTO_TLS_CREDS)
> -
>  typedef struct QCryptoTLSCredsClass QCryptoTLSCredsClass;
> +DECLARE_OBJ_CHECKERS(QCryptoTLSCreds, QCryptoTLSCredsClass, QCRYPTO_TLS_CREDS,
> +                     TYPE_QCRYPTO_TLS_CREDS)
> +
>  
>  #define QCRYPTO_TLS_CREDS_DH_PARAMS "dh-params.pem"
>  
>  
> +typedef bool (*CryptoTLSCredsReload)(QCryptoTLSCreds *, Error **);
>  /**
>   * QCryptoTLSCreds:
>   *
> @@ -61,6 +62,7 @@ struct QCryptoTLSCreds {
>  
>  struct QCryptoTLSCredsClass {
>      ObjectClass parent_class;
> +    CryptoTLSCredsReload reload;
>  };
>  
>  
> -- 
> 2.28.0
> 

Regards,
Daniel
diff mbox series

Patch

diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index dbadad4df28e..6b57f14f2df1 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -23,8 +23,10 @@ 
 #include "tlscredspriv.h"
 #include "crypto/secret.h"
 #include "qapi/error.h"
+#include "qapi/qmp/qstring.h"
 #include "qemu/module.h"
 #include "qom/object_interfaces.h"
+#include "qom/qom-qobject.h"
 #include "trace.h"
 
 
@@ -770,6 +772,118 @@  qcrypto_tls_creds_x509_prop_get_sanity(Object *obj,
 }
 
 
+#ifdef CONFIG_GNUTLS
+
+
+/*
+ * object_property_get_qobject() return "" for NULL QString,
+ * set NULL QString prop as "" is unsafe.
+ */
+static bool
+qcrypto_tls_creds_x509_need_set_prop(QObject *qobj)
+{
+    QString *qstring = qobject_to(QString, qobj);
+    /* prop type is not QString. */
+    if (!qstring) {
+        return true;
+    }
+
+    return strlen(qstring_get_str(qstring)) > 0;
+}
+
+
+static bool
+qcrypto_tls_creds_x509_copy_propertites(Object *new,
+                                        Object *old,
+                                        Error **errp)
+{
+    ObjectProperty *prop = NULL;
+    ObjectPropertyIterator iter;
+    Error *local_err = NULL;
+
+    object_property_iter_init(&iter, old);
+    while ((prop = object_property_iter_next(&iter))) {
+        QObject *value = NULL;
+        /* "loaded" depends on other props, copy it finally. */
+        if (g_strcmp0(prop->name, "loaded") == 0) {
+            continue;
+        }
+
+        /* prop do not support set. */
+        if (!prop->set) {
+            continue;
+        }
+
+        value = object_property_get_qobject(old, prop->name, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return false;
+        }
+
+        if (qcrypto_tls_creds_x509_need_set_prop(value)) {
+            object_property_set_qobject(new, prop->name, value, &local_err);
+            if (local_err) {
+                error_propagate(errp, local_err);
+                qobject_unref(value);
+                return false;
+            }
+        }
+        qobject_unref(value);
+    }
+
+    return true;
+}
+
+
+static bool
+qcrypto_tls_creds_x509_reload(QCryptoTLSCreds *creds, Error **errp)
+{
+    Object *old = OBJECT(creds);
+    QCryptoTLSCredsX509 *old_x509 = QCRYPTO_TLS_CREDS_X509(creds);
+    Object *new = object_new(TYPE_QCRYPTO_TLS_CREDS_X509);
+    QCryptoTLSCredsX509 *new_x509 = QCRYPTO_TLS_CREDS_X509(new);
+    Error *local_err = NULL;
+    bool ret = false;
+
+    if (!qcrypto_tls_creds_x509_copy_propertites(new, old, &local_err)) {
+        error_propagate(errp, local_err);
+        goto out;
+    }
+
+    qcrypto_tls_creds_x509_load(new_x509, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto out;
+    }
+
+    /* load new cert successfully, release old data and update */
+    qcrypto_tls_creds_x509_unload(old_x509);
+    old_x509->data = new_x509->data;
+    old_x509->parent_obj.dh_params = new_x509->parent_obj.dh_params;
+    new_x509->data = NULL;
+    new_x509->parent_obj.dh_params = NULL;
+
+    ret = true;
+
+ out:
+    object_unref(new);
+    return ret;
+}
+
+
+#else /* ! CONFIG_GNUTLS */
+
+
+static bool
+qcrypto_tls_creds_x509_reload(QCryptoTLSCreds *creds, Error **errp)
+{
+    return false;
+}
+
+
+#endif /* ! CONFIG_GNUTLS */
+
+
 static void
 qcrypto_tls_creds_x509_complete(UserCreatable *uc, Error **errp)
 {
@@ -800,6 +914,9 @@  static void
 qcrypto_tls_creds_x509_class_init(ObjectClass *oc, void *data)
 {
     UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+    QCryptoTLSCredsClass *ctcc = QCRYPTO_TLS_CREDS_CLASS(oc);
+
+    ctcc->reload = qcrypto_tls_creds_x509_reload;
 
     ucc->complete = qcrypto_tls_creds_x509_complete;
 
diff --git a/include/crypto/tlscreds.h b/include/crypto/tlscreds.h
index 079e37604784..d0808e391e91 100644
--- a/include/crypto/tlscreds.h
+++ b/include/crypto/tlscreds.h
@@ -30,14 +30,15 @@ 
 
 #define TYPE_QCRYPTO_TLS_CREDS "tls-creds"
 typedef struct QCryptoTLSCreds QCryptoTLSCreds;
-DECLARE_INSTANCE_CHECKER(QCryptoTLSCreds, QCRYPTO_TLS_CREDS,
-                         TYPE_QCRYPTO_TLS_CREDS)
-
 typedef struct QCryptoTLSCredsClass QCryptoTLSCredsClass;
+DECLARE_OBJ_CHECKERS(QCryptoTLSCreds, QCryptoTLSCredsClass, QCRYPTO_TLS_CREDS,
+                     TYPE_QCRYPTO_TLS_CREDS)
+
 
 #define QCRYPTO_TLS_CREDS_DH_PARAMS "dh-params.pem"
 
 
+typedef bool (*CryptoTLSCredsReload)(QCryptoTLSCreds *, Error **);
 /**
  * QCryptoTLSCreds:
  *
@@ -61,6 +62,7 @@  struct QCryptoTLSCreds {
 
 struct QCryptoTLSCredsClass {
     ObjectClass parent_class;
+    CryptoTLSCredsReload reload;
 };