Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[generate_dump] remove secrets from dump files #1886

Merged

Conversation

liuh-80
Copy link
Contributor

@liuh-80 liuh-80 commented Oct 19, 2021

Remove secrets from dump files.

What I did

Add bash functions to remove secrets from dump files.

How I did it

For tacacs key, radius key, snmp community srring, use sed command with regex to remove user secrets from dump files.
For certs, update tar command exclude list to remove those certs from dump file.

How to verify it

Run 'show techsupport' command and check secrets removed from dump files.

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

@liuh-80
Copy link
Contributor Author

liuh-80 commented Oct 21, 2021

azp /run

@liuh-80
Copy link
Contributor Author

liuh-80 commented Oct 21, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liuh-80 liuh-80 marked this pull request as ready for review October 22, 2021 01:55
@lguohan
Copy link
Contributor

lguohan commented Oct 22, 2021

need to back port this to 202012, 202006, 201911

@@ -180,6 +181,13 @@ save_cmd() {
redirect_eval=""
fi

# Cleanup metthod need do post procress on dump file.
Copy link
Contributor

@qiluo-msft qiluo-msft Oct 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

metthod

typo #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

echo "Remove secret from '$filepath' files."

# Remove tacacs & radius passkey from config DB
sed -i -E 's/\"passkey\"\s*:\s*\"([^\"]*)\"/\"passkey\":\"\"/g' $filepath
Copy link
Contributor

@qiluo-msft qiluo-msft Oct 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

passkey":""

Do you leave an emtpy passkey?
Is it better to replace with a fixed length of ****?
One benefit is indicating there is passkey cofigured. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

echo "Remove secret from etc files."
# Remove tacacs passkey from tacplus_nss.conf
local secret_regex='s/(secret=)([^,|\S]*)(.*)/\1***\3/g'
sed -i -E $secret_regex $TARDIR/etc/tacplus_nss.conf
Copy link
Contributor

@qiluo-msft qiluo-msft Oct 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$TARDIR

Better to use function parameter instread of global var. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@@ -155,6 +154,7 @@ save_bcmcmd_all_ns() {
# cmd: The command to run. Make sure that arguments with spaces have quotes
# filename: the filename to save the output as in $BASE/dump
# do_gzip: (OPTIONAL) true or false. Should the output be gzipped
# cleanup_method: (OPTIONAL) the cleanup method to procress dump file after it generated.
Copy link
Contributor

@qiluo-msft qiluo-msft Oct 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cleanup_method

Is it possible to accept both do_gzip=true and cleanup_method=something? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently when do_gzip=true, dump output been redirect to gzip command with pipeline.
However, sed can work with pipeline, so I will investigate if we can invoke cleanup method in pipeline.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, UT failed because azure pipeline issue.

@qiluo-msft
Copy link
Contributor

There is no 202006 branch. Is it a typo or do you really want creating that branch and backport?


In reply to: 949225407

@liuh-80
Copy link
Contributor Author

liuh-80 commented Oct 26, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liuh-80 liuh-80 merged commit 9f123c0 into sonic-net:master Oct 28, 2021
@qiluo-msft
Copy link
Contributor

Could you add some test cases in sonic-mgmt? I see some files in tests/show_techsupport/

@qiluo-msft
Copy link
Contributor

This commit could not be cleanly cherry-picked to 202012. Please submit another PR.

liuh-80 added a commit to liuh-80/sonic-utilities that referenced this pull request Nov 22, 2021
…to 202012

#### What I did
    Add bash functions to remove secrets from dump files.

#### How I did it
    For tacacs key, radius key, snmp community srring, use sed command with regex to remove user secrets from dump files.
    For certs, update tar command exclude list to remove those certs from dump file.

#### How to verify it
    Run 'show techsupport' command and check secrets removed from dump files.

#### Previous command output (if the output of a command-line utility has changed)

#### New command output (if the output of a command-line utility has changed)
liuh-80 added a commit that referenced this pull request Nov 22, 2021
…12 (#1938)

#### What I did
    Add bash functions to remove secrets from dump files.

#### How I did it
    For tacacs key, radius key, snmp community srring, use sed command with regex to remove user secrets from dump files.
    For certs, update tar command exclude list to remove those certs from dump file.

#### How to verify it
    Run 'show techsupport' command and check secrets removed from dump files.

#### Previous command output (if the output of a command-line utility has changed)

#### New command output (if the output of a command-line utility has changed)
liuh-80 added a commit to liuh-80/sonic-utilities that referenced this pull request Dec 6, 2021
Remove secrets from dump files.

What I did
    Add bash functions to remove secrets from dump files.
How I did it
    For tacacs key, radius key, snmp community srring, use sed command with regex to remove user secrets from dump files.
    For certs, update tar command exclude list to remove those certs from dump file.
How to verify it
    Run 'show techsupport' command and check secrets removed from dump files.
Previous command output (if the output of a command-line utility has changed)
New command output (if the output of a command-line utility has changed)
# Conflicts:
#	scripts/generate_dump
abdosi pushed a commit that referenced this pull request Dec 9, 2021
Backport [generate_dump] remove secrets from dump files #1886 to 201911

What I did
Add bash functions to remove secrets from dump files.
How I did it
For tacacs key, radius key, snmp community srring, use sed command with regex to remove user secrets from dump files.
For certs, update tar command exclude list to remove those certs from dump file.
How to verify it
Run 'show techsupport' command and check secrets removed from dump files.
stepanblyschak pushed a commit to stepanblyschak/sonic-utilities that referenced this pull request Apr 18, 2022
* c3691d3 [202012][pfcwd] Convert polling interval from ms to us in LUA scripts (sonic-net#1909)
* 549c804 Mux state order change (sonic-net#1902)
* 6b0b2c4 Update acl type check logic (sonic-net#1886)

Signed-off-by: Volodymyr Samotiy <volodymyrs@nvidia.com>
stepanblyschak pushed a commit to stepanblyschak/sonic-utilities that referenced this pull request Apr 18, 2022
Submodule update for sonic-utilities with following change:

ec9e5ee Backport [generate_dump] remove secrets from dump files sonic-net#1886 to 202012 (sonic-net#1938)
ce3b856 [fdbshow]: Handle FDB cleanup gracefully. (sonic-net#1926)
1437bf2 [202012] Add DHCPv6 Relay counter and ipv6 helper CLI (sonic-net#1917)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants