All of lore.kernel.org
 help / color / mirror / Atom feed
* [PULL 0/5] fw_cfg/crypto patches for 5.1 soft freeze
@ 2020-07-04 16:39 Philippe Mathieu-Daudé
  2020-07-04 16:39 ` [PULL 1/5] crypto: Add tls-cipher-suites object Philippe Mathieu-Daudé
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-04 16:39 UTC (permalink / raw)
  To: qemu-devel
  Cc: Philippe Mathieu-Daudé,
	Gerd Hoffmann, Laszlo Ersek, Daniel P. Berrangé,
	Paolo Bonzini

The following changes since commit 4abf70a661a5df3886ac9d7c19c3617fa92b922a:

  Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2020-06-24' =
into staging (2020-07-03 15:34:45 +0100)

are available in the Git repository at:

  https://gitlab.com/philmd/qemu.git tags/fw_cfg-20200704

for you to fetch changes up to 69699f3055a59e24f1153c329ae6eff4b9a343e0:

  crypto/tls-cipher-suites: Produce fw_cfg consumable blob (2020-07-03 18:16:=
01 +0200)

----------------------------------------------------------------
firmware (and crypto) patches

- add the tls-cipher-suites object,
- add the ability to QOM objects to produce data consumable
  by the fw_cfg device,
- let the tls-cipher-suites object implement the
  FW_CFG_DATA_GENERATOR interface.

This is required by EDK2 'HTTPS Boot' feature of OVMF to tell
the guest which TLS ciphers it can use.

CI jobs results:
  https://travis-ci.org/github/philmd/qemu/builds/704724619
  https://gitlab.com/philmd/qemu/-/pipelines/162938106
  https://cirrus-ci.com/build/4682977303068672

----------------------------------------------------------------

Philippe Mathieu-Daud=C3=A9 (5):
  crypto: Add tls-cipher-suites object
  hw/nvram/fw_cfg: Add the FW_CFG_DATA_GENERATOR interface
  softmmu/vl: Let -fw_cfg option take a 'gen_id' argument
  softmmu/vl: Allow -fw_cfg 'gen_id' option to use the 'etc/' namespace
  crypto/tls-cipher-suites: Produce fw_cfg consumable blob

 docs/specs/fw_cfg.txt              |  13 ++-
 include/crypto/tls-cipher-suites.h |  39 +++++++++
 include/hw/nvram/fw_cfg.h          |  43 ++++++++++
 crypto/tls-cipher-suites.c         | 126 +++++++++++++++++++++++++++++
 hw/nvram/fw_cfg.c                  |  35 ++++++++
 softmmu/vl.c                       |  37 ++++++---
 crypto/Makefile.objs               |   1 +
 crypto/trace-events                |   5 ++
 qemu-options.hx                    |  37 +++++++++
 9 files changed, 326 insertions(+), 10 deletions(-)
 create mode 100644 include/crypto/tls-cipher-suites.h
 create mode 100644 crypto/tls-cipher-suites.c

--=20
2.21.3



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PULL 1/5] crypto: Add tls-cipher-suites object
  2020-07-04 16:39 [PULL 0/5] fw_cfg/crypto patches for 5.1 soft freeze Philippe Mathieu-Daudé
@ 2020-07-04 16:39 ` Philippe Mathieu-Daudé
  2020-07-04 16:39 ` [PULL 2/5] hw/nvram/fw_cfg: Add the FW_CFG_DATA_GENERATOR interface Philippe Mathieu-Daudé
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-04 16:39 UTC (permalink / raw)
  To: qemu-devel
  Cc: Philippe Mathieu-Daudé,
	Gerd Hoffmann, Laszlo Ersek, Daniel P. Berrangé,
	Paolo Bonzini

On the host OS, various aspects of TLS operation are configurable.
In particular it is possible for the sysadmin to control the TLS
cipher/protocol algorithms that applications are permitted to use.

* Any given crypto library has a built-in default priority list
  defined by the distro maintainer of the library package (or by
  upstream).

* The "crypto-policies" RPM (or equivalent host OS package)
  provides a config file such as "/etc/crypto-policies/config",
  where the sysadmin can set a high level (library-independent)
  policy.

  The "update-crypto-policies --set" command (or equivalent) is
  used to translate the global policy to individual library
  representations, producing files such as
  "/etc/crypto-policies/back-ends/*.config". The generated files,
  if present, are loaded by the various crypto libraries to
  override their own built-in defaults.

  For example, the GNUTLS library may read
  "/etc/crypto-policies/back-ends/gnutls.config".

* A management application (or the QEMU user) may overide the
  system-wide crypto-policies config via their own config, if
  they need to diverge from the former.

Thus the priority order is "QEMU user config" > "crypto-policies
system config" > "library built-in config".

Introduce the "tls-cipher-suites" object for exposing the ordered
list of permitted TLS cipher suites from the host side to the
guest firmware, via fw_cfg. The list is represented as an array
of bytes.

The priority at which the host-side policy is retrieved is given
by the "priority" property of the new object type. For example,
"priority=@SYSTEM" may be used to refer to
"/etc/crypto-policies/back-ends/gnutls.config" (given that QEMU
uses GNUTLS).

The firmware uses the IANA_TLS_CIPHER array for configuring
guest-side TLS, for example in UEFI HTTPS Boot.

[Description from Daniel P. Berrangé, edited by Laszlo Ersek.]

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200623172726.21040-2-philmd@redhat.com>
---
 include/crypto/tls-cipher-suites.h |  39 ++++++++++
 crypto/tls-cipher-suites.c         | 115 +++++++++++++++++++++++++++++
 crypto/Makefile.objs               |   1 +
 crypto/trace-events                |   5 ++
 qemu-options.hx                    |  19 +++++
 5 files changed, 179 insertions(+)
 create mode 100644 include/crypto/tls-cipher-suites.h
 create mode 100644 crypto/tls-cipher-suites.c

diff --git a/include/crypto/tls-cipher-suites.h b/include/crypto/tls-cipher-suites.h
new file mode 100644
index 0000000000..28b3a73ce1
--- /dev/null
+++ b/include/crypto/tls-cipher-suites.h
@@ -0,0 +1,39 @@
+/*
+ * QEMU TLS Cipher Suites Registry (RFC8447)
+ *
+ * Copyright (c) 2018-2020 Red Hat, Inc.
+ *
+ * Author: Philippe Mathieu-Daudé <philmd@redhat.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef QCRYPTO_TLSCIPHERSUITES_H
+#define QCRYPTO_TLSCIPHERSUITES_H
+
+#include "qom/object.h"
+#include "crypto/tlscreds.h"
+
+#define TYPE_QCRYPTO_TLS_CIPHER_SUITES "tls-cipher-suites"
+#define QCRYPTO_TLS_CIPHER_SUITES(obj) \
+    OBJECT_CHECK(QCryptoTLSCipherSuites, (obj), TYPE_QCRYPTO_TLS_CIPHER_SUITES)
+
+typedef struct QCryptoTLSCipherSuites {
+    /* <private> */
+    QCryptoTLSCreds parent_obj;
+    /* <public> */
+} QCryptoTLSCipherSuites;
+
+/**
+  * qcrypto_tls_cipher_suites_get_data:
+  * @obj: pointer to a TLS cipher suites object
+  * @errp: pointer to a NULL-initialized error object
+  *
+  * Returns: reference to a byte array containing the data.
+  * The caller should release the reference when no longer
+  * required.
+  */
+GByteArray *qcrypto_tls_cipher_suites_get_data(QCryptoTLSCipherSuites *obj,
+                                               Error **errp);
+
+#endif /* QCRYPTO_TLSCIPHERSUITES_H */
diff --git a/crypto/tls-cipher-suites.c b/crypto/tls-cipher-suites.c
new file mode 100644
index 0000000000..a4e0f84307
--- /dev/null
+++ b/crypto/tls-cipher-suites.c
@@ -0,0 +1,115 @@
+/*
+ * QEMU TLS Cipher Suites
+ *
+ * Copyright (c) 2018-2020 Red Hat, Inc.
+ *
+ * Author: Philippe Mathieu-Daudé <philmd@redhat.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qom/object_interfaces.h"
+#include "crypto/tlscreds.h"
+#include "crypto/tls-cipher-suites.h"
+#include "trace.h"
+
+/*
+ * IANA registered TLS ciphers:
+ * https://www.iana.org/assignments/tls-parameters/tls-parameters.xhtml#tls-parameters-4
+ */
+typedef struct {
+    uint8_t data[2];
+} QEMU_PACKED IANA_TLS_CIPHER;
+
+GByteArray *qcrypto_tls_cipher_suites_get_data(QCryptoTLSCipherSuites *obj,
+                                               Error **errp)
+{
+    QCryptoTLSCreds *creds = QCRYPTO_TLS_CREDS(obj);
+    gnutls_priority_t pcache;
+    GByteArray *byte_array;
+    const char *err;
+    size_t i;
+    int ret;
+
+    trace_qcrypto_tls_cipher_suite_priority(creds->priority);
+    ret = gnutls_priority_init(&pcache, creds->priority, &err);
+    if (ret < 0) {
+        error_setg(errp, "Syntax error using priority '%s': %s",
+                   creds->priority, gnutls_strerror(ret));
+        return NULL;
+    }
+
+    byte_array = g_byte_array_new();
+
+    for (i = 0;; i++) {
+        int ret;
+        unsigned idx;
+        const char *name;
+        IANA_TLS_CIPHER cipher;
+        gnutls_protocol_t protocol;
+        const char *version;
+
+        ret = gnutls_priority_get_cipher_suite_index(pcache, i, &idx);
+        if (ret == GNUTLS_E_REQUESTED_DATA_NOT_AVAILABLE) {
+            break;
+        }
+        if (ret == GNUTLS_E_UNKNOWN_CIPHER_SUITE) {
+            continue;
+        }
+
+        name = gnutls_cipher_suite_info(idx, (unsigned char *)&cipher,
+                                        NULL, NULL, NULL, &protocol);
+        if (name == NULL) {
+            continue;
+        }
+
+        version = gnutls_protocol_get_name(protocol);
+        g_byte_array_append(byte_array, cipher.data, 2);
+        trace_qcrypto_tls_cipher_suite_info(cipher.data[0],
+                                            cipher.data[1],
+                                            version, name);
+    }
+    trace_qcrypto_tls_cipher_suite_count(byte_array->len);
+    gnutls_priority_deinit(pcache);
+
+    return byte_array;
+}
+
+static void qcrypto_tls_cipher_suites_complete(UserCreatable *uc,
+                                               Error **errp)
+{
+    QCryptoTLSCreds *creds = QCRYPTO_TLS_CREDS(uc);
+
+    if (!creds->priority) {
+        error_setg(errp, "'priority' property is not set");
+        return;
+    }
+}
+
+static void qcrypto_tls_cipher_suites_class_init(ObjectClass *oc, void *data)
+{
+    UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+
+    ucc->complete = qcrypto_tls_cipher_suites_complete;
+}
+
+static const TypeInfo qcrypto_tls_cipher_suites_info = {
+    .parent = TYPE_QCRYPTO_TLS_CREDS,
+    .name = TYPE_QCRYPTO_TLS_CIPHER_SUITES,
+    .instance_size = sizeof(QCryptoTLSCreds),
+    .class_size = sizeof(QCryptoTLSCredsClass),
+    .class_init = qcrypto_tls_cipher_suites_class_init,
+    .interfaces = (InterfaceInfo[]) {
+        { TYPE_USER_CREATABLE },
+        { }
+    }
+};
+
+static void qcrypto_tls_cipher_suites_register_types(void)
+{
+    type_register_static(&qcrypto_tls_cipher_suites_info);
+}
+
+type_init(qcrypto_tls_cipher_suites_register_types);
diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs
index 707c02ad37..f1965b1a68 100644
--- a/crypto/Makefile.objs
+++ b/crypto/Makefile.objs
@@ -13,6 +13,7 @@ crypto-obj-y += cipher.o
 crypto-obj-$(CONFIG_AF_ALG) += afalg.o
 crypto-obj-$(CONFIG_AF_ALG) += cipher-afalg.o
 crypto-obj-$(CONFIG_AF_ALG) += hash-afalg.o
+crypto-obj-$(CONFIG_GNUTLS) += tls-cipher-suites.o
 crypto-obj-y += tlscreds.o
 crypto-obj-y += tlscredsanon.o
 crypto-obj-y += tlscredspsk.o
diff --git a/crypto/trace-events b/crypto/trace-events
index 9e594d30e8..798b6067ab 100644
--- a/crypto/trace-events
+++ b/crypto/trace-events
@@ -21,3 +21,8 @@ qcrypto_tls_creds_x509_load_cert_list(void *creds, const char *file) "TLS creds
 # tlssession.c
 qcrypto_tls_session_new(void *session, void *creds, const char *hostname, const char *authzid, int endpoint) "TLS session new session=%p creds=%p hostname=%s authzid=%s endpoint=%d"
 qcrypto_tls_session_check_creds(void *session, const char *status) "TLS session check creds session=%p status=%s"
+
+# tls-cipher-suites.c
+qcrypto_tls_cipher_suite_priority(const char *name) "priority: %s"
+qcrypto_tls_cipher_suite_info(uint8_t data0, uint8_t data1, const char *version, const char *name) "data=[0x%02x,0x%02x] version=%s name=%s"
+qcrypto_tls_cipher_suite_count(unsigned count) "count: %u"
diff --git a/qemu-options.hx b/qemu-options.hx
index 196f468786..ecc4658e1f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4567,6 +4567,25 @@ SRST
         string as described at
         https://gnutls.org/manual/html_node/Priority-Strings.html.
 
+    ``-object tls-cipher-suites,id=id,priority=priority``
+        Creates a TLS cipher suites object, which can be used to control
+        the TLS cipher/protocol algorithms that applications are permitted
+        to use.
+
+        The ``id`` parameter is a unique ID which frontends will use to
+        access the ordered list of permitted TLS cipher suites from the
+        host.
+
+        The ``priority`` parameter allows to override the global default
+        priority used by gnutls. This can be useful if the system
+        administrator needs to use a weaker set of crypto priorities for
+        QEMU without potentially forcing the weakness onto all
+        applications. Or conversely if one wants wants a stronger
+        default for QEMU than for all other applications, they can do
+        this through this parameter. Its format is a gnutls priority
+        string as described at
+        https://gnutls.org/manual/html_node/Priority-Strings.html.
+
     ``-object filter-buffer,id=id,netdev=netdevid,interval=t[,queue=all|rx|tx][,status=on|off][,position=head|tail|id=<id>][,insert=behind|before]``
         Interval t can't be 0, this filter batches the packet delivery:
         all packets arriving in a given interval on netdev netdevid are
-- 
2.21.3



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 2/5] hw/nvram/fw_cfg: Add the FW_CFG_DATA_GENERATOR interface
  2020-07-04 16:39 [PULL 0/5] fw_cfg/crypto patches for 5.1 soft freeze Philippe Mathieu-Daudé
  2020-07-04 16:39 ` [PULL 1/5] crypto: Add tls-cipher-suites object Philippe Mathieu-Daudé
@ 2020-07-04 16:39 ` Philippe Mathieu-Daudé
  2020-07-04 16:39 ` [PULL 3/5] softmmu/vl: Let -fw_cfg option take a 'gen_id' argument Philippe Mathieu-Daudé
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-04 16:39 UTC (permalink / raw)
  To: qemu-devel
  Cc: Philippe Mathieu-Daudé,
	Gerd Hoffmann, Laszlo Ersek, Daniel P. Berrangé,
	Paolo Bonzini

The FW_CFG_DATA_GENERATOR allows any object to produce
blob of data consumable by the fw_cfg device.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20200623172726.21040-3-philmd@redhat.com>
---
 docs/specs/fw_cfg.txt     |  9 +++++++-
 include/hw/nvram/fw_cfg.h | 43 +++++++++++++++++++++++++++++++++++++++
 hw/nvram/fw_cfg.c         | 35 +++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
index 8f1ebc66fa..bc16daa38a 100644
--- a/docs/specs/fw_cfg.txt
+++ b/docs/specs/fw_cfg.txt
@@ -219,7 +219,7 @@ To check the result, read the "control" field:
 
 = Externally Provided Items =
 
-As of v2.4, "file" fw_cfg items (i.e., items with selector keys above
+Since v2.4, "file" fw_cfg items (i.e., items with selector keys above
 FW_CFG_FILE_FIRST, and with a corresponding entry in the fw_cfg file
 directory structure) may be inserted via the QEMU command line, using
 the following syntax:
@@ -230,6 +230,13 @@ Or
 
     -fw_cfg [name=]<item_name>,string=<string>
 
+Since v5.1, QEMU allows some objects to generate fw_cfg-specific content,
+the content is then associated with a "file" item using the 'gen_id' option
+in the command line, using the following syntax:
+
+    -object <generator-type>,id=<generated_id>,[generator-specific-options] \
+    -fw_cfg [name=]<item_name>,gen_id=<generated_id>
+
 See QEMU man page for more documentation.
 
 Using item_name with plain ASCII characters only is recommended.
diff --git a/include/hw/nvram/fw_cfg.h b/include/hw/nvram/fw_cfg.h
index 25d9307018..11feae3177 100644
--- a/include/hw/nvram/fw_cfg.h
+++ b/include/hw/nvram/fw_cfg.h
@@ -9,11 +9,36 @@
 #define TYPE_FW_CFG     "fw_cfg"
 #define TYPE_FW_CFG_IO  "fw_cfg_io"
 #define TYPE_FW_CFG_MEM "fw_cfg_mem"
+#define TYPE_FW_CFG_DATA_GENERATOR_INTERFACE "fw_cfg-data-generator"
 
 #define FW_CFG(obj)     OBJECT_CHECK(FWCfgState,    (obj), TYPE_FW_CFG)
 #define FW_CFG_IO(obj)  OBJECT_CHECK(FWCfgIoState,  (obj), TYPE_FW_CFG_IO)
 #define FW_CFG_MEM(obj) OBJECT_CHECK(FWCfgMemState, (obj), TYPE_FW_CFG_MEM)
 
+#define FW_CFG_DATA_GENERATOR_CLASS(class) \
+    OBJECT_CLASS_CHECK(FWCfgDataGeneratorClass, (class), \
+                       TYPE_FW_CFG_DATA_GENERATOR_INTERFACE)
+#define FW_CFG_DATA_GENERATOR_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(FWCfgDataGeneratorClass, (obj), \
+                     TYPE_FW_CFG_DATA_GENERATOR_INTERFACE)
+
+typedef struct FWCfgDataGeneratorClass {
+    /*< private >*/
+    InterfaceClass parent_class;
+    /*< public >*/
+
+    /**
+     * get_data:
+     * @obj: the object implementing this interface
+     * @errp: pointer to a NULL-initialized error object
+     *
+     * Returns: reference to a byte array containing the data.
+     * The caller should release the reference when no longer
+     * required.
+     */
+    GByteArray *(*get_data)(Object *obj, Error **errp);
+} FWCfgDataGeneratorClass;
+
 typedef struct fw_cfg_file FWCfgFile;
 
 #define FW_CFG_ORDER_OVERRIDE_VGA    70
