This is to silent:

  $ qemu-system-x86_64 \
    -object tls-cipher-suites,id=ciphersuite0,priority=@SYSTEM \
    -fw_cfg name=etc/edk2/https/ciphers,blob_id=ciphersuite0
  qemu-system-x86_64: -fw_cfg name=etc/edk2/https/ciphers,blob_id=ciphersuite0: warning: externally provided fw_cfg item names should be prefixed with "opt/"

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 softmmu/vl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/softmmu/vl.c b/softmmu/vl.c
index f76c53ad2e..3b77dcc00d 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2052,7 +2052,7 @@ 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(blob_id) && strncmp(name, "opt/", 4) != 0) {
         warn_report("externally provided fw_cfg item names "
                     "should be prefixed with \"opt/\"");
     }
-- 
2.21.3

On 5/19/20 8:20 PM, Philippe Mathieu-Daudé wrote:
> This is to silent:
> 
>    $ qemu-system-x86_64 \
>      -object tls-cipher-suites,id=ciphersuite0,priority=@SYSTEM \
>      -fw_cfg name=etc/edk2/https/ciphers,blob_id=ciphersuite0
>    qemu-system-x86_64: -fw_cfg name=etc/edk2/https/ciphers,blob_id=ciphersuite0: warning: externally provided fw_cfg item names should be prefixed with "opt/"

Oops, this was supposed to be the last patch of the series. RFC anyway.

> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   softmmu/vl.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index f76c53ad2e..3b77dcc00d 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2052,7 +2052,7 @@ 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(blob_id) && strncmp(name, "opt/", 4) != 0) {
>           warn_report("externally provided fw_cfg item names "
>                       "should be prefixed with \"opt/\"");
>       }
> 

On 05/19/20 20:20, Philippe Mathieu-Daudé wrote:
> This is to silent:
> 
>   $ qemu-system-x86_64 \
>     -object tls-cipher-suites,id=ciphersuite0,priority=@SYSTEM \
>     -fw_cfg name=etc/edk2/https/ciphers,blob_id=ciphersuite0
>   qemu-system-x86_64: -fw_cfg name=etc/edk2/https/ciphers,blob_id=ciphersuite0: warning: externally provided fw_cfg item names should be prefixed with "opt/"
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  softmmu/vl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index f76c53ad2e..3b77dcc00d 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2052,7 +2052,7 @@ 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(blob_id) && strncmp(name, "opt/", 4) != 0) {
>          warn_report("externally provided fw_cfg item names "
>                      "should be prefixed with \"opt/\"");
>      }
> 

Hmmm, difficult question! Is "ciphersuite0" now externally provided or not?

Because, ciphersuite0 is populated internally to QEMU alright (and so we
can think it's internal), but its *association with the name* is external.

What if we keep the same "-object" switch, but use a different (bogus)
"name" with "-fw_cfg"? IMO that kind of invalidates "-object" too.

Should the fw_cfg generator interface dictate the fw_cfg filename too?
Because that would eliminate this problem. Put differently, we now have
a possibility to label the "ciphersuite0" object in the fw_cfg file
directory any way we want -- but is that freedom *useful* for anything?

I guess we might want multiple "tls-cipher-suites" objects one day, so
hard-coding the fw_cfg names on that level could cause conflicts. On the
other hand, I wouldn't like "blob_id" to generally circumvent the "etc/"
namespace protection.

I'm leaning towards agreeing with this patch, but I'd appreciate some
convincing arguments.

Thanks
Laszlo

On 5/20/20 12:45 AM, Laszlo Ersek wrote:
> On 05/19/20 20:20, Philippe Mathieu-Daudé wrote:
>> This is to silent:
>>
>>   $ qemu-system-x86_64 \
>>     -object tls-cipher-suites,id=ciphersuite0,priority=@SYSTEM \
>>     -fw_cfg name=etc/edk2/https/ciphers,blob_id=ciphersuite0
>>   qemu-system-x86_64: -fw_cfg name=etc/edk2/https/ciphers,blob_id=ciphersuite0: warning: externally provided fw_cfg item names should be prefixed with "opt/"
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  softmmu/vl.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index f76c53ad2e..3b77dcc00d 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -2052,7 +2052,7 @@ 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(blob_id) && strncmp(name, "opt/", 4) != 0) {
>>          warn_report("externally provided fw_cfg item names "
>>                      "should be prefixed with \"opt/\"");
>>      }
>>
> 
> Hmmm, difficult question! Is "ciphersuite0" now externally provided or not?
> 
> Because, ciphersuite0 is populated internally to QEMU alright (and so we
> can think it's internal), but its *association with the name* is external.
> 
> What if we keep the same "-object" switch, but use a different (bogus)
> "name" with "-fw_cfg"? IMO that kind of invalidates "-object" too.
> 
> Should the fw_cfg generator interface dictate the fw_cfg filename too?
> Because that would eliminate this problem. Put differently, we now have
> a possibility to label the "ciphersuite0" object in the fw_cfg file
> directory any way we want -- but is that freedom *useful* for anything?

Good point. Not now.

> 
> I guess we might want multiple "tls-cipher-suites" objects one day, so
> hard-coding the fw_cfg names on that level could cause conflicts.

I haven't thought of that. If we set a limit today, we can relax it
tomorrow. That won't break anything.

> On the
> other hand, I wouldn't like "blob_id" to generally circumvent the "etc/"
> namespace protection.

Eh, that was too easy.

> 
> I'm leaning towards agreeing with this patch, but I'd appreciate some
> convincing arguments.

OK. I'll think about it. Maybe keep this as single-RFC patch & repost
the rest so review can go on until we settle on this.

> 
> Thanks
> Laszlo
>