[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 11/31] qapi/qom: Add ObjectOptions for tls-*, deprecate 'l
From: |
Kevin Wolf |
Subject: |
Re: [PATCH v2 11/31] qapi/qom: Add ObjectOptions for tls-*, deprecate 'loaded' |
Date: |
Tue, 2 Mar 2021 19:27:03 +0100 |
Am 26.02.2021 um 20:33 hat Eric Blake geschrieben:
> On 2/24/21 7:52 AM, Kevin Wolf wrote:
> > This adds a QAPI schema for the properties of the tls-* objects.
> >
> > The 'loaded' property doesn't seem to make sense as an external
> > interface: It is automatically set to true in ucc->complete, and
> > explicitly setting it to true earlier just means that additional options
> > will be silently ignored.
> >
> > In other words, the 'loaded' property is useless. Mark it as deprecated
> > in the schema from the start.
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> > qapi/crypto.json | 98 ++++++++++++++++++++++++++++++++++++++++++++++++
> > qapi/qom.json | 12 +++++-
> > 2 files changed, 108 insertions(+), 2 deletions(-)
> >
> > diff --git a/qapi/crypto.json b/qapi/crypto.json
> > index 0fef3de66d..7116ae9a46 100644
> > --- a/qapi/crypto.json
> > +++ b/qapi/crypto.json
> > @@ -442,3 +442,101 @@
> > { 'struct': 'SecretKeyringProperties',
> > 'base': 'SecretCommonProperties',
> > 'data': { 'serial': 'int32' } }
> > +
> > +##
> > +# @TlsCredsProperties:
> > +#
> > +# Properties for objects of classes derived from tls-creds.
> > +#
> > +# @verify-peer: if true the peer credentials will be verified once the
> > +# handshake is completed. This is a no-op for anonymous
> > +# credentials. (default: true)
> > +#
> > +# @dir: the path of the directory that contains the credential files
> > +#
> > +# @endpoint: whether the QEMU network backend that uses the credentials
> > will be
> > +# acting as a client or as a server (default: client)
> > +#
> > +# @priority: a gnutls priority string as described at
> > +# https://gnutls.org/manual/html_node/Priority-Strings.html
> > +#
> > +# Since: 2.5
> > +##
> > +{ 'struct': 'TlsCredsProperties',
> > + 'data': { '*verify-peer': 'bool',
> > + '*dir': 'str',
> > + '*endpoint': 'QCryptoTLSCredsEndpoint',
> > + '*priority': 'str' } }
>
> Matches crypto/tlscreds.c:qcrypto_tls_creds_class_init().
>
> > +
> > +##
> > +# @TlsCredsAnonProperties:
> > +#
> > +# Properties for tls-creds-anon objects.
> > +#
> > +# @loaded: if true, the credentials are loaded immediately when applying
> > this
> > +# option and will ignore options that are processed later. Don't
> > use;
> > +# only provided for compatibility. (default: false)
> > +#
> > +# Features:
> > +# @deprecated: Member @loaded is deprecated. Setting true doesn't make
> > sense,
> > +# and false is already the default.
> > +#
> > +# Since: 2.5
> > +##
> > +{ 'struct': 'TlsCredsAnonProperties',
> > + 'base': 'TlsCredsProperties',
> > + 'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] } } }
>
> Since we documented that 'verify-peer' is a no-op for this struct, is it
> worth altering our type hierarchy to make it explicit, as in:
>
> TlsCredsCommonProperties - dir, endpoint, priority
> TlsCredsProperties - TlsCredsCommonProperties + verify-peer
> TlsCredsAnonProperties - TlsCredsCommonProperties + loaded
> TlsCredsPskProperties - TlsCredsProperties + loaded, username
>
> But even if not, this matches
> crypto/tlscredsanon.c:qcrypto_tls_creds_anon_class_init().
We can't turn a no-op into an error without a deprecation period.
> > +
> > +##
> > +# @TlsCredsPskProperties:
> > +#
> > +# Properties for tls-creds-psk objects.
> > +#
> > +# @loaded: if true, the credentials are loaded immediately when applying
> > this
> > +# option and will ignore options that are processed later. Don't
> > use;
> > +# only provided for compatibility. (default: false)
> > +#
> > +# @username: the username which will be sent to the server. For clients
> > only.
> > +# If absent, "qemu" is sent and the property will read back as
> > an
> > +# empty string.
> > +#
> > +# Features:
> > +# @deprecated: Member @loaded is deprecated. Setting true doesn't make
> > sense,
> > +# and false is already the default.
> > +#
> > +# Since: 3.0
> > +##
> > +{ 'struct': 'TlsCredsPskProperties',
> > + 'base': 'TlsCredsProperties',
> > + 'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] },
> > + '*username': 'str' } }
>
> This matches crypto/tlscredspsk.c:qcrypto_tls_creds_psk_class_init().
>
> Do we want to use QAPI type inheritance to declare a union where
> 'endpoint' is the union discriminator, and 'username' is only present
> for 'endpoint':'client'? (Hmm, we'd have to improve the QAPI code
> generator to allow a flat union as the branch of yet another flat
> union...)
Probably not now then.
It also has the same problem as above, but I guess you could use the
deprecation period to build the required QAPI infrastructure. :-)
Kevin
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH v2 11/31] qapi/qom: Add ObjectOptions for tls-*, deprecate 'loaded',
Kevin Wolf <=