@@ -263,6 +288,24 @@ void fw_cfg_add_file_callback(FWCfgState *s, const char *filename,
 void *fw_cfg_modify_file(FWCfgState *s, const char *filename, void *data,
                          size_t len);
 
+/**
+ * fw_cfg_add_from_generator:
+ * @s: fw_cfg device being modified
+ * @filename: name of new fw_cfg file item
+ * @gen_id: name of object implementing FW_CFG_DATA_GENERATOR interface
+ * @errp: pointer to a NULL initialized error object
+ *
+ * Add a new NAMED fw_cfg item with the content generated from the
+ * @gen_id object. The data generated by the @gen_id object is copied
+ * into the data structure of the fw_cfg device.
+ * The next available (unused) selector key starting at FW_CFG_FILE_FIRST
+ * will be used; also, a new entry will be added to the file directory
+ * structure residing at key value FW_CFG_FILE_DIR, containing the item name,
+ * data size, and assigned selector key value.
+ */
+void fw_cfg_add_from_generator(FWCfgState *s, const char *filename,
+                               const char *gen_id, Error **errp);
+
 FWCfgState *fw_cfg_init_io_dma(uint32_t iobase, uint32_t dma_iobase,
                                 AddressSpace *dma_as);
 FWCfgState *fw_cfg_init_io(uint32_t iobase);
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 0408a31f8e..694722b212 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -1032,6 +1032,35 @@ void *fw_cfg_modify_file(FWCfgState *s, const char *filename,
     return NULL;
 }
 
+void fw_cfg_add_from_generator(FWCfgState *s, const char *filename,
+                               const char *gen_id, Error **errp)
+{
+    FWCfgDataGeneratorClass *klass;
+    Error *local_err = NULL;
+    GByteArray *array;
+    Object *obj;
+    gsize size;
+
+    obj = object_resolve_path_component(object_get_objects_root(), gen_id);
+    if (!obj) {
+        error_setg(errp, "Cannot find object ID '%s'", gen_id);
+        return;
+    }
+    if (!object_dynamic_cast(obj, TYPE_FW_CFG_DATA_GENERATOR_INTERFACE)) {
+        error_setg(errp, "Object ID '%s' is not a '%s' subclass",
+                   gen_id, TYPE_FW_CFG_DATA_GENERATOR_INTERFACE);
+        return;
+    }
+    klass = FW_CFG_DATA_GENERATOR_GET_CLASS(obj);
+    array = klass->get_data(obj, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        return;
+    }
+    size = array->len;
+    fw_cfg_add_file(s, filename, g_byte_array_free(array, TRUE), size);
+}
+
 static void fw_cfg_machine_reset(void *opaque)
 {
     MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
@@ -1333,12 +1362,18 @@ static const TypeInfo fw_cfg_mem_info = {
     .class_init    = fw_cfg_mem_class_init,
 };
 
+static const TypeInfo fw_cfg_data_generator_interface_info = {
+    .parent = TYPE_INTERFACE,
+    .name = TYPE_FW_CFG_DATA_GENERATOR_INTERFACE,
+    .class_size = sizeof(FWCfgDataGeneratorClass),
+};
 
 static void fw_cfg_register_types(void)
 {
     type_register_static(&fw_cfg_info);
     type_register_static(&fw_cfg_io_info);
     type_register_static(&fw_cfg_mem_info);
+    type_register_static(&fw_cfg_data_generator_interface_info);
 }
 
 type_init(fw_cfg_register_types)
-- 
2.21.3



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 3/5] softmmu/vl: Let -fw_cfg option take a 'gen_id' argument
  2020-07-04 16:39 [PULL 0/5] fw_cfg/crypto patches for 5.1 soft freeze Philippe Mathieu-Daudé
  2020-07-04 16:39 ` [PULL 1/5] crypto: Add tls-cipher-suites object Philippe Mathieu-Daudé
  2020-07-04 16:39 ` [PULL 2/5] hw/nvram/fw_cfg: Add the FW_CFG_DATA_GENERATOR interface Philippe Mathieu-Daudé
@ 2020-07-04 16:39 ` Philippe Mathieu-Daudé
  2020-07-13 13:13   ` Peter Maydell
  2020-07-04 16:39 ` [PULL 4/5] softmmu/vl: Allow -fw_cfg 'gen_id' option to use the 'etc/' namespace Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-04 16:39 UTC (permalink / raw)
  To: qemu-devel
  Cc: Philippe Mathieu-Daudé,
	Gerd Hoffmann, Laszlo Ersek, Daniel P. Berrangé,
	Paolo Bonzini

The 'gen_id' argument refers to a QOM object able to produce
data consumable by the fw_cfg device. The producer object must
implement the FW_CFG_DATA_GENERATOR interface.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20200623172726.21040-4-philmd@redhat.com>
---
 softmmu/vl.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/softmmu/vl.c b/softmmu/vl.c
index 3e15ee2435..13cada39d6 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -489,6 +489,11 @@ static QemuOptsList qemu_fw_cfg_opts = {
             .name = "string",
             .type = QEMU_OPT_STRING,
             .help = "Sets content of the blob to be inserted from a string",
+        }, {
+            .name = "gen_id",
+            .type = QEMU_OPT_STRING,
+            .help = "Sets id of the object generating the fw_cfg blob "
+                    "to be inserted",
         },
         { /* end of list */ }
     },
@@ -2020,7 +2025,7 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
 {
     gchar *buf;
     size_t size;
-    const char *name, *file, *str;
+    const char *name, *file, *str, *gen_id;
     FWCfgState *fw_cfg = (FWCfgState *) opaque;
 
     if (fw_cfg == NULL) {
@@ -2030,14 +2035,13 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
     name = qemu_opt_get(opts, "name");
     file = qemu_opt_get(opts, "file");
     str = qemu_opt_get(opts, "string");
+    gen_id = qemu_opt_get(opts, "gen_id");
 
-    /* we need name and either a file or the content string */
-    if (!(nonempty_str(name) && (nonempty_str(file) || nonempty_str(str)))) {
-        error_setg(errp, "invalid argument(s)");
-        return -1;
-    }
-    if (nonempty_str(file) && nonempty_str(str)) {
-        error_setg(errp, "file and string are mutually exclusive");
+    /* we need the name, and exactly one of: file, content string, gen_id */
+    if (!nonempty_str(name) ||
+        nonempty_str(file) + nonempty_str(str) + nonempty_str(gen_id) != 1) {
+        error_setg(errp, "name, plus exactly one of file,"
+                         " string and gen_id, are needed");
         return -1;
     }
     if (strlen(name) > FW_CFG_MAX_FILE_PATH - 1) {
@@ -2052,6 +2056,15 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
     if (nonempty_str(str)) {
         size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
         buf = g_memdup(str, size);
+    } else if (nonempty_str(gen_id)) {
+        Error *local_err = NULL;
+
+        fw_cfg_add_from_generator(fw_cfg, name, gen_id, errp);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return -1;
+        }
+        return 0;
     } else {
         GError *err = NULL;
         if (!g_file_get_contents(file, &buf, &size, &err)) {
-- 
2.21.3



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 4/5] softmmu/vl: Allow -fw_cfg 'gen_id' option to use the 'etc/' namespace
  2020-07-04 16:39 [PULL 0/5] fw_cfg/crypto patches for 5.1 soft freeze Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2020-07-04 16:39 ` [PULL 3/5] softmmu/vl: Let -fw_cfg option take a 'gen_id' argument Philippe Mathieu-Daudé
@ 2020-07-04 16:39 ` Philippe Mathieu-Daudé
  2020-07-04 16:39 ` [PULL 5/5] crypto/tls-cipher-suites: Produce fw_cfg consumable blob Philippe Mathieu-Daudé
  2020-07-10  8:01 ` [PULL 0/5] fw_cfg/crypto patches for 5.1 soft freeze Peter Maydell
  5 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-04 16:39 UTC (permalink / raw)
  To: qemu-devel
  Cc: Philippe Mathieu-Daudé,
	Gerd Hoffmann, Laszlo Ersek, Daniel P. Berrangé,
	Paolo Bonzini

Names of user-provided fw_cfg items are supposed to start
with "opt/". However FW_CFG_DATA_GENERATOR items are generated
by QEMU, so allow the "etc/" namespace in this specific case.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20200623172726.21040-5-philmd@redhat.com>
---
 docs/specs/fw_cfg.txt | 4 ++++
 softmmu/vl.c          | 8 +++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/docs/specs/fw_cfg.txt b/docs/specs/fw_cfg.txt
index bc16daa38a..3e6d586f66 100644
--- a/docs/specs/fw_cfg.txt
+++ b/docs/specs/fw_cfg.txt
@@ -258,4 +258,8 @@ Prefix "opt/org.qemu/" is reserved for QEMU itself.
 Use of names not beginning with "opt/" is potentially dangerous and
 entirely unsupported.  QEMU will warn if you try.
 
+Use of names not beginning with "opt/" is tolerated with 'gen_id' (that
+is, the warning is suppressed), but you must know exactly what you're
+doing.
+
 All externally provided fw_cfg items are read-only to the guest.
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 13cada39d6..159f0352a9 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2049,7 +2049,13 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
                    FW_CFG_MAX_FILE_PATH - 1);
         return -1;
     }
-    if (strncmp(name, "opt/", 4) != 0) {
+    if (nonempty_str(gen_id)) {
+        /*
+         * In this particular case where the content is populated
+         * internally, the "etc/" namespace protection is relaxed,
+         * so do not emit a warning.
+         */
+    } else if (strncmp(name, "opt/", 4) != 0) {
         warn_report("externally provided fw_cfg item names "
                     "should be prefixed with \"opt/\"");
     }
-- 
2.21.3



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PULL 5/5] crypto/tls-cipher-suites: Produce fw_cfg consumable blob
  2020-07-04 16:39 [PULL 0/5] fw_cfg/crypto patches for 5.1 soft freeze Philippe Mathieu-Daudé
                   ` (3 preceding siblings ...)
  2020-07-04 16:39 ` [PULL 4/5] softmmu/vl: Allow -fw_cfg 'gen_id' option to use the 'etc/' namespace Philippe Mathieu-Daudé
@ 2020-07-04 16:39 ` Philippe Mathieu-Daudé
  2020-09-29 15:46   ` Kevin Wolf
  2020-07-10  8:01 ` [PULL 0/5] fw_cfg/crypto patches for 5.1 soft freeze Peter Maydell
  5 siblings, 1 reply; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-04 16:39 UTC (permalink / raw)
  To: qemu-devel
  Cc: Philippe Mathieu-Daudé,
	Gerd Hoffmann, Laszlo Ersek, Daniel P. Berrangé,
	Paolo Bonzini

Since our format is consumable by the fw_cfg device,
we can implement the FW_CFG_DATA_GENERATOR interface.

Example of use to dump the cipher suites (if tracing enabled):

  $ qemu-system-x86_64 -S \
    -object tls-cipher-suites,id=mysuite1,priority=@SYSTEM \
    -fw_cfg name=etc/path/to/ciphers,gen_id=mysuite1 \
    -trace qcrypto\*
  1590664444.197123:qcrypto_tls_cipher_suite_priority priority: @SYSTEM
  1590664444.197219:qcrypto_tls_cipher_suite_info data=[0x13,0x02] version=TLS1.3 name=TLS_AES_256_GCM_SHA384
  1590664444.197228:qcrypto_tls_cipher_suite_info data=[0x13,0x03] version=TLS1.3 name=TLS_CHACHA20_POLY1305_SHA256
  1590664444.197233:qcrypto_tls_cipher_suite_info data=[0x13,0x01] version=TLS1.3 name=TLS_AES_128_GCM_SHA256
  1590664444.197236:qcrypto_tls_cipher_suite_info data=[0x13,0x04] version=TLS1.3 name=TLS_AES_128_CCM_SHA256
  1590664444.197240:qcrypto_tls_cipher_suite_info data=[0xc0,0x30] version=TLS1.2 name=TLS_ECDHE_RSA_AES_256_GCM_SHA384
  1590664444.197245:qcrypto_tls_cipher_suite_info data=[0xcc,0xa8] version=TLS1.2 name=TLS_ECDHE_RSA_CHACHA20_POLY1305
  1590664444.197250:qcrypto_tls_cipher_suite_info data=[0xc0,0x14] version=TLS1.0 name=TLS_ECDHE_RSA_AES_256_CBC_SHA1
  1590664444.197254:qcrypto_tls_cipher_suite_info data=[0xc0,0x2f] version=TLS1.2 name=TLS_ECDHE_RSA_AES_128_GCM_SHA256
  1590664444.197258:qcrypto_tls_cipher_suite_info data=[0xc0,0x13] version=TLS1.0 name=TLS_ECDHE_RSA_AES_128_CBC_SHA1
  1590664444.197261:qcrypto_tls_cipher_suite_info data=[0xc0,0x2c] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_256_GCM_SHA384
  1590664444.197266:qcrypto_tls_cipher_suite_info data=[0xcc,0xa9] version=TLS1.2 name=TLS_ECDHE_ECDSA_CHACHA20_POLY1305
  1590664444.197270:qcrypto_tls_cipher_suite_info data=[0xc0,0xad] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_256_CCM
  1590664444.197274:qcrypto_tls_cipher_suite_info data=[0xc0,0x0a] version=TLS1.0 name=TLS_ECDHE_ECDSA_AES_256_CBC_SHA1
  1590664444.197278:qcrypto_tls_cipher_suite_info data=[0xc0,0x2b] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_128_GCM_SHA256
  1590664444.197283:qcrypto_tls_cipher_suite_info data=[0xc0,0xac] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_128_CCM
  1590664444.197287:qcrypto_tls_cipher_suite_info data=[0xc0,0x09] version=TLS1.0 name=TLS_ECDHE_ECDSA_AES_128_CBC_SHA1
  1590664444.197291:qcrypto_tls_cipher_suite_info data=[0x00,0x9d] version=TLS1.2 name=TLS_RSA_AES_256_GCM_SHA384
  1590664444.197296:qcrypto_tls_cipher_suite_info data=[0xc0,0x9d] version=TLS1.2 name=TLS_RSA_AES_256_CCM
  1590664444.197300:qcrypto_tls_cipher_suite_info data=[0x00,0x35] version=TLS1.0 name=TLS_RSA_AES_256_CBC_SHA1
  1590664444.197304:qcrypto_tls_cipher_suite_info data=[0x00,0x9c] version=TLS1.2 name=TLS_RSA_AES_128_GCM_SHA256
  1590664444.197308:qcrypto_tls_cipher_suite_info data=[0xc0,0x9c] version=TLS1.2 name=TLS_RSA_AES_128_CCM
  1590664444.197312:qcrypto_tls_cipher_suite_info data=[0x00,0x2f] version=TLS1.0 name=TLS_RSA_AES_128_CBC_SHA1
  1590664444.197316:qcrypto_tls_cipher_suite_info data=[0x00,0x9f] version=TLS1.2 name=TLS_DHE_RSA_AES_256_GCM_SHA384
  1590664444.197320:qcrypto_tls_cipher_suite_info data=[0xcc,0xaa] version=TLS1.2 name=TLS_DHE_RSA_CHACHA20_POLY1305
  1590664444.197325:qcrypto_tls_cipher_suite_info data=[0xc0,0x9f] version=TLS1.2 name=TLS_DHE_RSA_AES_256_CCM
  1590664444.197329:qcrypto_tls_cipher_suite_info data=[0x00,0x39] version=TLS1.0 name=TLS_DHE_RSA_AES_256_CBC_SHA1
  1590664444.197333:qcrypto_tls_cipher_suite_info data=[0x00,0x9e] version=TLS1.2 name=TLS_DHE_RSA_AES_128_GCM_SHA256
  1590664444.197337:qcrypto_tls_cipher_suite_info data=[0xc0,0x9e] version=TLS1.2 name=TLS_DHE_RSA_AES_128_CCM
  1590664444.197341:qcrypto_tls_cipher_suite_info data=[0x00,0x33] version=TLS1.0 name=TLS_DHE_RSA_AES_128_CBC_SHA1
  1590664444.197345:qcrypto_tls_cipher_suite_count count: 29

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Acked-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20200623172726.21040-6-philmd@redhat.com>
---
 crypto/tls-cipher-suites.c | 11 +++++++++++
 qemu-options.hx            | 18 ++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/crypto/tls-cipher-suites.c b/crypto/tls-cipher-suites.c
index a4e0f84307..0d305b684b 100644
--- a/crypto/tls-cipher-suites.c
+++ b/crypto/tls-cipher-suites.c
@@ -13,6 +13,7 @@
 #include "qom/object_interfaces.h"
 #include "crypto/tlscreds.h"
 #include "crypto/tls-cipher-suites.h"
+#include "hw/nvram/fw_cfg.h"
 #include "trace.h"
 
 /*
@@ -88,11 +89,20 @@ static void qcrypto_tls_cipher_suites_complete(UserCreatable *uc,
     }
 }
 
+static GByteArray *qcrypto_tls_cipher_suites_fw_cfg_gen_data(Object *obj,
+                                                             Error **errp)
+{
+    return qcrypto_tls_cipher_suites_get_data(QCRYPTO_TLS_CIPHER_SUITES(obj),
+                                              errp);
+}
+
 static void qcrypto_tls_cipher_suites_class_init(ObjectClass *oc, void *data)
 {
     UserCreatableClass *ucc = USER_CREATABLE_CLASS(oc);
+    FWCfgDataGeneratorClass *fwgc = FW_CFG_DATA_GENERATOR_CLASS(oc);
 
     ucc->complete = qcrypto_tls_cipher_suites_complete;
+    fwgc->get_data = qcrypto_tls_cipher_suites_fw_cfg_gen_data;
 }
 
 static const TypeInfo qcrypto_tls_cipher_suites_info = {
@@ -103,6 +113,7 @@ static const TypeInfo qcrypto_tls_cipher_suites_info = {
     .class_init = qcrypto_tls_cipher_suites_class_init,
     .interfaces = (InterfaceInfo[]) {
         { TYPE_USER_CREATABLE },
+        { TYPE_FW_CFG_DATA_GENERATOR_INTERFACE },
         { }
     }
 };
diff --git a/qemu-options.hx b/qemu-options.hx
index ecc4658e1f..b2cbbbf281 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4586,6 +4586,24 @@ SRST
         string as described at
         https://gnutls.org/manual/html_node/Priority-Strings.html.
 
+        An example of use of this object is to control UEFI HTTPS Boot.
+        The tls-cipher-suites object exposes the ordered list of permitted
+        TLS cipher suites from the host side to the guest firmware, via
+        fw_cfg. The list is represented as an array of IANA_TLS_CIPHER
+        objects. The firmware uses the IANA_TLS_CIPHER array for configuring
+        guest-side TLS.
+
+        In the following example, the priority at which the host-side policy
+        is retrieved is given by the ``priority`` property.
+        Given that QEMU uses GNUTLS, ``priority=@SYSTEM`` may be used to
+        refer to /etc/crypto-policies/back-ends/gnutls.config.
+
+        .. parsed-literal::
+
+             # |qemu_system| \
+                 -object tls-cipher-suites,id=mysuite0,priority=@SYSTEM \
+                 -fw_cfg name=etc/edk2/https/ciphers,gen_id=mysuite0
+
     ``-object filter-buffer,id=id,netdev=netdevid,interval=t[,queue=all|rx|tx][,status=on|off][,position=head|tail|id=<id>][,insert=behind|before]``
         Interval t can't be 0, this filter batches the packet delivery:
         all packets arriving in a given interval on netdev netdevid are
-- 
2.21.3



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PULL 0/5] fw_cfg/crypto patches for 5.1 soft freeze
  2020-07-04 16:39 [PULL 0/5] fw_cfg/crypto patches for 5.1 soft freeze Philippe Mathieu-Daudé
                   ` (4 preceding siblings ...)
  2020-07-04 16:39 ` [PULL 5/5] crypto/tls-cipher-suites: Produce fw_cfg consumable blob Philippe Mathieu-Daudé
@ 2020-07-10  8:01 ` Peter Maydell
  5 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2020-07-10  8:01 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Daniel P. Berrangé,
	Paolo Bonzini, Laszlo Ersek, QEMU Developers, Gerd Hoffmann

On Sat, 4 Jul 2020 at 17:41, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> The following changes since commit 4abf70a661a5df3886ac9d7c19c3617fa92b922a:
>
>   Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2020-06-24' =
> into staging (2020-07-03 15:34:45 +0100)
>
> are available in the Git repository at:
>
>   https://gitlab.com/philmd/qemu.git tags/fw_cfg-20200704
>
> for you to fetch changes up to 69699f3055a59e24f1153c329ae6eff4b9a343e0:
>
>   crypto/tls-cipher-suites: Produce fw_cfg consumable blob (2020-07-03 18:16:=
> 01 +0200)
>
> ----------------------------------------------------------------
> firmware (and crypto) patches
>
> - add the tls-cipher-suites object,
> - add the ability to QOM objects to produce data consumable
>   by the fw_cfg device,
> - let the tls-cipher-suites object implement the
>   FW_CFG_DATA_GENERATOR interface.
>
> This is required by EDK2 'HTTPS Boot' feature of OVMF to tell
> the guest which TLS ciphers it can use.
>
> CI jobs results:
>   https://travis-ci.org/github/philmd/qemu/builds/704724619
>   https://gitlab.com/philmd/qemu/-/pipelines/162938106
>   https://cirrus-ci.com/build/4682977303068672
>
> ----------------------------------------------------------------
>
> Philippe Mathieu-Daud=C3=A9 (5):
>   crypto: Add tls-cipher-suites object
>   hw/nvram/fw_cfg: Add the FW_CFG_DATA_GENERATOR interface
>   softmmu/vl: Let -fw_cfg option take a 'gen_id' argument
>   softmmu/vl: Allow -fw_cfg 'gen_id' option to use the 'etc/' namespace
>   crypto/tls-cipher-suites: Produce fw_cfg consumable blob


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL 3/5] softmmu/vl: Let -fw_cfg option take a 'gen_id' argument
  2020-07-04 16:39 ` [PULL 3/5] softmmu/vl: Let -fw_cfg option take a 'gen_id' argument Philippe Mathieu-Daudé
@ 2020-07-13 13:13   ` Peter Maydell
  2020-07-13 14:50     ` Laszlo Ersek
  0 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2020-07-13 13:13 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Daniel P. Berrangé,
	Paolo Bonzini, Laszlo Ersek, QEMU Developers, Gerd Hoffmann

On Sat, 4 Jul 2020 at 17:41, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> The 'gen_id' argument refers to a QOM object able to produce
> data consumable by the fw_cfg device. The producer object must
> implement the FW_CFG_DATA_GENERATOR interface.
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Message-Id: <20200623172726.21040-4-philmd@redhat.com>

Coverity points out (CID 1430396) an issue with the error handling
in this patch:


> @@ -2052,6 +2056,15 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
>      if (nonempty_str(str)) {
>          size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
>          buf = g_memdup(str, size);
> +    } else if (nonempty_str(gen_id)) {
> +        Error *local_err = NULL;

We set local_err to NULL here...

> +
> +        fw_cfg_add_from_generator(fw_cfg, name, gen_id, errp);

...but we don't pass it to the function here...

> +        if (local_err) {

...so this condition is always false and the body of the if is dead code.

> +            error_propagate(errp, local_err);
> +            return -1;
> +        }
> +        return 0;
>      } else {
>          GError *err = NULL;
>          if (!g_file_get_contents(file, &buf, &size, &err)) {

thanks
-- PMM


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL 3/5] softmmu/vl: Let -fw_cfg option take a 'gen_id' argument
  2020-07-13 13:13   ` Peter Maydell
@ 2020-07-13 14:50     ` Laszlo Ersek
  2020-07-13 14:55       ` Peter Maydell
  2020-07-14 13:04       ` Philippe Mathieu-Daudé
  0 siblings, 2 replies; 16+ messages in thread
From: Laszlo Ersek @ 2020-07-13 14:50 UTC (permalink / raw)
  To: Peter Maydell, Philippe Mathieu-Daudé
  Cc: Paolo Bonzini, Daniel P. Berrangé, QEMU Developers, Gerd Hoffmann

On 07/13/20 15:13, Peter Maydell wrote:
> On Sat, 4 Jul 2020 at 17:41, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>> The 'gen_id' argument refers to a QOM object able to produce
>> data consumable by the fw_cfg device. The producer object must
>> implement the FW_CFG_DATA_GENERATOR interface.
>>
>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> Message-Id: <20200623172726.21040-4-philmd@redhat.com>
> 
> Coverity points out (CID 1430396) an issue with the error handling
> in this patch:
> 
> 
>> @@ -2052,6 +2056,15 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
>>      if (nonempty_str(str)) {
>>          size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
>>          buf = g_memdup(str, size);
>> +    } else if (nonempty_str(gen_id)) {
>> +        Error *local_err = NULL;
> 
> We set local_err to NULL here...
> 
>> +
>> +        fw_cfg_add_from_generator(fw_cfg, name, gen_id, errp);
> 
> ...but we don't pass it to the function here...

Ugh, I should have noticed that in review. I'm sorry.

Laszlo

> 
>> +        if (local_err) {
> 
> ...so this condition is always false and the body of the if is dead code.
> 
>> +            error_propagate(errp, local_err);
>> +            return -1;
>> +        }
>> +        return 0;
>>      } else {
>>          GError *err = NULL;
>>          if (!g_file_get_contents(file, &buf, &size, &err)) {
> 
> thanks
> -- PMM
> 



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL 3/5] softmmu/vl: Let -fw_cfg option take a 'gen_id' argument
  2020-07-13 14:50     ` Laszlo Ersek
@ 2020-07-13 14:55       ` Peter Maydell
  2020-07-14 13:04       ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2020-07-13 14:55 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: Daniel P. Berrangé,
	Paolo Bonzini, Philippe Mathieu-Daudé,
	QEMU Developers, Gerd Hoffmann

On Mon, 13 Jul 2020 at 15:50, Laszlo Ersek <lersek@redhat.com> wrote:
> Ugh, I should have noticed that in review. I'm sorry.

No worries, catching this kind of thing is what static
analysers are for :-)

-- PMM


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL 3/5] softmmu/vl: Let -fw_cfg option take a 'gen_id' argument
  2020-07-13 14:50     ` Laszlo Ersek
  2020-07-13 14:55       ` Peter Maydell
@ 2020-07-14 13:04       ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-14 13:04 UTC (permalink / raw)
  To: Laszlo Ersek, Peter Maydell
  Cc: Paolo Bonzini, Daniel P. Berrangé, QEMU Developers, Gerd Hoffmann

On 7/13/20 4:50 PM, Laszlo Ersek wrote:
> On 07/13/20 15:13, Peter Maydell wrote:
>> On Sat, 4 Jul 2020 at 17:41, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>>
>>> The 'gen_id' argument refers to a QOM object able to produce
>>> data consumable by the fw_cfg device. The producer object must
>>> implement the FW_CFG_DATA_GENERATOR interface.
>>>
>>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>> Message-Id: <20200623172726.21040-4-philmd@redhat.com>
>>
>> Coverity points out (CID 1430396) an issue with the error handling
>> in this patch:
>>
>>
>>> @@ -2052,6 +2056,15 @@ static int parse_fw_cfg(void *opaque, QemuOpts *opts, Error **errp)
>>>      if (nonempty_str(str)) {
>>>          size = strlen(str); /* NUL terminator NOT included in fw_cfg blob */
>>>          buf = g_memdup(str, size);
>>> +    } else if (nonempty_str(gen_id)) {
>>> +        Error *local_err = NULL;
>>
>> We set local_err to NULL here...
>>
>>> +
>>> +        fw_cfg_add_from_generator(fw_cfg, name, gen_id, errp);
>>
>> ...but we don't pass it to the function here...
> 
> Ugh, I should have noticed that in review. I'm sorry.

You reviewed v9 which was OK, I added it while addressing
Daniel comment for v10, while keeping your R-b tag from v9.
Since you already had reviewed 9 different versions, I didn't
want to overload you for a trivial change, but I should have
dropped your tag to be certain.

Also this has been merged at the same time Markus was doing
a big rework on the Error API, so I was very confused between
reviews of the new API.

So don't be sorry, Daniel/Myself let that in ;)

I'll send a patch, hoping it can be queued via qemu-trivial.

Regards,

Phil.

> Laszlo
> 
>>
>>> +        if (local_err) {
>>
>> ...so this condition is always false and the body of the if is dead code.
>>
>>> +            error_propagate(errp, local_err);
>>> +            return -1;
>>> +        }
>>> +        return 0;
>>>      } else {
>>>          GError *err = NULL;
>>>          if (!g_file_get_contents(file, &buf, &size, &err)) {
>>
>> thanks
>> -- PMM
>>
> 



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL 5/5] crypto/tls-cipher-suites: Produce fw_cfg consumable blob
  2020-07-04 16:39 ` [PULL 5/5] crypto/tls-cipher-suites: Produce fw_cfg consumable blob Philippe Mathieu-Daudé
@ 2020-09-29 15:46   ` Kevin Wolf
  2020-10-01  7:18     ` Laszlo Ersek
  0 siblings, 1 reply; 16+ messages in thread
From: Kevin Wolf @ 2020-09-29 15:46 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Daniel P. Berrangé,
	qemu-block, qemu-devel, Gerd Hoffmann, Paolo Bonzini,
	Laszlo Ersek

Am 04.07.2020 um 18:39 hat Philippe Mathieu-Daudé geschrieben:
> Since our format is consumable by the fw_cfg device,
> we can implement the FW_CFG_DATA_GENERATOR interface.
> 
> Example of use to dump the cipher suites (if tracing enabled):
> 
>   $ qemu-system-x86_64 -S \
>     -object tls-cipher-suites,id=mysuite1,priority=@SYSTEM \
>     -fw_cfg name=etc/path/to/ciphers,gen_id=mysuite1 \
>     -trace qcrypto\*
>   1590664444.197123:qcrypto_tls_cipher_suite_priority priority: @SYSTEM
>   1590664444.197219:qcrypto_tls_cipher_suite_info data=[0x13,0x02] version=TLS1.3 name=TLS_AES_256_GCM_SHA384
>   1590664444.197228:qcrypto_tls_cipher_suite_info data=[0x13,0x03] version=TLS1.3 name=TLS_CHACHA20_POLY1305_SHA256
>   1590664444.197233:qcrypto_tls_cipher_suite_info data=[0x13,0x01] version=TLS1.3 name=TLS_AES_128_GCM_SHA256
>   1590664444.197236:qcrypto_tls_cipher_suite_info data=[0x13,0x04] version=TLS1.3 name=TLS_AES_128_CCM_SHA256
>   1590664444.197240:qcrypto_tls_cipher_suite_info data=[0xc0,0x30] version=TLS1.2 name=TLS_ECDHE_RSA_AES_256_GCM_SHA384
>   1590664444.197245:qcrypto_tls_cipher_suite_info data=[0xcc,0xa8] version=TLS1.2 name=TLS_ECDHE_RSA_CHACHA20_POLY1305
>   1590664444.197250:qcrypto_tls_cipher_suite_info data=[0xc0,0x14] version=TLS1.0 name=TLS_ECDHE_RSA_AES_256_CBC_SHA1
>   1590664444.197254:qcrypto_tls_cipher_suite_info data=[0xc0,0x2f] version=TLS1.2 name=TLS_ECDHE_RSA_AES_128_GCM_SHA256
>   1590664444.197258:qcrypto_tls_cipher_suite_info data=[0xc0,0x13] version=TLS1.0 name=TLS_ECDHE_RSA_AES_128_CBC_SHA1
>   1590664444.197261:qcrypto_tls_cipher_suite_info data=[0xc0,0x2c] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_256_GCM_SHA384
>   1590664444.197266:qcrypto_tls_cipher_suite_info data=[0xcc,0xa9] version=TLS1.2 name=TLS_ECDHE_ECDSA_CHACHA20_POLY1305
>   1590664444.197270:qcrypto_tls_cipher_suite_info data=[0xc0,0xad] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_256_CCM
>   1590664444.197274:qcrypto_tls_cipher_suite_info data=[0xc0,0x0a] version=TLS1.0 name=TLS_ECDHE_ECDSA_AES_256_CBC_SHA1
>   1590664444.197278:qcrypto_tls_cipher_suite_info data=[0xc0,0x2b] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_128_GCM_SHA256
>   1590664444.197283:qcrypto_tls_cipher_suite_info data=[0xc0,0xac] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_128_CCM
>   1590664444.197287:qcrypto_tls_cipher_suite_info data=[0xc0,0x09] version=TLS1.0 name=TLS_ECDHE_ECDSA_AES_128_CBC_SHA1
>   1590664444.197291:qcrypto_tls_cipher_suite_info data=[0x00,0x9d] version=TLS1.2 name=TLS_RSA_AES_256_GCM_SHA384
>   1590664444.197296:qcrypto_tls_cipher_suite_info data=[0xc0,0x9d] version=TLS1.2 name=TLS_RSA_AES_256_CCM
>   1590664444.197300:qcrypto_tls_cipher_suite_info data=[0x00,0x35] version=TLS1.0 name=TLS_RSA_AES_256_CBC_SHA1
>   1590664444.197304:qcrypto_tls_cipher_suite_info data=[0x00,0x9c] version=TLS1.2 name=TLS_RSA_AES_128_GCM_SHA256
>   1590664444.197308:qcrypto_tls_cipher_suite_info data=[0xc0,0x9c] version=TLS1.2 name=TLS_RSA_AES_128_CCM
>   1590664444.197312:qcrypto_tls_cipher_suite_info data=[0x00,0x2f] version=TLS1.0 name=TLS_RSA_AES_128_CBC_SHA1
>   1590664444.197316:qcrypto_tls_cipher_suite_info data=[0x00,0x9f] version=TLS1.2 name=TLS_DHE_RSA_AES_256_GCM_SHA384
>   1590664444.197320:qcrypto_tls_cipher_suite_info data=[0xcc,0xaa] version=TLS1.2 name=TLS_DHE_RSA_CHACHA20_POLY1305
>   1590664444.197325:qcrypto_tls_cipher_suite_info data=[0xc0,0x9f] version=TLS1.2 name=TLS_DHE_RSA_AES_256_CCM
>   1590664444.197329:qcrypto_tls_cipher_suite_info data=[0x00,0x39] version=TLS1.0 name=TLS_DHE_RSA_AES_256_CBC_SHA1
>   1590664444.197333:qcrypto_tls_cipher_suite_info data=[0x00,0x9e] version=TLS1.2 name=TLS_DHE_RSA_AES_128_GCM_SHA256
>   1590664444.197337:qcrypto_tls_cipher_suite_info data=[0xc0,0x9e] version=TLS1.2 name=TLS_DHE_RSA_AES_128_CCM
>   1590664444.197341:qcrypto_tls_cipher_suite_info data=[0x00,0x33] version=TLS1.0 name=TLS_DHE_RSA_AES_128_CBC_SHA1
>   1590664444.197345:qcrypto_tls_cipher_suite_count count: 29
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> Message-Id: <20200623172726.21040-6-philmd@redhat.com>

I noticed only now that this breaks '--object help' in
qemu-storage-daemon:

$ qemu-storage-daemon --object help
List of user creatable objects:
qemu-storage-daemon: missing interface 'fw_cfg-data-generator' for object 'tls-creds'
Aborted (core dumped)

The reason is that we don't (and can't) link hw/nvram/fw_cfg.c into the
storage daemon because it requires other system emulator stuff.

Kevin



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL 5/5] crypto/tls-cipher-suites: Produce fw_cfg consumable blob
  2020-09-29 15:46   ` Kevin Wolf
@ 2020-10-01  7:18     ` Laszlo Ersek
  2020-10-05  9:16       ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 16+ messages in thread
From: Laszlo Ersek @ 2020-10-01  7:18 UTC (permalink / raw)
  To: Kevin Wolf, Philippe Mathieu-Daudé
  Cc: Paolo Bonzini, Daniel P. Berrangé,
	qemu-devel, qemu-block, Gerd Hoffmann

On 09/29/20 17:46, Kevin Wolf wrote:
> Am 04.07.2020 um 18:39 hat Philippe Mathieu-Daudé geschrieben:
>> Since our format is consumable by the fw_cfg device,
>> we can implement the FW_CFG_DATA_GENERATOR interface.
>>
>> Example of use to dump the cipher suites (if tracing enabled):
>>
>>   $ qemu-system-x86_64 -S \
>>     -object tls-cipher-suites,id=mysuite1,priority=@SYSTEM \
>>     -fw_cfg name=etc/path/to/ciphers,gen_id=mysuite1 \
>>     -trace qcrypto\*
>>   1590664444.197123:qcrypto_tls_cipher_suite_priority priority: @SYSTEM
>>   1590664444.197219:qcrypto_tls_cipher_suite_info data=[0x13,0x02] version=TLS1.3 name=TLS_AES_256_GCM_SHA384
>>   1590664444.197228:qcrypto_tls_cipher_suite_info data=[0x13,0x03] version=TLS1.3 name=TLS_CHACHA20_POLY1305_SHA256
>>   1590664444.197233:qcrypto_tls_cipher_suite_info data=[0x13,0x01] version=TLS1.3 name=TLS_AES_128_GCM_SHA256
>>   1590664444.197236:qcrypto_tls_cipher_suite_info data=[0x13,0x04] version=TLS1.3 name=TLS_AES_128_CCM_SHA256
>>   1590664444.197240:qcrypto_tls_cipher_suite_info data=[0xc0,0x30] version=TLS1.2 name=TLS_ECDHE_RSA_AES_256_GCM_SHA384
>>   1590664444.197245:qcrypto_tls_cipher_suite_info data=[0xcc,0xa8] version=TLS1.2 name=TLS_ECDHE_RSA_CHACHA20_POLY1305
>>   1590664444.197250:qcrypto_tls_cipher_suite_info data=[0xc0,0x14] version=TLS1.0 name=TLS_ECDHE_RSA_AES_256_CBC_SHA1
>>   1590664444.197254:qcrypto_tls_cipher_suite_info data=[0xc0,0x2f] version=TLS1.2 name=TLS_ECDHE_RSA_AES_128_GCM_SHA256
>>   1590664444.197258:qcrypto_tls_cipher_suite_info data=[0xc0,0x13] version=TLS1.0 name=TLS_ECDHE_RSA_AES_128_CBC_SHA1
>>   1590664444.197261:qcrypto_tls_cipher_suite_info data=[0xc0,0x2c] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_256_GCM_SHA384
>>   1590664444.197266:qcrypto_tls_cipher_suite_info data=[0xcc,0xa9] version=TLS1.2 name=TLS_ECDHE_ECDSA_CHACHA20_POLY1305
>>   1590664444.197270:qcrypto_tls_cipher_suite_info data=[0xc0,0xad] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_256_CCM
>>   1590664444.197274:qcrypto_tls_cipher_suite_info data=[0xc0,0x0a] version=TLS1.0 name=TLS_ECDHE_ECDSA_AES_256_CBC_SHA1
>>   1590664444.197278:qcrypto_tls_cipher_suite_info data=[0xc0,0x2b] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_128_GCM_SHA256
>>   1590664444.197283:qcrypto_tls_cipher_suite_info data=[0xc0,0xac] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_128_CCM
>>   1590664444.197287:qcrypto_tls_cipher_suite_info data=[0xc0,0x09] version=TLS1.0 name=TLS_ECDHE_ECDSA_AES_128_CBC_SHA1
>>   1590664444.197291:qcrypto_tls_cipher_suite_info data=[0x00,0x9d] version=TLS1.2 name=TLS_RSA_AES_256_GCM_SHA384
>>   1590664444.197296:qcrypto_tls_cipher_suite_info data=[0xc0,0x9d] version=TLS1.2 name=TLS_RSA_AES_256_CCM
>>   1590664444.197300:qcrypto_tls_cipher_suite_info data=[0x00,0x35] version=TLS1.0 name=TLS_RSA_AES_256_CBC_SHA1
>>   1590664444.197304:qcrypto_tls_cipher_suite_info data=[0x00,0x9c] version=TLS1.2 name=TLS_RSA_AES_128_GCM_SHA256
>>   1590664444.197308:qcrypto_tls_cipher_suite_info data=[0xc0,0x9c] version=TLS1.2 name=TLS_RSA_AES_128_CCM
>>   1590664444.197312:qcrypto_tls_cipher_suite_info data=[0x00,0x2f] version=TLS1.0 name=TLS_RSA_AES_128_CBC_SHA1
>>   1590664444.197316:qcrypto_tls_cipher_suite_info data=[0x00,0x9f] version=TLS1.2 name=TLS_DHE_RSA_AES_256_GCM_SHA384
>>   1590664444.197320:qcrypto_tls_cipher_suite_info data=[0xcc,0xaa] version=TLS1.2 name=TLS_DHE_RSA_CHACHA20_POLY1305
>>   1590664444.197325:qcrypto_tls_cipher_suite_info data=[0xc0,0x9f] version=TLS1.2 name=TLS_DHE_RSA_AES_256_CCM
>>   1590664444.197329:qcrypto_tls_cipher_suite_info data=[0x00,0x39] version=TLS1.0 name=TLS_DHE_RSA_AES_256_CBC_SHA1
>>   1590664444.197333:qcrypto_tls_cipher_suite_info data=[0x00,0x9e] version=TLS1.2 name=TLS_DHE_RSA_AES_128_GCM_SHA256
>>   1590664444.197337:qcrypto_tls_cipher_suite_info data=[0xc0,0x9e] version=TLS1.2 name=TLS_DHE_RSA_AES_128_CCM
>>   1590664444.197341:qcrypto_tls_cipher_suite_info data=[0x00,0x33] version=TLS1.0 name=TLS_DHE_RSA_AES_128_CBC_SHA1
>>   1590664444.197345:qcrypto_tls_cipher_suite_count count: 29
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> Acked-by: Laszlo Ersek <lersek@redhat.com>
>> Message-Id: <20200623172726.21040-6-philmd@redhat.com>
> 
> I noticed only now that this breaks '--object help' in
> qemu-storage-daemon:
> 
> $ qemu-storage-daemon --object help
> List of user creatable objects:
> qemu-storage-daemon: missing interface 'fw_cfg-data-generator' for object 'tls-creds'
> Aborted (core dumped)
> 
> The reason is that we don't (and can't) link hw/nvram/fw_cfg.c into the
> storage daemon because it requires other system emulator stuff.

Ouch. I've been completely oblivious to "--object help" and how it
affects qemu-storage-daemon. Sorry about that.

Could you please include a backtrace about the abort()?

Grepping for the error message, I can find type_initialize() in
"qom/object.c", but my knowledge about QOM internals is practically nil.

The error message seems bogus FWIW -- why would
TYPE_FW_CFG_DATA_GENERATOR_INTERFACE be *required* from "tls-creds"?

TYPE_FW_CFG_DATA_GENERATOR_INTERFACE is implemented by
"tls-cipher-suites", and required by "-fw_cfg name=...,gen_id=...". If
that -fw_cfg switch is not used, then why would anything look for the
TYPE_FW_CFG_DATA_GENERATOR_INTERFACE interface? Especially under the
tls-creds object?

Thanks,
Laszlo



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL 5/5] crypto/tls-cipher-suites: Produce fw_cfg consumable blob
  2020-10-01  7:18     ` Laszlo Ersek
@ 2020-10-05  9:16       ` Philippe Mathieu-Daudé
  2020-10-06  8:41         ` Laszlo Ersek
  0 siblings, 1 reply; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-05  9:16 UTC (permalink / raw)
  To: Laszlo Ersek, Kevin Wolf
  Cc: Paolo Bonzini, Daniel P. Berrangé,
	qemu-devel, qemu-block, Gerd Hoffmann

Hi Laszlo,

On 10/1/20 9:18 AM, Laszlo Ersek wrote:
> On 09/29/20 17:46, Kevin Wolf wrote:
>> Am 04.07.2020 um 18:39 hat Philippe Mathieu-Daudé geschrieben:
>>> Since our format is consumable by the fw_cfg device,
>>> we can implement the FW_CFG_DATA_GENERATOR interface.
>>>
>>> Example of use to dump the cipher suites (if tracing enabled):
>>>
>>>   $ qemu-system-x86_64 -S \
>>>     -object tls-cipher-suites,id=mysuite1,priority=@SYSTEM \
>>>     -fw_cfg name=etc/path/to/ciphers,gen_id=mysuite1 \
>>>     -trace qcrypto\*
>>>   1590664444.197123:qcrypto_tls_cipher_suite_priority priority: @SYSTEM
>>>   1590664444.197219:qcrypto_tls_cipher_suite_info data=[0x13,0x02] version=TLS1.3 name=TLS_AES_256_GCM_SHA384
>>>   1590664444.197228:qcrypto_tls_cipher_suite_info data=[0x13,0x03] version=TLS1.3 name=TLS_CHACHA20_POLY1305_SHA256
>>>   1590664444.197233:qcrypto_tls_cipher_suite_info data=[0x13,0x01] version=TLS1.3 name=TLS_AES_128_GCM_SHA256
>>>   1590664444.197236:qcrypto_tls_cipher_suite_info data=[0x13,0x04] version=TLS1.3 name=TLS_AES_128_CCM_SHA256
>>>   1590664444.197240:qcrypto_tls_cipher_suite_info data=[0xc0,0x30] version=TLS1.2 name=TLS_ECDHE_RSA_AES_256_GCM_SHA384
>>>   1590664444.197245:qcrypto_tls_cipher_suite_info data=[0xcc,0xa8] version=TLS1.2 name=TLS_ECDHE_RSA_CHACHA20_POLY1305
>>>   1590664444.197250:qcrypto_tls_cipher_suite_info data=[0xc0,0x14] version=TLS1.0 name=TLS_ECDHE_RSA_AES_256_CBC_SHA1
>>>   1590664444.197254:qcrypto_tls_cipher_suite_info data=[0xc0,0x2f] version=TLS1.2 name=TLS_ECDHE_RSA_AES_128_GCM_SHA256
>>>   1590664444.197258:qcrypto_tls_cipher_suite_info data=[0xc0,0x13] version=TLS1.0 name=TLS_ECDHE_RSA_AES_128_CBC_SHA1
>>>   1590664444.197261:qcrypto_tls_cipher_suite_info data=[0xc0,0x2c] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_256_GCM_SHA384
>>>   1590664444.197266:qcrypto_tls_cipher_suite_info data=[0xcc,0xa9] version=TLS1.2 name=TLS_ECDHE_ECDSA_CHACHA20_POLY1305
>>>   1590664444.197270:qcrypto_tls_cipher_suite_info data=[0xc0,0xad] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_256_CCM
>>>   1590664444.197274:qcrypto_tls_cipher_suite_info data=[0xc0,0x0a] version=TLS1.0 name=TLS_ECDHE_ECDSA_AES_256_CBC_SHA1
>>>   1590664444.197278:qcrypto_tls_cipher_suite_info data=[0xc0,0x2b] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_128_GCM_SHA256
>>>   1590664444.197283:qcrypto_tls_cipher_suite_info data=[0xc0,0xac] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_128_CCM
>>>   1590664444.197287:qcrypto_tls_cipher_suite_info data=[0xc0,0x09] version=TLS1.0 name=TLS_ECDHE_ECDSA_AES_128_CBC_SHA1
>>>   1590664444.197291:qcrypto_tls_cipher_suite_info data=[0x00,0x9d] version=TLS1.2 name=TLS_RSA_AES_256_GCM_SHA384
>>>   1590664444.197296:qcrypto_tls_cipher_suite_info data=[0xc0,0x9d] version=TLS1.2 name=TLS_RSA_AES_256_CCM
>>>   1590664444.197300:qcrypto_tls_cipher_suite_info data=[0x00,0x35] version=TLS1.0 name=TLS_RSA_AES_256_CBC_SHA1
>>>   1590664444.197304:qcrypto_tls_cipher_suite_info data=[0x00,0x9c] version=TLS1.2 name=TLS_RSA_AES_128_GCM_SHA256
>>>   1590664444.197308:qcrypto_tls_cipher_suite_info data=[0xc0,0x9c] version=TLS1.2 name=TLS_RSA_AES_128_CCM
>>>   1590664444.197312:qcrypto_tls_cipher_suite_info data=[0x00,0x2f] version=TLS1.0 name=TLS_RSA_AES_128_CBC_SHA1
>>>   1590664444.197316:qcrypto_tls_cipher_suite_info data=[0x00,0x9f] version=TLS1.2 name=TLS_DHE_RSA_AES_256_GCM_SHA384
>>>   1590664444.197320:qcrypto_tls_cipher_suite_info data=[0xcc,0xaa] version=TLS1.2 name=TLS_DHE_RSA_CHACHA20_POLY1305
>>>   1590664444.197325:qcrypto_tls_cipher_suite_info data=[0xc0,0x9f] version=TLS1.2 name=TLS_DHE_RSA_AES_256_CCM
>>>   1590664444.197329:qcrypto_tls_cipher_suite_info data=[0x00,0x39] version=TLS1.0 name=TLS_DHE_RSA_AES_256_CBC_SHA1
>>>   1590664444.197333:qcrypto_tls_cipher_suite_info data=[0x00,0x9e] version=TLS1.2 name=TLS_DHE_RSA_AES_128_GCM_SHA256
>>>   1590664444.197337:qcrypto_tls_cipher_suite_info data=[0xc0,0x9e] version=TLS1.2 name=TLS_DHE_RSA_AES_128_CCM
>>>   1590664444.197341:qcrypto_tls_cipher_suite_info data=[0x00,0x33] version=TLS1.0 name=TLS_DHE_RSA_AES_128_CBC_SHA1
>>>   1590664444.197345:qcrypto_tls_cipher_suite_count count: 29
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>> Acked-by: Laszlo Ersek <lersek@redhat.com>
>>> Message-Id: <20200623172726.21040-6-philmd@redhat.com>
>>
>> I noticed only now that this breaks '--object help' in
>> qemu-storage-daemon:
>>
>> $ qemu-storage-daemon --object help
>> List of user creatable objects:
>> qemu-storage-daemon: missing interface 'fw_cfg-data-generator' for object 'tls-creds'
>> Aborted (core dumped)
>>
>> The reason is that we don't (and can't) link hw/nvram/fw_cfg.c into the
>> storage daemon because it requires other system emulator stuff.
> 
> Ouch. I've been completely oblivious to "--object help" and how it
> affects qemu-storage-daemon. Sorry about that.
> 
> Could you please include a backtrace about the abort()?
> 
> Grepping for the error message, I can find type_initialize() in
> "qom/object.c", but my knowledge about QOM internals is practically nil.
> 
> The error message seems bogus FWIW -- why would
> TYPE_FW_CFG_DATA_GENERATOR_INTERFACE be *required* from "tls-creds"?
> 
> TYPE_FW_CFG_DATA_GENERATOR_INTERFACE is implemented by
> "tls-cipher-suites", and required by "-fw_cfg name=...,gen_id=...". If
> that -fw_cfg switch is not used, then why would anything look for the
> TYPE_FW_CFG_DATA_GENERATOR_INTERFACE interface? Especially under the
> tls-creds object?

