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

Validation of role template scripts is too strict for Painless scripts #62744

Closed
ywangd opened this issue Sep 22, 2020 · 1 comment · Fixed by #62845
Closed

Validation of role template scripts is too strict for Painless scripts #62744

ywangd opened this issue Sep 22, 2020 · 1 comment · Fixed by #62845
Assignees
Labels
:Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) Team:Security Meta label for security team

Comments

@ywangd
Copy link
Member

ywangd commented Sep 22, 2020

PR #52636 introduces validation for role template scripts. The validation is done by attempting to execute the scripts with an empty input. If no error is encountered, the script is considered to be valid. This approach is chosen under the assumption that all scripts are Mustache. However this is true only for inline scripts. Stored scripts on the other hand can be in painless.

Unlike Mustache, which is very lenient for empty variables, Painless can throw NEP on empty variables if not guarded properly. It feels too aggressive asking user to always perform null checking in Painless just for validation purpose because some of the input parameters are guaranteed to be non-empty, e.g. username.

In summary, we need to relax the validation for Painless scripts. Possible options are:

  • Just skip the validation for Painless. Since it is a stored script, it is better that the script is checked when it is stored and in this case, role template can just assume it is valid. (Note this is not true currently, we have no checking for scripts when they are stored)
  • Use compilation instead of execution as validation to avoid having to pass empty input.

Relates: #48773

@ywangd ywangd added the :Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) label Sep 22, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security (:Security/Authentication)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Security/Authentication Logging in, Usernames/passwords, Realms (Native/LDAP/AD/SAML/PKI/etc) Team:Security Meta label for security team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants