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

MaskingJsonGeneratorDecorator masks only complete string #400

Closed
Kostakes opened this issue Apr 23, 2020 · 6 comments
Closed

MaskingJsonGeneratorDecorator masks only complete string #400

Kostakes opened this issue Apr 23, 2020 · 6 comments
Milestone

Comments

@Kostakes
Copy link

When the configuration is like this:

<appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender">
        <encoder class="net.logstash.logback.encoder.LogstashEncoder">
            <jsonGeneratorDecorator class="net.logstash.logback.mask.MaskingJsonGeneratorDecorator">
                <defaultMask>****</defaultMask>
                <value>command</value>
            </jsonGeneratorDecorator>
        </encoder>
    </appender>

It would mask only the string which is exactly "command".
For example log {"message":"command"} would be transformed into {"message":"****"}.

But it doesn't mask if word 'command' is part of the string.
For example log {"message":"Sending command bla"} would result in {"message":"Sending command bla"} and I would expect {"message":"Sending **** bla"}

I think it's because while checking if value matches logs, matches() method is used on Matcher, but should be used find()

Could you please take a look into this.

@philsttr
Copy link
Collaborator

philsttr commented Apr 24, 2020

The current design is meant to mask entire field values, not partially mask field values.

This would be a good enhancement though.

In the meantime, you can provide your own implementation of net.logstash.logback.mask.ValueMasker to perform masking however you like.

@michael-wirth
Copy link

I have the same issue.

A working solution is to create a new implementation based on the net.logstash.logback.mask.RegexValueMasker.
Would be great to have this option in the upcoming releases.

    public Object mask(JsonStreamContext context, Object o) {
        if (o instanceof CharSequence) {
            Matcher matcher = pattern.matcher((CharSequence) o);
            if (matcher.matches()) { // --> change to matcher.find()
                return mask instanceof String
                        ? matcher.replaceAll((String) mask)
                        : mask;
            }
        }
        return null;
    } 

@bhavin9695
Copy link

Hi @michael-wirth ,

I am facing same issue. how were you manage to solve this ? can you share more detail ?

I getting below error when i try to create another class like RegexValueMasker.

ERROR in ch.qos.logback.core.joran.action.NestedComplexPropertyIA - Could not create component [valueMasker] of type [com.mypackage.config.SensitiveDataMasker] java.lang.InstantiationException: com.mypackage.config.SensitiveDataMasker

Below is the SensitiveDataMasker class

public class SensitiveDataMasker implements ValueMasker {

    private final Pattern pattern;
    private final Object mask;


    public SensitiveDataMasker(String regex, Object mask) {
        this(Pattern.compile(regex), mask);
    }

    public SensitiveDataMasker(Pattern pattern, Object mask) {
        this.pattern = Objects.requireNonNull(pattern, "pattern must not be null");
        this.mask = Objects.requireNonNull(mask, "rmask must not be null");
    }

    @Override
    public Object mask(JsonStreamContext context, Object o) {
        if (o instanceof CharSequence) {
            Matcher matcher = pattern.matcher((CharSequence) o);
            if (matcher.find()) {
                return mask instanceof String
                        ? matcher.replaceAll((String) mask)
                        : mask;
            }
        }
        return null;
    }
}

Below is the configuration of logback-spring.xml .

<encoder class="net.logstash.logback.encoder.LogstashEncoder">
    <jsonGeneratorDecorator class="net.logstash.logback.mask.MaskingJsonGeneratorDecorator">
        <defaultMask>****</defaultMask>
        <value>(Authorization\s*:\s*\"?\'?Bearer\s|Authorization\s*=\s*\"?\'?Bearer\s)[\w\.\-^]+\"?\'?</value>
        <valueMasker class="com.mypackage.config.SensitiveDataMasker" />
    </jsonGeneratorDecorator>
</encoder>

I believe it is not able to instantiate the class as it does not have any default constructor. but then how to pass regex and mask strings ? Although i can hardcode those values in class itself but i am looking for a way to pass it from logback-spring.xml file

Thanks

@michael-wirth
Copy link

Hi @bhavin9695

I didn't find a way to define it in the logback-spring.xml.

I solved it by defining the sensitive pattern in an external file.

Here is my source code (I implemented it in Kotlin).
Hope this helps.

class RegexFindValueMasker : ValueMasker {

    private val patterns: List<Regex>

    init {
        ClassPathResource(REGEX_PATTERN_FILE_LOCATION).run {
            patterns = if (isFile) {
                inputStream.reader().readLines().map(::Regex)
            } else listOf()
        }
    }

    override fun mask(context: JsonStreamContext, value: Any) =
        if (context.currentName == MESSAGE && value is String) {

            patterns.flatMap { it.findAll(value) }
                .map { it.groupValues[1.coerceAtMost(it.groupValues.size)] }
                .distinct()
                .fold(value) { newValue, matchedLabel -> newValue.replace(matchedLabel, MASK) }
        } else null

    companion object {
        private const val REGEX_PATTERN_FILE_LOCATION = "logstash/mask.patterns"
        private const val MESSAGE = "message"
        private const val MASK = "*****"
    }
}

logstash-spring.xml

<!-- mask values in the log message -->
<jsonGeneratorDecorator class="net.logstash.logback.mask.MaskingJsonGeneratorDecorator">
    <!-- custom value masker, replaces values matching the patterns in logstash/mask.patterns -->
    <valueMasker class="ch.migrosbank.eb.starter.web.logging.logstash.RegexFindValueMasker"/>
</jsonGeneratorDecorator>

logstash/mask.patterns

(?i)contractId=(.*)(?:,|\)|$)

@bhavin9695
Copy link

Thanks, @michael-wirth
I had to do with the same approach except for a separate file of 'mask.patterns'. I have defined patterns in the same ValueMasker class file.

Thanks,
Bhavin

@philsttr
Copy link
Collaborator

philsttr commented Jun 25, 2021

After reviewing the current implementation, I believe my initial comment above was incorrect. The path matching support was intended to mask full values. The value matching support was intended to mask all matching substrings within a string field value.

I have changed the current implementation to mask all matching substrings, and clarified the documentation. I'll call out this change in the release notes for the next version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants