[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v5 1/3] crypto: add reload for QCryptoTLSCredsClass
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v5 1/3] crypto: add reload for QCryptoTLSCredsClass |
Date: |
Thu, 11 Mar 2021 11:12:36 +0000 |
User-agent: |
Mutt/2.0.5 (2021-01-21) |
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
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|