[1/2] qcow2: fail if encryption opts are provided to non-encrypted image
diff mbox series

Message ID 20190219125044.5416-2-berrange@redhat.com
State New
Headers show
Series
  • qcow2: improve error handling when luks creation fails
Related show

Commit Message

Daniel P. Berrangé Feb. 19, 2019, 12:50 p.m. UTC
If the qcow2 image does not have any encryption method specified in its
header, the user should not be providing any encryption options when
opening it. We already detect this if the user had set "encrypt.format"
but this field is optional so must consider any "encrypt.*" option to be
an error.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 block/qcow2.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Eric Blake Feb. 19, 2019, 3:56 p.m. UTC | #1
On 2/19/19 6:50 AM, Daniel P. Berrangé wrote:
> If the qcow2 image does not have any encryption method specified in its
> header, the user should not be providing any encryption options when
> opening it. We already detect this if the user had set "encrypt.format"
> but this field is optional so must consider any "encrypt.*" option to be
> an error.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  block/qcow2.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
Max Reitz Feb. 22, 2019, 7:17 p.m. UTC | #2
On 19.02.19 13:50, Daniel P. Berrangé wrote:
> If the qcow2 image does not have any encryption method specified in its
> header, the user should not be providing any encryption options when
> opening it. We already detect this if the user had set "encrypt.format"
> but this field is optional so must consider any "encrypt.*" option to be
> an error.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  block/qcow2.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 65a54c9ac6..ecc577175f 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -1045,6 +1045,12 @@ static int qcow2_update_options_prepare(BlockDriverState *bs,
>              ret = -EINVAL;
>              goto fail;
>          }
> +        if (encryptopts && qdict_size(encryptopts)) {
> +            error_setg(errp, "No encryption in image header, but encryption "
> +                       "options provided");
> +            ret = -EINVAL;
> +            goto fail;
> +        }

Doesn't this make the block right before this one a bit superfluous?

Max

>          break;
>  
>      case QCOW_CRYPT_AES:
>
Daniel P. Berrangé Feb. 25, 2019, 10:36 a.m. UTC | #3
On Fri, Feb 22, 2019 at 08:17:40PM +0100, Max Reitz wrote:
> On 19.02.19 13:50, Daniel P. Berrangé wrote:
> > If the qcow2 image does not have any encryption method specified in its
> > header, the user should not be providing any encryption options when
> > opening it. We already detect this if the user had set "encrypt.format"
> > but this field is optional so must consider any "encrypt.*" option to be
> > an error.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  block/qcow2.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/block/qcow2.c b/block/qcow2.c
> > index 65a54c9ac6..ecc577175f 100644
> > --- a/block/qcow2.c
> > +++ b/block/qcow2.c
> > @@ -1045,6 +1045,12 @@ static int qcow2_update_options_prepare(BlockDriverState *bs,
> >              ret = -EINVAL;
> >              goto fail;
> >          }
> > +        if (encryptopts && qdict_size(encryptopts)) {
> > +            error_setg(errp, "No encryption in image header, but encryption "
> > +                       "options provided");
> > +            ret = -EINVAL;
> > +            goto fail;
> > +        }
> 
> Doesn't this make the block right before this one a bit superfluous?

Yes, in the sense that we'll still get an error if we removed the
prior block. The prior block has a more useful error message which
will help diagnosis though, so I thought it worth keeping both.

Regards,
Daniel

Patch
diff mbox series

diff --git a/block/qcow2.c b/block/qcow2.c
index 65a54c9ac6..ecc577175f 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1045,6 +1045,12 @@  static int qcow2_update_options_prepare(BlockDriverState *bs,
             ret = -EINVAL;
             goto fail;
         }
+        if (encryptopts && qdict_size(encryptopts)) {
+            error_setg(errp, "No encryption in image header, but encryption "
+                       "options provided");
+            ret = -EINVAL;
+            goto fail;
+        }
         break;
 
     case QCOW_CRYPT_AES: