| Message ID | 20210224135255.253837-12-kwolf@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | qapi/qom: QAPIfy --object and object-add | expand |
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(). > + > +## > +# @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...) > + > +## > +# @TlsCredsX509Properties: > +# > +# Properties for tls-creds-x509 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) > +# > +# @sanity-check: if true, perform some sanity checks before using the > +# credentials (default: true) > +# > +# @passwordid: For the server-key.pem and client-key.pem files which contain > +# sensitive private keys, it is possible to use an encrypted > +# version by providing the @passwordid parameter. This provides > +# the ID of a previously created secret object containing the > +# password for decryption. > +# > +# Features: > +# @deprecated: Member @loaded is deprecated. Setting true doesn't make sense, > +# and false is already the default. > +# > +# Since: 2.5 > +## > +{ 'struct': 'TlsCredsX509Properties', > + 'base': 'TlsCredsProperties', > + 'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] }, > + '*sanity-check': 'bool', > + '*passwordid': 'str' } } This matches crypto/tlscredsx509.c:qcrypto_tls_creds_x509_class_init(). > diff --git a/qapi/qom.json b/qapi/qom.json > index 2668ad8369..f22b7aa99b 100644 > --- a/qapi/qom.json > +++ b/qapi/qom.json > @@ -452,7 +452,11 @@ > 'rng-random', > 'secret', > 'secret_keyring', > - 'throttle-group' > + 'throttle-group', > + 'tls-creds-anon', > + 'tls-creds-psk', > + 'tls-creds-x509', > + 'tls-cipher-suites' Matches crypto/tls-cipher-suites.c:qcrypto_tls_cipher_suites_class_init(). > ] } > > ## > @@ -488,7 +492,11 @@ > 'rng-random': 'RngRandomProperties', > 'secret': 'SecretProperties', > 'secret_keyring': 'SecretKeyringProperties', > - 'throttle-group': 'ThrottleGroupProperties' > + 'throttle-group': 'ThrottleGroupProperties', > + 'tls-creds-anon': 'TlsCredsAnonProperties', > + 'tls-creds-psk': 'TlsCredsPskProperties', > + 'tls-creds-x509': 'TlsCredsX509Properties', > + 'tls-cipher-suites': 'TlsCredsProperties' Reviewed-by: Eric Blake <eblake@redhat.com>
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
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' } } + +## +# @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'] } } } + +## +# @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' } } + +## +# @TlsCredsX509Properties: +# +# Properties for tls-creds-x509 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) +# +# @sanity-check: if true, perform some sanity checks before using the +# credentials (default: true) +# +# @passwordid: For the server-key.pem and client-key.pem files which contain +# sensitive private keys, it is possible to use an encrypted +# version by providing the @passwordid parameter. This provides +# the ID of a previously created secret object containing the +# password for decryption. +# +# Features: +# @deprecated: Member @loaded is deprecated. Setting true doesn't make sense, +# and false is already the default. +# +# Since: 2.5 +## +{ 'struct': 'TlsCredsX509Properties', + 'base': 'TlsCredsProperties', + 'data': { '*loaded': { 'type': 'bool', 'features': ['deprecated'] }, + '*sanity-check': 'bool', + '*passwordid': 'str' } } diff --git a/qapi/qom.json b/qapi/qom.json index 2668ad8369..f22b7aa99b 100644 --- a/qapi/qom.json +++ b/qapi/qom.json @@ -452,7 +452,11 @@ 'rng-random', 'secret', 'secret_keyring', - 'throttle-group' + 'throttle-group', + 'tls-creds-anon', + 'tls-creds-psk', + 'tls-creds-x509', + 'tls-cipher-suites' ] } ## @@ -488,7 +492,11 @@ 'rng-random': 'RngRandomProperties', 'secret': 'SecretProperties', 'secret_keyring': 'SecretKeyringProperties', - 'throttle-group': 'ThrottleGroupProperties' + 'throttle-group': 'ThrottleGroupProperties', + 'tls-creds-anon': 'TlsCredsAnonProperties', + 'tls-creds-psk': 'TlsCredsPskProperties', + 'tls-creds-x509': 'TlsCredsX509Properties', + 'tls-cipher-suites': 'TlsCredsProperties' } } ##
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(-)