@Mask - new Annotation suggestion - Field Target

[Vidyadhar V. Bhutaki]

Hello All,

I am new to Annotation processing and would like to learn more about it as well suggest to have a Mask annotation added to existing annotations

The behavior that i am expecting goes as below

@Mask annotation to be applied at field level.

This annotation would provide 2 methods for that field

1. A getter method that would always return String value with certain characters masked. The characters to be masked could be part of annotation declaration

2. A unmasked method that would actually return the value of the field without making. In other words the unmasked function would act as a getter method.

By doing so, we would prevent from accidently logging sensitive information in logs or other systems when we use getter method.

I would like to contribute to the annotation if anyone could guide on how do i proceed here as i am new to this annotation processing

[Reinier Zwitserloot]

This falls in the more general camp of 'can lombok help me because I log sensitive information'. Our policy is that you're doomed if this is your strategy to prevent such things. Why is there sensitive information in fields - and why are you logging these objects? Masking 'certain things out' is even crazier. I bet 9 out of 10 users will do dumb things, such as replacing a password with an amount of stars equal to the number of characters in it. Don't do that.

There's already something like:

```

public class Example {

  private final String password;

  @ToString.Include

  private String password() {

    return password.isEmpty() ? "(no password)" : "(some password set)";

  }

}

```

and you can't call that 'oh, boilerplate we should remove!' because the word 'password', as well as the sentinelling behaviour, is non-standard (perhaps for other things, it's not about being empty but being null, or containing a dash, or who knows).

Feel free to propose a specific transformation (with 'this is what it looks like with lombok', and 'this is what it would look like without'), but I rate the odds 99%+ it would be shot down as pointless or 'doing that to your code base is actively ruining things often enough that we will veto the feature'.

[Vidyadhar V. Bhutaki]

Hello  Reinier,

Thank you for your response.

For us the idea was to use Lombok to prevent accidental logging of critical sensitive information in any Log files or any other system that it is not intended to

Like in a scenario, where a Developer accidentally logs critical information using getter methods

Example for Current Model class

@Data

public class User{

      private String name;

      private String dob;

      private String securityNumber;

}

Usage

log.error("Failed to find name:{}, DOB: {}, SecurityNumber:{}", user.getName(),user.getDob,user.getSecurityNumber);

As you can see the above line would already print security number in logs.

Proposed change in generated Model class if annoted with @Mask

@Data

public class User{

     private String name;

     private String dob;

    @Mask(value=****,start=4,end=8)

    private String securityNumber;

/*Lombok generate functions due to @Mask annotation*/

private String getSecurityNumber(){

      return securityNumber.substring(0,  start  ) + "****" + str.substring(8+securityNumber.length());

}

private String getUnmaskedSecurityNumber(){

      return securityNumber;

}

Usage when logging

log.error("Failed to find name:{}, DOB: {}, SecurityNumber:{}", user.getName(),user.getDob,user.getSecurityNumber);

Usage when Calculations

user.getUnmaskedSecurityNumber();

I would appreciate if you could guide me with any another approaches/patterns that I can use if already exists

Thank you

Vidyadhar

[Mat Jaggard]

Accidental logging is really not a good use case for breaking conventions like you're proposing here. Getters are designed to get a value, not a changed version of the value. In addition, any implementation in here would have to be rock solid. For example you could use a timing attack on your example code to find the length of a password.

Your best solution is to have a good code review policy so that at least 2 pairs of trusted eyes have looked at the code before it goes live. Things like logging a social security number should be spotted immediately. Also add @ToString.Exclude so that people don't log private information accidentally when logging the whole object which would much harder to spot.

--
You received this message because you are subscribed to the Google Groups "Project Lombok" group.
To unsubscribe from this group and stop receiving emails from it, send an email to project-lombo...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/project-lombok/5b986c54-4a1a-420e-8927-c0fbc856f9b3n%40googlegroups.com.

[Vidyadhar V. Bhutaki]

Hello Matthew,

Thank you for your inputs. 

I workfor a banking organization and thats why i was trying to explore if any solution as such exist that could be applied for critical information. But i get your points

Thank you for the inputs. We will consider using Tostring.exclude

Thank you

Vidyadhar 

To view this discussion on the web visit https://groups.google.com/d/msgid/project-lombok/CAD0R3sBs5hDsfPEyNKfPKUB%2BHCN9-SeEa5TD9oTSzETNQ7Ff1w%40mail.gmail.com.

--

Thank you ,

Vidyadhar Bhutaki

[Mat Jaggard]

I work for a UK bank and frequently get requests to make things more complicated in the name of security but it is not a good idea. Fewer lines of more understandable code that's been reviewed by trusted people is always better for security.

We did implement a layer in our logging framework to filter out things that looked like credit card numbers though. Maybe you could do something similar?

To view this discussion on the web visit https://groups.google.com/d/msgid/project-lombok/CAPvN%2BNfw%2B8bsDTihEjmnWaj5HkkLdDuFxY-7BWbneU%2Bj4h76_g%40mail.gmail.com.

[Emil Lundberg]

I believe a better solution to this, if you want it, is to instead wrap your sensitive values in a wrapper type that does the masking. For example, in one project of mine I introduced a Secret<T> type whose toString method would always return "(Secret)". Something like this:

class Secret<T> {
  private final T value;
  public Secret(T value) {
    this.value = value;
  }

  @Override
  public String toString() {
    return "(Secret)";
  }

  public <U> Secret<U> map(Function<T, U> f) {
    return new Secret(f.apply(this.value));
  }

  public T reveal() {
    return value;
  }

  public void revealTo(Consumer<T> f) {
    f.accept(this.value);
  }
}

Using a wrapper type like this also has the added benefit that you the developer can easily track which values are sensitive and not, because that property is encoded in the type system. When you want to access the sensitive value you need to take an explicit extra step to do so. You could even annotate the reveal() method with @Deprecated or use other linting tools to get warnings on possible abuse of sensitive values. Logs aren't the only place sensitive information might be exposed, they could end up in HTTP responses or the like too. So of course you should also configure any serialization tools you use to serialize these values to an opaque constant value by default.

Emil Lundberg

Senior Software Engineer | Yubico

To view this discussion on the web visit https://groups.google.com/d/msgid/project-lombok/CAD0R3sDey2Myt_wzRq5ex20dDAtDR31mZa2VOjpncVU2Ud3hwQ%40mail.gmail.com.

[Nikolay Shustov]

Just my 2 cents: to filter out the sensitive information, sanitizing the code (especially with heuristic algorithms) will still give you quite a spotty solution, at best.
The cost of mistake though could be way too much to prove such approach sufficient.
First of all, the financial institutions, like banks are subjects to various information protection compliance regulations; the inability to implement those reliably may lead to severe fines.
Second of all, the accidental disclosure of such information may lead to real monetary losses of the customers which may damage bank's reputation and operations beyond repairs.

The enterprise solution requires enterprise based approach. For example, your bank may consider data masking solutions from Ab Initio.

I understand that it is probably not "your bank" and you may have little say in what's going on.

At any rate, if I were you, I would try keep a distance from such (imho, questionable) practices at least not to become a scapegoat when shit hits the fan.

Cheers :-)

[Luck]

I understand you and I think this functionality is very good. In Brazil we have a LGPD (general data protection law) and this feature would be a good feature in Brazil. Currently we used converters to do it.

[Martin Poitras]

Another 2 cents, stay away from finger pointing bank style, that aside:

I think you see the problem from the wrong direction.

1. First, never put a getter on a sensitive information. Instead create a function that will tell you if an action is appropriate base on class sensitive information.
2. Second make that field transient with @ToString.Exclude for sure.
   
3. String can be intern() and remains visible, prefer byte array.