[PATCH 1/3] conf: clean up memory containing secrets before freeing

[Peter Krempa]

On Tue, Sep 06, 2022 at 21:48:29 +0800, Jiacheng Jiang wrote:
> From: jiangjiacheng <jiangjiacheng at huawei.com>
> 
> The password may not be valid in the error branch, but for
> higher security, it's better to clean up the memory before
> freeing it.
> 
> Signed-off-by: jiangjiacheng <jiangjiacheng at huawei.com>
> ---
>  src/conf/domain_conf.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 970cc85ded..d456fd0067 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -60,6 +60,7 @@
>  #include "virdomainsnapshotobjlist.h"
>  #include "virdomaincheckpointobjlist.h"
>  #include "virutil.h"
> +#include "virsecureerase.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_DOMAIN
>  
> @@ -10888,6 +10889,7 @@ virDomainGraphicsAuthDefParseXML(xmlNodePtr node,
>              virReportError(VIR_ERR_INTERNAL_ERROR,
>                             _("cannot parse password validity time '%s', expect YYYY-MM-DDTHH:MM:SS"),
>                             validTo);
> +            virSecureEraseString(def->passwd);
>              VIR_FREE(def->passwd);
>              return -1;
>          }

In this form the patch is not very useful. When freeing a successfully
parsed definition the 'passwd' field isn't erased either.

In general it makes little sense to do this for the XML parser because
there's another instance in the XML parser structs which isn't erased
either and there are multiple other instances of the code where we don't
do that either.

This patch should not be merged.