You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/10/20 19:39:23 UTC
[GitHub] [nifi] exceptionfactory opened a new pull request, #6562: NIFI-10674 Block evaluateELString from reading Sensitive Parameters
exceptionfactory opened a new pull request, #6562:
URL: https://github.com/apache/nifi/pull/6562
# Summary
[NIFI-10674](https://issues.apache.org/jira/browse/NIFI-10674) Blocks the `evaluateELString()` function from accessing Sensitive Parameter Values.
Changes include new unit tests that run `evaluateELString()` with and without Sensitive Parameter references to illustrate intended behavior.
Additional changes include cleaning up several unused and unnecessary expressions in test classes.
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [X] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### Pull Request Tracking
- [X] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
- [X] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
### Pull Request Formatting
- [X] Pull Request based on current revision of the `main` branch
- [X] Pull Request refers to a feature branch with one commit containing changes
# Verification
Please indicate the verification steps performed prior to pull request creation.
### Build
- [ ] Build completed using `mvn clean install -P contrib-check`
- [ ] JDK 8
- [ ] JDK 11
- [ ] JDK 17
### Licensing
- [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
- [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
### Documentation
- [ ] Documentation formatting appears as expected in rendered files
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #6562: NIFI-10674 Block evaluateELString from reading Sensitive Parameters
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6562:
URL: https://github.com/apache/nifi/pull/6562#issuecomment-1286078324
That's a great question @MikeThomsen. I would be surprised if this scenario occurs in deployed systems, given the required interpolation of Parameter References is strings subsequently evaluated through `evaluateELString()`. The `evaluateELString()` function is in a unique category, so these changes are more to align the implementation with expected uses.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markap14 commented on pull request #6562: NIFI-10674 Block evaluateELString from reading Sensitive Parameters
Posted by GitBox <gi...@apache.org>.
markap14 commented on PR #6562:
URL: https://github.com/apache/nifi/pull/6562#issuecomment-1287294299
Thanks @exceptionfactory. Changes look good, unit tests are helpful, all completed successfully. +1 will merge to main.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markap14 merged pull request #6562: NIFI-10674 Block evaluateELString from reading Sensitive Parameters
Posted by GitBox <gi...@apache.org>.
markap14 merged PR #6562:
URL: https://github.com/apache/nifi/pull/6562
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] MikeThomsen commented on pull request #6562: NIFI-10674 Block evaluateELString from reading Sensitive Parameters
Posted by GitBox <gi...@apache.org>.
MikeThomsen commented on PR #6562:
URL: https://github.com/apache/nifi/pull/6562#issuecomment-1286064430
@exceptionfactory curious where this actually rears its ugly head in live systems.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] markap14 commented on pull request #6562: NIFI-10674 Block evaluateELString from reading Sensitive Parameters
Posted by GitBox <gi...@apache.org>.
markap14 commented on PR #6562:
URL: https://github.com/apache/nifi/pull/6562#issuecomment-1287089943
Thanks @exceptionfactory. I think this does prevent exactly what the Jira describes, and this is good to address. But I don't think it goes far enough, actually. This function actually has the ability to sidestep authorization to the Parameter Context itself. When a user sets a processor's properties, if any property references a parameter, we have an authorization check that the user making the change has read permissions to the parameter context. But consider if a user wanted to sidestep it using a value of:
```
${literal('#'):append('{'):append('myParameter'):append('}'):evaluateELString()}
```
Now, the framework has no way to detect that hey, this is actually referencing a parameter. So it doesn't do any validation checks. We need to completely disable this function from accessing any parameters at all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on pull request #6562: NIFI-10674 Block evaluateELString from reading Sensitive Parameters
Posted by GitBox <gi...@apache.org>.
exceptionfactory commented on PR #6562:
URL: https://github.com/apache/nifi/pull/6562#issuecomment-1287175965
Thanks for the feedback @markap14!
Good point about indirect parameter references, I agree that blocking all Parameter access from `evaluateELString()` provides the best solution.
I renamed the delegating `EvaluationContext` implementation to `ParametersDisabledEvaluationContext` and updated the unit tests for non-sensitive parameters.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org