[PATCH] report-error: Mask AWS credentials to avoid leaking them

[PATCH] report-error: Mask AWS credentials to avoid leaking them

[Damian Wrobel]

From: Adam Romanek <romanek.adam@gmail.com>

Removes potentially presonal information from error reports.

Signed-off-by: Adam Romanek <romanek.adam@gmail.com>
Signed-off-by: Damian Wrobel <dwrobel@ertelnet.rybnik.pl>
---
 meta/classes/report-error.bbclass | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/meta/classes/report-error.bbclass b/meta/classes/report-error.bbclass
index de48e4ff0f..c17e7e2851 100644
--- a/meta/classes/report-error.bbclass
+++ b/meta/classes/report-error.bbclass
@@ -38,6 +38,15 @@ def get_conf_data(e, filename):
                     jsonstring=jsonstring + line
     return jsonstring
 
+def errorreport_maskvars(e, logdata):
+    origenv = e.data.getVar("BB_ORIGENV", False)
+    vars_to_mask = ("TOPDIR", "TMPDIR", "AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY")
+    for var_name in vars_to_mask:
+        var_value = e.data.getVar(var_name, True) or (origenv and origenv.getVar(var_name, True))
+        if var_value:
+            logdata = logdata.replace(var_value, var_name)
+    return logdata
+
 python errorreport_handler () {
         import json
         import codecs
@@ -82,11 +91,7 @@ python errorreport_handler () {
                 try:
                     with codecs.open(log, encoding='utf-8') as logFile:
                         logdata = logFile.read()
-                    # Replace host-specific paths so the logs are cleaner
-                    for d in ("TOPDIR", "TMPDIR"):
-                        s = e.data.getVar(d)
-                        if s:
-                            logdata = logdata.replace(s, d)
+                    logdata = errorreport_maskvars(e, logdata)
                 except:
                     logdata = "Unable to read log file"
             else:
-- 
2.31.1

Re: [OE-core] [PATCH] report-error: Mask AWS credentials to avoid leaking them

[Josef Holzmayr]

Howdy!

Am 13.07.2021 um 11:22 schrieb Damian Wrobel:
> From: Adam Romanek <romanek.adam@gmail.com>
> 
> Removes potentially presonal information from error reports.
> 
> Signed-off-by: Adam Romanek <romanek.adam@gmail.com>
> Signed-off-by: Damian Wrobel <dwrobel@ertelnet.rybnik.pl>
> ---
>   meta/classes/report-error.bbclass | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/meta/classes/report-error.bbclass b/meta/classes/report-error.bbclass
> index de48e4ff0f..c17e7e2851 100644
> --- a/meta/classes/report-error.bbclass
> +++ b/meta/classes/report-error.bbclass
> @@ -38,6 +38,15 @@ def get_conf_data(e, filename):
>                       jsonstring=jsonstring + line
>       return jsonstring
>   
> +def errorreport_maskvars(e, logdata):
> +    origenv = e.data.getVar("BB_ORIGENV", False)
> +    vars_to_mask = ("TOPDIR", "TMPDIR", "AWS_ACCESS_KEY_ID", "AWS_SECRET_ACCESS_KEY")
> +    for var_name in vars_to_mask:
> +        var_value = e.data.getVar(var_name, True) or (origenv and origenv.getVar(var_name, True))
> +        if var_value:
> +            logdata = logdata.replace(var_value, var_name)
> +    return logdata
> +
>   python errorreport_handler () {
>           import json
>           import codecs
> @@ -82,11 +91,7 @@ python errorreport_handler () {
>                   try:
>                       with codecs.open(log, encoding='utf-8') as logFile:
>                           logdata = logFile.read()
> -                    # Replace host-specific paths so the logs are cleaner
> -                    for d in ("TOPDIR", "TMPDIR"):
> -                        s = e.data.getVar(d)
> -                        if s:
> -                            logdata = logdata.replace(s, d)
> +                    logdata = errorreport_maskvars(e, logdata)
>                   except:
>                       logdata = "Unable to read log file"
>               else:
> 
> 

This sounds a bit like suggesting that we add a hardcoded magic list of 
masks for possbible places for credentials, and I don't think this is a 
good approach. If at all, then there should be a more generic way that 
doesn't treat S3 as super special.

Greetz

> 
> 
>