Sorry for not updating Kevin's post in time (we have been discussing
over IRC).

What happens here is a QOM design flow, first triggered by fw_cfg as
we are now trying to have QEMU split into more components.

QOM interface/object type names are simple strings, so we don't get
any link failure in case of missing dependency (which are resolved at
runtime using strcmp).

"tls-cipher-suites" is a generic crypto object, it happens to implement
the fw_cfg-data-generator interface. The fw_cfg-data-generator interface
is registered as QOM type in hw/nvram/fw_cfg.c which is only built when
SOFTMMU is selected. qemu-storage-daemon doesn't select SOFTMMU.
We don't want to restrict "tls-cipher-suites" to SOFTMMU.

The simplest fix we discuss is to have a single C file to register the
fw_cfg-data-generator interface in QOM, and link with it if any of
CRYPTO / NVRAM kconfig is selected.

I'll send a patch.

Regards,

Phil.



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL 5/5] crypto/tls-cipher-suites: Produce fw_cfg consumable blob
  2020-10-05  9:16       ` Philippe Mathieu-Daudé
@ 2020-10-06  8:41         ` Laszlo Ersek
  2020-10-06  9:26           ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 16+ messages in thread
From: Laszlo Ersek @ 2020-10-06  8:41 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, Kevin Wolf
  Cc: Paolo Bonzini, Daniel P. Berrangé,
	qemu-devel, qemu-block, Gerd Hoffmann

On 10/05/20 11:16, Philippe Mathieu-Daudé wrote:
> Hi Laszlo,
> 
> On 10/1/20 9:18 AM, Laszlo Ersek wrote:
>> On 09/29/20 17:46, Kevin Wolf wrote:
>>> Am 04.07.2020 um 18:39 hat Philippe Mathieu-Daudé geschrieben:
>>>> Since our format is consumable by the fw_cfg device,
>>>> we can implement the FW_CFG_DATA_GENERATOR interface.
>>>>
>>>> Example of use to dump the cipher suites (if tracing enabled):
>>>>
>>>>   $ qemu-system-x86_64 -S \
>>>>     -object tls-cipher-suites,id=mysuite1,priority=@SYSTEM \
>>>>     -fw_cfg name=etc/path/to/ciphers,gen_id=mysuite1 \
>>>>     -trace qcrypto\*
>>>>   1590664444.197123:qcrypto_tls_cipher_suite_priority priority: @SYSTEM
>>>>   1590664444.197219:qcrypto_tls_cipher_suite_info data=[0x13,0x02] version=TLS1.3 name=TLS_AES_256_GCM_SHA384
>>>>   1590664444.197228:qcrypto_tls_cipher_suite_info data=[0x13,0x03] version=TLS1.3 name=TLS_CHACHA20_POLY1305_SHA256
>>>>   1590664444.197233:qcrypto_tls_cipher_suite_info data=[0x13,0x01] version=TLS1.3 name=TLS_AES_128_GCM_SHA256
>>>>   1590664444.197236:qcrypto_tls_cipher_suite_info data=[0x13,0x04] version=TLS1.3 name=TLS_AES_128_CCM_SHA256
>>>>   1590664444.197240:qcrypto_tls_cipher_suite_info data=[0xc0,0x30] version=TLS1.2 name=TLS_ECDHE_RSA_AES_256_GCM_SHA384
>>>>   1590664444.197245:qcrypto_tls_cipher_suite_info data=[0xcc,0xa8] version=TLS1.2 name=TLS_ECDHE_RSA_CHACHA20_POLY1305
>>>>   1590664444.197250:qcrypto_tls_cipher_suite_info data=[0xc0,0x14] version=TLS1.0 name=TLS_ECDHE_RSA_AES_256_CBC_SHA1
>>>>   1590664444.197254:qcrypto_tls_cipher_suite_info data=[0xc0,0x2f] version=TLS1.2 name=TLS_ECDHE_RSA_AES_128_GCM_SHA256
>>>>   1590664444.197258:qcrypto_tls_cipher_suite_info data=[0xc0,0x13] version=TLS1.0 name=TLS_ECDHE_RSA_AES_128_CBC_SHA1
>>>>   1590664444.197261:qcrypto_tls_cipher_suite_info data=[0xc0,0x2c] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_256_GCM_SHA384
>>>>   1590664444.197266:qcrypto_tls_cipher_suite_info data=[0xcc,0xa9] version=TLS1.2 name=TLS_ECDHE_ECDSA_CHACHA20_POLY1305
>>>>   1590664444.197270:qcrypto_tls_cipher_suite_info data=[0xc0,0xad] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_256_CCM
>>>>   1590664444.197274:qcrypto_tls_cipher_suite_info data=[0xc0,0x0a] version=TLS1.0 name=TLS_ECDHE_ECDSA_AES_256_CBC_SHA1
>>>>   1590664444.197278:qcrypto_tls_cipher_suite_info data=[0xc0,0x2b] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_128_GCM_SHA256
>>>>   1590664444.197283:qcrypto_tls_cipher_suite_info data=[0xc0,0xac] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_128_CCM
>>>>   1590664444.197287:qcrypto_tls_cipher_suite_info data=[0xc0,0x09] version=TLS1.0 name=TLS_ECDHE_ECDSA_AES_128_CBC_SHA1
>>>>   1590664444.197291:qcrypto_tls_cipher_suite_info data=[0x00,0x9d] version=TLS1.2 name=TLS_RSA_AES_256_GCM_SHA384
>>>>   1590664444.197296:qcrypto_tls_cipher_suite_info data=[0xc0,0x9d] version=TLS1.2 name=TLS_RSA_AES_256_CCM
>>>>   1590664444.197300:qcrypto_tls_cipher_suite_info data=[0x00,0x35] version=TLS1.0 name=TLS_RSA_AES_256_CBC_SHA1
>>>>   1590664444.197304:qcrypto_tls_cipher_suite_info data=[0x00,0x9c] version=TLS1.2 name=TLS_RSA_AES_128_GCM_SHA256
>>>>   1590664444.197308:qcrypto_tls_cipher_suite_info data=[0xc0,0x9c] version=TLS1.2 name=TLS_RSA_AES_128_CCM
>>>>   1590664444.197312:qcrypto_tls_cipher_suite_info data=[0x00,0x2f] version=TLS1.0 name=TLS_RSA_AES_128_CBC_SHA1
>>>>   1590664444.197316:qcrypto_tls_cipher_suite_info data=[0x00,0x9f] version=TLS1.2 name=TLS_DHE_RSA_AES_256_GCM_SHA384
>>>>   1590664444.197320:qcrypto_tls_cipher_suite_info data=[0xcc,0xaa] version=TLS1.2 name=TLS_DHE_RSA_CHACHA20_POLY1305
>>>>   1590664444.197325:qcrypto_tls_cipher_suite_info data=[0xc0,0x9f] version=TLS1.2 name=TLS_DHE_RSA_AES_256_CCM
>>>>   1590664444.197329:qcrypto_tls_cipher_suite_info data=[0x00,0x39] version=TLS1.0 name=TLS_DHE_RSA_AES_256_CBC_SHA1
>>>>   1590664444.197333:qcrypto_tls_cipher_suite_info data=[0x00,0x9e] version=TLS1.2 name=TLS_DHE_RSA_AES_128_GCM_SHA256
>>>>   1590664444.197337:qcrypto_tls_cipher_suite_info data=[0xc0,0x9e] version=TLS1.2 name=TLS_DHE_RSA_AES_128_CCM
>>>>   1590664444.197341:qcrypto_tls_cipher_suite_info data=[0x00,0x33] version=TLS1.0 name=TLS_DHE_RSA_AES_128_CBC_SHA1
>>>>   1590664444.197345:qcrypto_tls_cipher_suite_count count: 29
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>>> Acked-by: Laszlo Ersek <lersek@redhat.com>
>>>> Message-Id: <20200623172726.21040-6-philmd@redhat.com>
>>>
>>> I noticed only now that this breaks '--object help' in
>>> qemu-storage-daemon:
>>>
>>> $ qemu-storage-daemon --object help
>>> List of user creatable objects:
>>> qemu-storage-daemon: missing interface 'fw_cfg-data-generator' for object 'tls-creds'
>>> Aborted (core dumped)
>>>
>>> The reason is that we don't (and can't) link hw/nvram/fw_cfg.c into the
>>> storage daemon because it requires other system emulator stuff.
>>
>> Ouch. I've been completely oblivious to "--object help" and how it
>> affects qemu-storage-daemon. Sorry about that.
>>
>> Could you please include a backtrace about the abort()?
>>
>> Grepping for the error message, I can find type_initialize() in
>> "qom/object.c", but my knowledge about QOM internals is practically nil.
>>
>> The error message seems bogus FWIW -- why would
>> TYPE_FW_CFG_DATA_GENERATOR_INTERFACE be *required* from "tls-creds"?
>>
>> TYPE_FW_CFG_DATA_GENERATOR_INTERFACE is implemented by
>> "tls-cipher-suites", and required by "-fw_cfg name=...,gen_id=...". If
>> that -fw_cfg switch is not used, then why would anything look for the
>> TYPE_FW_CFG_DATA_GENERATOR_INTERFACE interface? Especially under the
>> tls-creds object?
> 
> Sorry for not updating Kevin's post in time (we have been discussing
> over IRC).
> 
> What happens here is a QOM design flow, first triggered by fw_cfg as
> we are now trying to have QEMU split into more components.
> 
> QOM interface/object type names are simple strings, so we don't get
> any link failure in case of missing dependency (which are resolved at
> runtime using strcmp).
> 
> "tls-cipher-suites" is a generic crypto object, it happens to implement
> the fw_cfg-data-generator interface. The fw_cfg-data-generator interface
> is registered as QOM type in hw/nvram/fw_cfg.c which is only built when
> SOFTMMU is selected. qemu-storage-daemon doesn't select SOFTMMU.
> We don't want to restrict "tls-cipher-suites" to SOFTMMU.
> 
> The simplest fix we discuss is to have a single C file to register the
> fw_cfg-data-generator interface in QOM, and link with it if any of
> CRYPTO / NVRAM kconfig is selected.
> 
> I'll send a patch.

