When "default" version of TPM was provided, our parses accepts it
happily even though the value is forbidden by our RNG and not
documented as accepted value. This is because of < 0 vs <= 0
comparison of virDomainTPMModelTypeFromString() retval.

Make the parser error out explicitly in this case. Users can
always chose to not specify the attribute in which case we pick a
sane default (in qemuDomainDefTPMsPostParse()).

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/conf/domain_conf.c | 2 +-
 src/conf/domain_conf.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b7147945da..6c178783af 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10400,7 +10400,7 @@ virDomainTPMDefParseXML(virDomainXMLOption *xmlopt,
     if (!version) {
         def->version = VIR_DOMAIN_TPM_VERSION_DEFAULT;
     } else {
-        if ((def->version = virDomainTPMVersionTypeFromString(version)) < 0) {
+        if ((def->version = virDomainTPMVersionTypeFromString(version)) <= 0) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("Unsupported TPM version '%s'"),
                            version);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5a057c36b8..7139b91aca 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1417,7 +1417,7 @@ typedef enum {
 } virDomainTPMBackendType;
 
 typedef enum {
-    VIR_DOMAIN_TPM_VERSION_DEFAULT,
+    VIR_DOMAIN_TPM_VERSION_DEFAULT = 0,
     VIR_DOMAIN_TPM_VERSION_1_2,
     VIR_DOMAIN_TPM_VERSION_2_0,
 
-- 
2.35.1

On Mon, Jul 18, 2022 at 11:30:44 +0200, Michal Privoznik wrote:
> When "default" version of TPM was provided, our parses accepts it
> happily even though the value is forbidden by our RNG and not
> documented as accepted value. This is because of < 0 vs <= 0
> comparison of virDomainTPMModelTypeFromString() retval.
> 
> Make the parser error out explicitly in this case. Users can
> always chose to not specify the attribute in which case we pick a
> sane default (in qemuDomainDefTPMsPostParse()).
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/conf/domain_conf.c | 2 +-
>  src/conf/domain_conf.h | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)

This has the same issue as I've reported in previous patch.

On 8/1/22 13:11, Peter Krempa wrote:
> On Mon, Jul 18, 2022 at 11:30:44 +0200, Michal Privoznik wrote:
>> When "default" version of TPM was provided, our parses accepts it
>> happily even though the value is forbidden by our RNG and not
>> documented as accepted value. This is because of < 0 vs <= 0
>> comparison of virDomainTPMModelTypeFromString() retval.
>>
>> Make the parser error out explicitly in this case. Users can
>> always chose to not specify the attribute in which case we pick a
>> sane default (in qemuDomainDefTPMsPostParse()).
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  src/conf/domain_conf.c | 2 +-
>>  src/conf/domain_conf.h | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> This has the same issue as I've reported in previous patch.
> 

The 'default' gets overwritten in qemuDomainDefTPMsPostParse() to
something sensible. So we would never ever output 'default'.

Michal

On Mon, Aug 01, 2022 at 15:08:27 +0200, Michal Prívozník wrote:
> On 8/1/22 13:11, Peter Krempa wrote:
> > On Mon, Jul 18, 2022 at 11:30:44 +0200, Michal Privoznik wrote:
> >> When "default" version of TPM was provided, our parses accepts it
> >> happily even though the value is forbidden by our RNG and not
> >> documented as accepted value. This is because of < 0 vs <= 0
> >> comparison of virDomainTPMModelTypeFromString() retval.
> >>
> >> Make the parser error out explicitly in this case. Users can
> >> always chose to not specify the attribute in which case we pick a
> >> sane default (in qemuDomainDefTPMsPostParse()).
> >>
> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> >> ---
> >>  src/conf/domain_conf.c | 2 +-
> >>  src/conf/domain_conf.h | 2 +-
> >>  2 files changed, 2 insertions(+), 2 deletions(-)
> > 
> > This has the same issue as I've reported in previous patch.
> > 
> 
> The 'default' gets overwritten in qemuDomainDefTPMsPostParse() to
> something sensible. So we would never ever output 'default'.

Thus this boils down to same condition I gave with previous patch. If
you want to make the parser more strict, make the formatter skip the
value which would be rejected by the parser.