Make sure that no passwords are shown in the logs

[fuss86]

Task Description

When a user authenticates, the logs may contain his hashed password. We should make sure that we don't leak any credential information in the logs (even hashed passwords).

Tasks

The following tasks will need to be carried out:

• [ ] Enumerate all possible classes that uses credentials / passwords
• [ ] Make sure their toString methods do not contain sensitive data
• [ ] Make sure credentials / passwords are not logged anywhere

Help

• Our chat
• Points of contact:
  • @carlspring
  • @fuss86
  • @sbespalov
  • @steve-todorov

[n00ner]

I want to help

[carlspring]

Feel free to dig in! :)

[onobc]

I would recommend doing this for all PII. If we are already going to be scouring logs for creds - it woulld be worth doing that as well. Could be done in a follow on ticket as well.

[carlspring]

I'm assuming you're referring to "personally identifiable information"?

If so, I believe the only information that we keep a record of is the username/e-mail, so I'm not quite sure what else we could sanitize and restrict.

[onobc]

Yep. Email would be one.

[rohithjayaraman]

Has someone taken this up already ?

[carlspring]

@theparselmouth , @n00ner said they'd look into it, but as we haven't seen a pull request yet, I'd say it's probably up for grabs, so you can have a go at it, if you like.

[n00ner]

@theparselmouth, try it =)

I just studying and work slowly.

[RutgerDOW]

I'm participating at hacktoberfest and choose this issue to start working on.

[carlspring]

Hi @RutgerDOW ,

Feel free to look into it! Thanks for offering to help! :)

(As we haven't heard back from @n00ner and @theparselmouth , or seen a pull request, I believe it's safe to proceed).

Please, feel free to join our chat channel! :)

[RutgerDOW]

• Enumerate all possible classes that uses credentials / passwords

Do you need tooling for it, or is manually search and output to enumeration satisfying? In last case, how will it be maintained?

• Make sure their toString methods do not contain sensitive data

Is a search and delete in the logged data the solution, or is must all password object or string be omitted from methods of toString object in underlying classes?

[carlspring]

I think you can start by ommitting things from the toString method and work your way through. Deleting things through the logger might be quite an undertaking...

[steve-todorov]

@carlspring / @fuss86 / @sbespalov in addition and as a fallback option, maybe we could try to configure logback to mask certain messages using replace (example)?

[fuss86]

@steve-todorov I think it might be hard to prepare a regex for such purpose. We can't predict it I think.

[deepankkartikey]

@carlspring @fuss86 @sbespalov @steve-todorov Can I look into this? Just a beginner, willing to learn about code

[steve-todorov]

Hi @deepank120896 - feel free to pick it up. You can also join our chat if you have more questions. :)

[VishnuLakkimsetty]

Hi @deepank120896 - Are you contributing to this issue. If not I'm interested in contributing to this issue

[deepankkartikey]

@VishnuLakkimsetty Yes I am working on it

[Syntax753]

@deepank120896 Are you still working on this?

I've done a quick initial investigation and happy to continue. These would be the first files to look into:

find . -type f -name '*.java' | grep -v "/test" | xargs grep -l password | xargs grep -l toString                                              
./strongbox-rest-client/src/main/java/org/carlspring/strongbox/client/RestClient.java
./strongbox-security/strongbox-user-management/src/main/java/org/carlspring/strongbox/users/dto/UserDto.java
./strongbox-security/strongbox-user-management/src/main/java/org/carlspring/strongbox/users/userdetails/SpringSecurityUser.java
./strongbox-security/strongbox-user-management/src/main/java/org/carlspring/strongbox/users/domain/UserData.java
./strongbox-security/strongbox-authentication-providers/strongbox-default-authentication-provider/src/main/java/org/carlspring/strongbox/authentication/api/password/PasswordAuthenticationProvider.java
./strongbox-security/strongbox-authentication-providers/strongbox-default-authentication-provider/src/main/java/org/carlspring/strongbox/authentication/api/CacheManagerAuthenticationCache.java
./strongbox-storage/strongbox-storage-core/src/main/java/org/carlspring/strongbox/configuration/MutableProxyConfiguration.java
./strongbox-client/src/main/java/org/carlspring/strongbox/client/ArtifactClient.java

[steve-todorov]

@Syntax753 feel free to pick this up if you're still interested and open a PR :)

[DSkilton]

Hiya. Is this issue still open, could I have a go at it?

[Syntax753]

Gosh I forgot about that - I didn't make progress so +1 from my side :)

On Thu, 21 Apr 2022 at 20:14, Duncan Skilton @.***> wrote:

Hiya. Is this issue still open, could I have a go at it?

— Reply to this email directly, view it on GitHub https://github.com/strongbox/strongbox/issues/1511#issuecomment-1105656239, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFIVGGYWA2V4RARSXEILEILVGGSILANCNFSM4I6J366Q . You are receiving this because you were mentioned.Message ID: @.***>

[colinldbd]

Hi, I'm a contributor and I see this is a good first issue. Can you assign this issue to me so I can work on it?

Many thanks!

[carlspring]

@colinldbd : This project is on hold.

[khushbukareliya07]

Hi @carlspring - I would recommend adding an appropriate label for the status.

[carlspring]

@khushbukareliya07 : The point being? I assume you've read my last comment.