Thank you for the explanation!

(I suggest keeping such discussions *originally* on the list. IRC is
practically indistinguishable from the bit bucket, and you'll have to
write up a summary for others on the mailing list anyway. (In some cases
it could even require filing a ticket in some tracker, in the end.) Best
to have the discussion at once on the list. Just my suggestion of course.)

Thanks!
Laszlo



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PULL 5/5] crypto/tls-cipher-suites: Produce fw_cfg consumable blob
  2020-10-06  8:41         ` Laszlo Ersek
@ 2020-10-06  9:26           ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 16+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-06  9:26 UTC (permalink / raw)
  To: Laszlo Ersek, Kevin Wolf
  Cc: Paolo Bonzini, Daniel P. Berrangé,
	qemu-devel, qemu-block, Gerd Hoffmann

On 10/6/20 10:41 AM, Laszlo Ersek wrote:
> On 10/05/20 11:16, Philippe Mathieu-Daudé wrote:
>> Hi Laszlo,
>>
>> On 10/1/20 9:18 AM, Laszlo Ersek wrote:
>>> On 09/29/20 17:46, Kevin Wolf wrote:
>>>> Am 04.07.2020 um 18:39 hat Philippe Mathieu-Daudé geschrieben:
>>>>> Since our format is consumable by the fw_cfg device,
>>>>> we can implement the FW_CFG_DATA_GENERATOR interface.
>>>>>
>>>>> Example of use to dump the cipher suites (if tracing enabled):
>>>>>
>>>>>   $ qemu-system-x86_64 -S \
>>>>>     -object tls-cipher-suites,id=mysuite1,priority=@SYSTEM \
>>>>>     -fw_cfg name=etc/path/to/ciphers,gen_id=mysuite1 \
>>>>>     -trace qcrypto\*
>>>>>   1590664444.197123:qcrypto_tls_cipher_suite_priority priority: @SYSTEM
>>>>>   1590664444.197219:qcrypto_tls_cipher_suite_info data=[0x13,0x02] version=TLS1.3 name=TLS_AES_256_GCM_SHA384
>>>>>   1590664444.197228:qcrypto_tls_cipher_suite_info data=[0x13,0x03] version=TLS1.3 name=TLS_CHACHA20_POLY1305_SHA256
>>>>>   1590664444.197233:qcrypto_tls_cipher_suite_info data=[0x13,0x01] version=TLS1.3 name=TLS_AES_128_GCM_SHA256
>>>>>   1590664444.197236:qcrypto_tls_cipher_suite_info data=[0x13,0x04] version=TLS1.3 name=TLS_AES_128_CCM_SHA256
>>>>>   1590664444.197240:qcrypto_tls_cipher_suite_info data=[0xc0,0x30] version=TLS1.2 name=TLS_ECDHE_RSA_AES_256_GCM_SHA384
>>>>>   1590664444.197245:qcrypto_tls_cipher_suite_info data=[0xcc,0xa8] version=TLS1.2 name=TLS_ECDHE_RSA_CHACHA20_POLY1305
>>>>>   1590664444.197250:qcrypto_tls_cipher_suite_info data=[0xc0,0x14] version=TLS1.0 name=TLS_ECDHE_RSA_AES_256_CBC_SHA1
>>>>>   1590664444.197254:qcrypto_tls_cipher_suite_info data=[0xc0,0x2f] version=TLS1.2 name=TLS_ECDHE_RSA_AES_128_GCM_SHA256
>>>>>   1590664444.197258:qcrypto_tls_cipher_suite_info data=[0xc0,0x13] version=TLS1.0 name=TLS_ECDHE_RSA_AES_128_CBC_SHA1
>>>>>   1590664444.197261:qcrypto_tls_cipher_suite_info data=[0xc0,0x2c] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_256_GCM_SHA384
>>>>>   1590664444.197266:qcrypto_tls_cipher_suite_info data=[0xcc,0xa9] version=TLS1.2 name=TLS_ECDHE_ECDSA_CHACHA20_POLY1305
>>>>>   1590664444.197270:qcrypto_tls_cipher_suite_info data=[0xc0,0xad] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_256_CCM
>>>>>   1590664444.197274:qcrypto_tls_cipher_suite_info data=[0xc0,0x0a] version=TLS1.0 name=TLS_ECDHE_ECDSA_AES_256_CBC_SHA1
>>>>>   1590664444.197278:qcrypto_tls_cipher_suite_info data=[0xc0,0x2b] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_128_GCM_SHA256
>>>>>   1590664444.197283:qcrypto_tls_cipher_suite_info data=[0xc0,0xac] version=TLS1.2 name=TLS_ECDHE_ECDSA_AES_128_CCM
>>>>>   1590664444.197287:qcrypto_tls_cipher_suite_info data=[0xc0,0x09] version=TLS1.0 name=TLS_ECDHE_ECDSA_AES_128_CBC_SHA1
>>>>>   1590664444.197291:qcrypto_tls_cipher_suite_info data=[0x00,0x9d] version=TLS1.2 name=TLS_RSA_AES_256_GCM_SHA384
>>>>>   1590664444.197296:qcrypto_tls_cipher_suite_info data=[0xc0,0x9d] version=TLS1.2 name=TLS_RSA_AES_256_CCM
>>>>>   1590664444.197300:qcrypto_tls_cipher_suite_info data=[0x00,0x35] version=TLS1.0 name=TLS_RSA_AES_256_CBC_SHA1
>>>>>   1590664444.197304:qcrypto_tls_cipher_suite_info data=[0x00,0x9c] version=TLS1.2 name=TLS_RSA_AES_128_GCM_SHA256
>>>>>   1590664444.197308:qcrypto_tls_cipher_suite_info data=[0xc0,0x9c] version=TLS1.2 name=TLS_RSA_AES_128_CCM
>>>>>   1590664444.197312:qcrypto_tls_cipher_suite_info data=[0x00,0x2f] version=TLS1.0 name=TLS_RSA_AES_128_CBC_SHA1
>>>>>   1590664444.197316:qcrypto_tls_cipher_suite_info data=[0x00,0x9f] version=TLS1.2 name=TLS_DHE_RSA_AES_256_GCM_SHA384
>>>>>   1590664444.197320:qcrypto_tls_cipher_suite_info data=[0xcc,0xaa] version=TLS1.2 name=TLS_DHE_RSA_CHACHA20_POLY1305
>>>>>   1590664444.197325:qcrypto_tls_cipher_suite_info data=[0xc0,0x9f] version=TLS1.2 name=TLS_DHE_RSA_AES_256_CCM
>>>>>   1590664444.197329:qcrypto_tls_cipher_suite_info data=[0x00,0x39] version=TLS1.0 name=TLS_DHE_RSA_AES_256_CBC_SHA1
>>>>>   1590664444.197333:qcrypto_tls_cipher_suite_info data=[0x00,0x9e] version=TLS1.2 name=TLS_DHE_RSA_AES_128_GCM_SHA256
>>>>>   1590664444.197337:qcrypto_tls_cipher_suite_info data=[0xc0,0x9e] version=TLS1.2 name=TLS_DHE_RSA_AES_128_CCM
>>>>>   1590664444.197341:qcrypto_tls_cipher_suite_info data=[0x00,0x33] version=TLS1.0 name=TLS_DHE_RSA_AES_128_CBC_SHA1
>>>>>   1590664444.197345:qcrypto_tls_cipher_suite_count count: 29
>>>>>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>>>>> Acked-by: Laszlo Ersek <lersek@redhat.com>
>>>>> Message-Id: <20200623172726.21040-6-philmd@redhat.com>
>>>>
>>>> I noticed only now that this breaks '--object help' in
>>>> qemu-storage-daemon:
>>>>
>>>> $ qemu-storage-daemon --object help
>>>> List of user creatable objects:
>>>> qemu-storage-daemon: missing interface 'fw_cfg-data-generator' for object 'tls-creds'
>>>> Aborted (core dumped)
>>>>
>>>> The reason is that we don't (and can't) link hw/nvram/fw_cfg.c into the
>>>> storage daemon because it requires other system emulator stuff.
>>>
>>> Ouch. I've been completely oblivious to "--object help" and how it
>>> affects qemu-storage-daemon. Sorry about that.
>>>
>>> Could you please include a backtrace about the abort()?
>>>
>>> Grepping for the error message, I can find type_initialize() in
>>> "qom/object.c", but my knowledge about QOM internals is practically nil.
>>>
>>> The error message seems bogus FWIW -- why would
>>> TYPE_FW_CFG_DATA_GENERATOR_INTERFACE be *required* from "tls-creds"?
>>>
>>> TYPE_FW_CFG_DATA_GENERATOR_INTERFACE is implemented by
>>> "tls-cipher-suites", and required by "-fw_cfg name=...,gen_id=...". If
>>> that -fw_cfg switch is not used, then why would anything look for the
>>> TYPE_FW_CFG_DATA_GENERATOR_INTERFACE interface? Especially under the
>>> tls-creds object?
>>
>> Sorry for not updating Kevin's post in time (we have been discussing
>> over IRC).
>>
>> What happens here is a QOM design flow, first triggered by fw_cfg as
>> we are now trying to have QEMU split into more components.
>>
>> QOM interface/object type names are simple strings, so we don't get
>> any link failure in case of missing dependency (which are resolved at
>> runtime using strcmp).
>>
>> "tls-cipher-suites" is a generic crypto object, it happens to implement
>> the fw_cfg-data-generator interface. The fw_cfg-data-generator interface
>> is registered as QOM type in hw/nvram/fw_cfg.c which is only built when
>> SOFTMMU is selected. qemu-storage-daemon doesn't select SOFTMMU.
>> We don't want to restrict "tls-cipher-suites" to SOFTMMU.
>>
>> The simplest fix we discuss is to have a single C file to register the
>> fw_cfg-data-generator interface in QOM, and link with it if any of
>> CRYPTO / NVRAM kconfig is selected.
>>
>> I'll send a patch.
> 
> Thank you for the explanation!
> 
> (I suggest keeping such discussions *originally* on the list. IRC is
> practically indistinguishable from the bit bucket, and you'll have to
> write up a summary for others on the mailing list anyway. (In some cases
> it could even require filing a ticket in some tracker, in the end.) Best
> to have the discussion at once on the list. Just my suggestion of course.)

There are so many discussions there... But you are right,
noted for the areas I'm co-maintaining, I'll summarize on
the list.

Regards,

Phil.



^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2020-10-06  9:27 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-04 16:39 [PULL 0/5] fw_cfg/crypto patches for 5.1 soft freeze Philippe Mathieu-Daudé
2020-07-04 16:39 ` [PULL 1/5] crypto: Add tls-cipher-suites object Philippe Mathieu-Daudé
2020-07-04 16:39 ` [PULL 2/5] hw/nvram/fw_cfg: Add the FW_CFG_DATA_GENERATOR interface Philippe Mathieu-Daudé
2020-07-04 16:39 ` [PULL 3/5] softmmu/vl: Let -fw_cfg option take a 'gen_id' argument Philippe Mathieu-Daudé
2020-07-13 13:13   ` Peter Maydell
2020-07-13 14:50     ` Laszlo Ersek
2020-07-13 14:55       ` Peter Maydell
2020-07-14 13:04       ` Philippe Mathieu-Daudé
2020-07-04 16:39 ` [PULL 4/5] softmmu/vl: Allow -fw_cfg 'gen_id' option to use the 'etc/' namespace Philippe Mathieu-Daudé
2020-07-04 16:39 ` [PULL 5/5] crypto/tls-cipher-suites: Produce fw_cfg consumable blob Philippe Mathieu-Daudé
2020-09-29 15:46   ` Kevin Wolf
2020-10-01  7:18     ` Laszlo Ersek
2020-10-05  9:16       ` Philippe Mathieu-Daudé
2020-10-06  8:41         ` Laszlo Ersek
2020-10-06  9:26           ` Philippe Mathieu-Daudé
2020-07-10  8:01 ` [PULL 0/5] fw_cfg/crypto patches for 5.1 soft freeze Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.