You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ranger.apache.org by "sneethiraj (via GitHub)" <gi...@apache.org> on 2026/04/01 04:44:49 UTC
[PR] RANGER-5534: added validation logic for username and groupname for usersync process [ranger]
sneethiraj opened a new pull request, #903:
URL: https://github.com/apache/ranger/pull/903
## What changes were proposed in this pull request?
* Added username and groupname validation based on regEx
* Configuration - userName validation enabled [true/false]
* Configuration - groupName validation enabled [true/false]
* Configuration - RegEx for the username validation [default: "^[a-z_][a-z0-9_-]{0,31}$" ]
* Configuration - RegEx for the groupname validation [default: "^[a-z][a-z0-9-]{0,30}$" ]
## How was this patch tested?
Tested locally on Mac
--
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: dev-unsubscribe@ranger.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RANGER-5534: added validation logic for username and groupname for usersync process [ranger]
Posted by "sneethiraj (via GitHub)" <gi...@apache.org>.
sneethiraj commented on code in PR #903:
URL: https://github.com/apache/ranger/pull/903#discussion_r3023478030
##########
ugsync/src/main/java/org/apache/ranger/unixusersync/process/UnixUserGroupBuilder.java:
##########
@@ -330,6 +340,13 @@ private void buildUnixUserList(String command) throws Throwable {
continue;
}
+ if (validateUserName) {
+ if (!isValidUserName(userName)) {
+ LOG.warn("Ignoring Unix Username: [{}]: failed to confirm to validation-pattern: [{}]", userName, regExUserNameValidator);
Review Comment:
@vyommani - Thanks for the above fix. That takes care of the core issue on those executions. Even with the above fix, the calls like one below exists and the command was formed by groupId and/or groupName that are read from different source / unvalidated.
`String[] cmd = new String[] {"bash", "-c", command };`
Shouldn't we need to validate before it is executed directly?
On a different note, have you looked at the above REGEX that supports unicode based characters in username/groups? Do you have any other alternative options to address this issue?
If the customer do not want to validate, they can disable the validation via configuration or set the regEx that suites their needs.
What do you think ?
--
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: dev-unsubscribe@ranger.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RANGER-5534: added validation logic for username and groupname for usersync process [ranger]
Posted by "sneethiraj (via GitHub)" <gi...@apache.org>.
sneethiraj closed pull request #903: RANGER-5534: added validation logic for username and groupname for usersync process
URL: https://github.com/apache/ranger/pull/903
--
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: dev-unsubscribe@ranger.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RANGER-5534: added validation logic for username and groupname for usersync process [ranger]
Posted by "vyommani (via GitHub)" <gi...@apache.org>.
vyommani commented on code in PR #903:
URL: https://github.com/apache/ranger/pull/903#discussion_r3023292236
##########
ugsync/src/main/java/org/apache/ranger/unixusersync/process/UnixUserGroupBuilder.java:
##########
@@ -330,6 +340,13 @@ private void buildUnixUserList(String command) throws Throwable {
continue;
}
+ if (validateUserName) {
+ if (!isValidUserName(userName)) {
+ LOG.warn("Ignoring Unix Username: [{}]: failed to confirm to validation-pattern: [{}]", userName, regExUserNameValidator);
Review Comment:
Hi,
I had already addressed this vulnerability in an earlier change by updating the command execution from: {"bash", "-c", "id -G " + userName}
to the safer:
{"id", "-G", userName}
This change ensures that the shell is never invoked. As a result, metacharacters in the username have no special meaning at the OS execve() level, which fully mitigates the command injection risk.
The validation logic proposed in this PR is a good attempt to add defense-in-depth. However, I'm concerned that the regex-based approach (even with the updated broader patterns) could unintentionally reject many valid usernames — for example, legitimate usernames written in Korean, Japanese, Chinese, or other languages that use non-ASCII characters.
Could you please take a look at the fix in RANGER-5508 and let me know what you think about the above analysis? If this turns out to be a completely unrelated issue, please feel free to let me know as well.
--
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: dev-unsubscribe@ranger.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RANGER-5534: added validation logic for username and groupname for usersync process [ranger]
Posted by "sneethiraj (via GitHub)" <gi...@apache.org>.
sneethiraj commented on code in PR #903:
URL: https://github.com/apache/ranger/pull/903#discussion_r3020012221
##########
ugsync/src/main/java/org/apache/ranger/unixusersync/process/UnixUserGroupBuilder.java:
##########
@@ -330,6 +340,13 @@ private void buildUnixUserList(String command) throws Throwable {
continue;
}
+ if (validateUserName) {
+ if (!isValidUserName(userName)) {
+ LOG.warn("Ignoring Unix Username: [{}]: failed to confirm to validation-pattern: [{}]", userName, regExUserNameValidator);
Review Comment:
**Good catch**. We can implement the following regex validations that supports international char sets:
```
private static final String DEFAULT_REGEX_USERNAME_VALIDATOR = "^[\\p{L}\\p{Mn}\\p{Nd}\\._-]{3,64}$" ;
private static final String DEFAULT_REGEX_GROUPNAME_VALIDATOR = "^[\\p{L}\\p{N}._\\-\\s]{1,256}$" ;
```
However, we need to document this as part of the upgrade to 2.8.1+ release that the username and groupname validations are defaulted to be ON with specific regex and customers are requested to review and modify if needed before deploying newer version of code.
Can you share your thoughts on the best approach to resolve this?
--
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: dev-unsubscribe@ranger.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RANGER-5534: added validation logic for username and groupname for usersync process [ranger]
Posted by "sneethiraj (via GitHub)" <gi...@apache.org>.
sneethiraj commented on PR #903:
URL: https://github.com/apache/ranger/pull/903#issuecomment-4317100728
Closing this PR to resolve this via different approach.
--
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: dev-unsubscribe@ranger.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RANGER-5534: added validation logic for username and groupname for usersync process [ranger]
Posted by "vyommani (via GitHub)" <gi...@apache.org>.
vyommani commented on code in PR #903:
URL: https://github.com/apache/ranger/pull/903#discussion_r3025715075
##########
ugsync/src/main/java/org/apache/ranger/unixusersync/process/UnixUserGroupBuilder.java:
##########
@@ -330,6 +340,13 @@ private void buildUnixUserList(String command) throws Throwable {
continue;
}
+ if (validateUserName) {
+ if (!isValidUserName(userName)) {
+ LOG.warn("Ignoring Unix Username: [{}]: failed to confirm to validation-pattern: [{}]", userName, regExUserNameValidator);
Review Comment:
I had already addressed this issue in an earlier change by updating the command execution from:
{"bash", "-c", "id -G " + userName}
to the safer:
{"id", "-G", userName}
This change ensures that the shell is never invoked. As a result, metacharacters in the username have no special meaning at the OS execve() level, which fully solve the issue.
The validation logic proposed in this PR is a good validation. However, I'm concerned that the regex-based approach (even with the updated broader patterns) could unintentionally reject many valid usernames — for example, legitimate usernames written in Korean, Japanese, Chinese, or other languages that use non-ASCII characters.
Could you please take a look at the fix in RANGER-5508 and let me know what you think about the above analysis? If this turns out to be a completely unrelated issue, please feel free to let me know as well.
--
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: dev-unsubscribe@ranger.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RANGER-5534: added validation logic for username and groupname for usersync process [ranger]
Posted by "vyommani (via GitHub)" <gi...@apache.org>.
vyommani commented on code in PR #903:
URL: https://github.com/apache/ranger/pull/903#discussion_r3025715075
##########
ugsync/src/main/java/org/apache/ranger/unixusersync/process/UnixUserGroupBuilder.java:
##########
@@ -330,6 +340,13 @@ private void buildUnixUserList(String command) throws Throwable {
continue;
}
+ if (validateUserName) {
+ if (!isValidUserName(userName)) {
+ LOG.warn("Ignoring Unix Username: [{}]: failed to confirm to validation-pattern: [{}]", userName, regExUserNameValidator);
Review Comment:
I had already addressed this issue in an earlier change by updating the command execution from:
{"bash", "-c", "id -G " + userName}
to the safer:
{"id", "-G", userName}
This change ensures that the shell is never invoked. As a result, metacharacters in the username have no special meaning at the OS execve() level, which fully solve the issue.
The validation logic proposed in this PR is a good attempt at adding defense-in-depth. However, I'm concerned that the regex-based approach (even with the updated broader patterns) could unintentionally reject many valid usernames — for example, legitimate usernames written in Korean, Japanese, Chinese, or other languages that use non-ASCII characters.
Could you please take a look at the fix in RANGER-5508 and let me know what you think about the above analysis? If this turns out to be a completely unrelated issue, please feel free to let me know as well.
--
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: dev-unsubscribe@ranger.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RANGER-5534: added validation logic for username and groupname for usersync process [ranger]
Posted by "vyommani (via GitHub)" <gi...@apache.org>.
vyommani commented on code in PR #903:
URL: https://github.com/apache/ranger/pull/903#discussion_r3025715075
##########
ugsync/src/main/java/org/apache/ranger/unixusersync/process/UnixUserGroupBuilder.java:
##########
@@ -330,6 +340,13 @@ private void buildUnixUserList(String command) throws Throwable {
continue;
}
+ if (validateUserName) {
+ if (!isValidUserName(userName)) {
+ LOG.warn("Ignoring Unix Username: [{}]: failed to confirm to validation-pattern: [{}]", userName, regExUserNameValidator);
Review Comment:
I had already addressed this issue in an earlier change by updating the command execution from:
{"bash", "-c", "id -G " + userName}
to the safer:
{"id", "-G", userName}
This change ensures that the shell is never invoked. As a result, metacharacters in the username have no special meaning at the OS execve() level, which fully solve the issue.
The validation logic proposed in this PR is a good validation. However, I'm concerned that the regex-based approach (even with the updated broader patterns) could unintentionally reject many valid usernames — for example, legitimate usernames written in Korean, Japanese, Chinese, or other languages that use non-ASCII characters.
Could you please take a look at the fix in RANGER-5508 and let me know what you think about the above analysis? If this turns out to be a completely unrelated issue, please feel free to let me know as well.
--
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: dev-unsubscribe@ranger.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RANGER-5534: added validation logic for username and groupname for usersync process [ranger]
Posted by "sneethiraj (via GitHub)" <gi...@apache.org>.
sneethiraj commented on code in PR #903:
URL: https://github.com/apache/ranger/pull/903#discussion_r3023478030
##########
ugsync/src/main/java/org/apache/ranger/unixusersync/process/UnixUserGroupBuilder.java:
##########
@@ -330,6 +340,13 @@ private void buildUnixUserList(String command) throws Throwable {
continue;
}
+ if (validateUserName) {
+ if (!isValidUserName(userName)) {
+ LOG.warn("Ignoring Unix Username: [{}]: failed to confirm to validation-pattern: [{}]", userName, regExUserNameValidator);
Review Comment:
@vyommani - Thanks for the above fix. That takes care of the core issue on those executions. Even with the above fix, the calls like one below exists and the command was formed by groupId and/or groupName that are read from different source / unvalidated.
`String[] cmd = new String[] {"bash", "-c", command };`
Shouldn't we need to validate before it is executed directly?
On a different note, have you looked at the above REGEX that supports unicode based characters in username/groups? Do you have any other alternative options to address this issue?
If the customer do not want to validate, they can disable the validation via configuration or set the regEx that suites their needs.
What do you think ?
--
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: dev-unsubscribe@ranger.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RANGER-5534: added validation logic for username and groupname for usersync process [ranger]
Posted by "vyommani (via GitHub)" <gi...@apache.org>.
vyommani commented on code in PR #903:
URL: https://github.com/apache/ranger/pull/903#discussion_r3025715075
##########
ugsync/src/main/java/org/apache/ranger/unixusersync/process/UnixUserGroupBuilder.java:
##########
@@ -330,6 +340,13 @@ private void buildUnixUserList(String command) throws Throwable {
continue;
}
+ if (validateUserName) {
+ if (!isValidUserName(userName)) {
+ LOG.warn("Ignoring Unix Username: [{}]: failed to confirm to validation-pattern: [{}]", userName, regExUserNameValidator);
Review Comment:
I had already addressed this vulnerability in an earlier change by updating the command execution from:
{"bash", "-c", "id -G " + userName}
to the safer:
{"id", "-G", userName}
This change ensures that the shell is never invoked. As a result, metacharacters in the username have no special meaning at the OS execve() level, which fully mitigates the command injection risk.
The validation logic proposed in this PR is a good attempt at adding defense-in-depth. However, I'm concerned that the regex-based approach (even with the updated broader patterns) could unintentionally reject many valid usernames — for example, legitimate usernames written in Korean, Japanese, Chinese, or other languages that use non-ASCII characters.
Could you please take a look at the fix in RANGER-5508 and let me know what you think about the above analysis? If this turns out to be a completely unrelated issue, please feel free to let me know as well.
--
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: dev-unsubscribe@ranger.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RANGER-5534: added validation logic for username and groupname for usersync process [ranger]
Posted by "mneethiraj (via GitHub)" <gi...@apache.org>.
mneethiraj commented on code in PR #903:
URL: https://github.com/apache/ranger/pull/903#discussion_r3019809025
##########
ugsync/src/main/java/org/apache/ranger/unixusersync/process/UnixUserGroupBuilder.java:
##########
@@ -330,6 +340,13 @@ private void buildUnixUserList(String command) throws Throwable {
continue;
}
+ if (validateUserName) {
+ if (!isValidUserName(userName)) {
+ LOG.warn("Ignoring Unix Username: [{}]: failed to confirm to validation-pattern: [{}]", userName, regExUserNameValidator);
Review Comment:
@sneethiraj - this could result in usernames having valid non-English characters (accent characters, CJK character sets) to be skipped with default configuration. This can break environments already have such user names and group names.
--
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: dev-unsubscribe@ranger.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RANGER-5534: added validation logic for username and groupname for usersync process [ranger]
Posted by "vyommani (via GitHub)" <gi...@apache.org>.
vyommani commented on code in PR #903:
URL: https://github.com/apache/ranger/pull/903#discussion_r3023292236
##########
ugsync/src/main/java/org/apache/ranger/unixusersync/process/UnixUserGroupBuilder.java:
##########
@@ -330,6 +340,13 @@ private void buildUnixUserList(String command) throws Throwable {
continue;
}
+ if (validateUserName) {
+ if (!isValidUserName(userName)) {
+ LOG.warn("Ignoring Unix Username: [{}]: failed to confirm to validation-pattern: [{}]", userName, regExUserNameValidator);
Review Comment:
Hi,
I had already addressed this vulnerability in an earlier change by updating the command execution from: {"bash", "-c", "id -G " + userName}
to the safer:
{"id", "-G", userName}
This change ensures that the shell is never invoked. As a result, metacharacters in the username have no special meaning at the OS execve() level, which fully mitigates the command injection risk.
The validation logic proposed in this PR is a good attempt to add defense-in-depth. However, I'm concerned that the regex-based approach (even with the updated broader patterns) could unintentionally reject many valid usernames — for example, legitimate usernames written in Korean, Japanese, Chinese, or other languages that use non-ASCII characters.
Could you please take a look at the fix in RANGER-5508 and let me know what you think about the above analysis? If this turns out to be a completely unrelated issue, please feel free to let me know as well.
--
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: dev-unsubscribe@ranger.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] RANGER-5534: added validation logic for username and groupname for usersync process [ranger]
Posted by "vyommani (via GitHub)" <gi...@apache.org>.
vyommani commented on code in PR #903:
URL: https://github.com/apache/ranger/pull/903#discussion_r3026024559
##########
ugsync/src/main/java/org/apache/ranger/unixusersync/process/UnixUserGroupBuilder.java:
##########
@@ -330,6 +340,13 @@ private void buildUnixUserList(String command) throws Throwable {
continue;
}
+ if (validateUserName) {
+ if (!isValidUserName(userName)) {
+ LOG.warn("Ignoring Unix Username: [{}]: failed to confirm to validation-pattern: [{}]", userName, regExUserNameValidator);
Review Comment:
I already fixed this issue as part of RANGER-5508 please check this fix and do let me know if it is completely different issue.
--
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: dev-unsubscribe@ranger.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org