Re: [Qemu-devel] [RFC][BROKEN] rbd: Allow configuration of authentication scheme

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4320 bytes --]

Am 18.04.2018 um 15:26 hat Eric Blake geschrieben:
> On 04/05/2018 12:06 PM, Kevin Wolf wrote:
> > The legacy command line syntax supports a "password-secret" option that
> > allows to pass an authentication key to Ceph. This was not supported in
> > QMP so far.
> > 
> > This patch introduces authentication options in the QAPI schema, makes
> > them do the corresponding rados_conf_set() calls and adds compatibility
> > code that translates the old "password-secret" option both for opening
> > and creating images to the new set of options.
> > 
> > Note that the old option didn't allow to explicitly specify the set of
> > allowed authentication schemes. The compatibility code assumes that if
> > "password-secret" is given, only the cephx scheme is allowed. If it's
> > missing, both none and cephx are allowed because the configuration file
> > could still provide a key.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> 
> > Any thoughts on the proposed QAPI schema or the two implementation
> > problems are welcome.
> > 
> 
> >  qapi/block-core.json |  22 +++++++++++
> >  block/rbd.c          | 102 ++++++++++++++++++++++++++++++++++++++-------------
> >  2 files changed, 99 insertions(+), 25 deletions(-)
> > 
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index c50517bff3..d5ce588add 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -3170,6 +3170,19 @@
> >  
> >  
> >  ##
> > +# @RbdAuthCephx:
> > +#
> > +# @key-secret:         ID of a QCryptoSecret object providing a key for cephx
> > +#                      authentication. If not specified, a key from the
> > +#                      specified configuration file, or the system default
> > +#                      configuration is used, if present.
> > +#
> > +# Since: 2.13
> > +##
> > +{ 'struct': 'RbdAuthCephx',
> > +  'data': { '*key-secret': 'str' } }
> > +
> > +##
> >  # @BlockdevOptionsRbd:
> >  #
> >  # @pool:               Ceph pool name.
> > @@ -3184,6 +3197,13 @@
> >  #
> >  # @user:               Ceph id name.
> >  #
> > +# @auth-none:          true if connecting to a server without authentication
> > +#                      should be allowed (default: false; since 2.13)
> > +#
> > +# @auth-cephx:         Configuration for cephx authentication if specified. If
> > +#                      not specified, cephx authentication is not allowed.
> > +#                      (since 2.13)
> > +#
> >  # @server:             Monitor host address and port.  This maps
> >  #                      to the "mon_host" Ceph option.
> >  #
> > @@ -3195,6 +3215,8 @@
> >              '*conf': 'str',
> >              '*snapshot': 'str',
> >              '*user': 'str',
> > +            '*auth-none': 'bool',
> > +            '*auth-cephx': 'RbdAuthCephx',
> >              '*server': ['InetSocketAddressBase'] } }
> 
> Would it be better to have this be a flat union with 'auth' with enum
> values 'none', 'cephx', 'both' as a discriminator that determines which
> additional fields can be present?  Or does that require that we first
> fix the QAPI generator to allow nesting a flat union within another flat
> union (probably doable, just no one has needed it before now)?  Is it
> also time to improve the QAPI generator to allow a default value to the
> discriminator field, rather than requiring the field to be present?

Both options can be enabled at the same time, so that the client
connects to a server no matter whether it does 'cephx' authentication or
only 'none. This is even the default for rbd driver (in the existing
command line interface, but I think we need to stay compatible with it).
With a union you would have to explicitly choose one or the other, but
could never accept both.

The other option we were considering was a list of authentication
options, which would be easier to implement, but isn't really an
accurate representation of what we really accept. There is no way we
could meaningfully implement something like this:

    'auth': [ { 'type': 'cephx', 'key-secret': 'foo' },
              { 'type': 'cephx', 'key-secret': 'bar' } ]

Because Ceph only allows us to enable the 'cephx' authentication method
and to set a single key for it.

Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]