Skip to content

use a pattern to identify sensitive config keys - #16555

Merged
MorrisJobke merged 1 commit into
masterfrom
fix/16529/mask-keys
Jul 26, 2019
Merged

use a pattern to identify sensitive config keys#16555
MorrisJobke merged 1 commit into
masterfrom
fix/16529/mask-keys

Conversation

@blizzz

@blizzzblizzz commented Jul 26, 2019•
edited
Loading

Copy link
Copy Markdown
Member

regex for the win :)

fixes#16529 (see also for repro steps)

@blizzz
treat sensitive config keys by pattern
78201bc
Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
@blizzzblizzz added this to the Nextcloud 17 milestone Jul 26, 2019
MorrisJobke
MorrisJobke approved these changes Jul 26, 2019
View reviewed changes

@MorrisJobkeMorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Code makes sense 👍

rullzer
rullzer approved these changes Jul 26, 2019
View reviewed changes

@rullzerrullzer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Makes sense and even tests :)

@rullzerrullzer added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jul 26, 2019
@MorrisJobke
MorrisJobke merged commit 2e803dc into masterJul 26, 2019
@delete-merged-branch
delete-merged-branchBot deleted the fix/16529/mask-keys branch July 26, 2019 13:16
@MorrisJobke

MorrisJobke commented Jul 26, 2019

Copy link
Copy Markdown
Member

@blizzz Backport to 16? I would say so.

@MorrisJobke

MorrisJobke commented Jul 26, 2019

Copy link
Copy Markdown
Member

/backport to stable16

@MorrisJobke

MorrisJobke commented Jul 26, 2019

Copy link
Copy Markdown
Member

Just noticed the backport label. 🙈

@backportbot-nextcloudbackportbot-nextcloudBot mentioned this pull request Jul 26, 2019
Merged
@backportbot-nextcloud

backportbot-nextcloudBot commented Jul 26, 2019

Copy link
Copy Markdown

backport to stable16 in #16562

@blizzz

blizzz commented Jul 26, 2019

Copy link
Copy Markdown
MemberAuthor

i'd also say nc 15 to prevent accidential leaking

@blizzz

blizzz commented Jul 26, 2019

Copy link
Copy Markdown
MemberAuthor

/backport to stable15

@backportbot-nextcloudbackportbot-nextcloudBot mentioned this pull request Jul 26, 2019
Merged
@backportbot-nextcloud

backportbot-nextcloudBot commented Jul 26, 2019

Copy link
Copy Markdown

backport to stable15 in #16565

@CyrilBrulebois

CyrilBrulebois commented Jul 26, 2019

Copy link
Copy Markdown

Thanks for the quick fix for #16529; the regex strikes me as suboptimal as it would not catch the (possibly unlikely) cases where one reaches s100 and above…

/^(s\d\d+)?ldap_agent_password$/'

would ensure detecting two or more digits after the initial s. (\d\d+ can also be written as \d{2,}, but that's maybe less readable for newcomers.)

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rullzerrullzerrullzer approved these changes
@MorrisJobkeMorrisJobkeMorrisJobke approved these changes
@nickvergessennickvergessenAwaiting requested review from nickvergessen
@michaelletzgusmichaelletzgusAwaiting requested review from michaelletzgus
@juliusknorrjuliusknorrAwaiting requested review from juliusknorr

Assignees

No one assigned

Labels

4. to releaseReady to be released and/or waiting for tests to finishbug

Projects

None yet

Milestone

Nextcloud 17

Development

Successfully merging this pull request may close these issues.

user_ldap's ldap_agent_password not effectively tagged as sensitive data

4 participants

@blizzz@MorrisJobke@CyrilBrulebois@rullzer