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

config-schema shouldn't log sensitive data #58652

Closed
mshustov opened this issue Feb 26, 2020 · 3 comments · Fixed by #58843
Closed

config-schema shouldn't log sensitive data #58652

mshustov opened this issue Feb 26, 2020 · 3 comments · Fixed by #58843
Assignees
Labels
Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc

Comments

@mshustov
Copy link
Contributor

A customer complains that we config-schema logs sensitive data in the plain text

We provided an invalid encryption key for Kibana 7.6.0 and were surprised to find that when there is an error it logs the encryption key in plain text:
config validation of [xpack.encryptedSavedObjects].encryptionKey]: value is [some_value] but it must have a minimum length of [32].

We need to provide a way to filter out sensitive data. For example, we can mark a key as containing sensitive data to prevent disclosure.

@mshustov mshustov added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:New Platform labels Feb 26, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@pgayvallet
Copy link
Contributor

One other option would be to have a convention to never display the actual data value in any error message. It seems there are only very few messages where we do display the raw values. Most messages are like expected value of type [string] but got [${typeDetect(value)}]

The value is [some_value] but it must have a minimum length of [32] could be changed to valuehas length [XX] but it must have a minimum length of [32].

This would avoid introducing a parameter for that, and the risk that a developer actually forget to flag sensitive data validation with it.

@pgayvallet
Copy link
Contributor

So, after a 'quick' look, impacted types are:

  • uri

case 'string.uriCustomScheme':
return `expected URI with scheme [${scheme}] but got [${value}].`;
case 'string.uri':
return `value is [${value}] but it must be a valid URI (see RFC 3986).`;

  • string

if (options.minLength !== undefined) {
schema = schema.custom(value => {
if (value.length < options.minLength!) {
return `value is [${value}] but it must have a minimum length of [${options.minLength}].`;
}
});
}
if (options.maxLength !== undefined) {
schema = schema.custom(value => {
if (value.length > options.maxLength!) {
return `value is [${value}] but it must have a maximum length of [${options.maxLength}].`;
}
});
}

case 'string.hostname':
return `value is [${value}] but it must be a valid hostname (see RFC 1123).`;

  • number

case 'number.min':
return `Value is [${value}] but it must be equal to or greater than [${limit}].`;
case 'number.max':
return `Value is [${value}] but it must be equal to or lower than [${limit}].`;

  • record / object / map

case 'record.parse':
return `could not parse record value from [${value}]`;

  • literal

case 'any.allowOnly':
return `expected value to equal [${expectedValue}] but got [${value}]`;

  • byte_size

case 'bytes.min':
return `Value is [${value.toString()}] ([${value.toString(
'b'
)}]) but it must be equal to or greater than [${limit.toString()}]`;
case 'bytes.max':
return `Value is [${value.toString()}] ([${value.toString(
'b'
)}]) but it must be equal to or less than [${limit.toString()}]`;

Hiding the actual value in some of these errors will strongly reduce the help the message actually provides (thinking about literal, number, byte_size mostly but all overall). It's probably not that important though. Do we think this is still acceptable to just remove value reference in every of them?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:New Platform Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants