Re: Review Request 67694: RANGER-2139: UnixUserGroupBuilder fails to detect consecutive updates on UNIX passwd and group files
> On June 21, 2018, 7:44 p.m., Velmurugan Periasamy wrote: > > ugsync/src/main/java/org/apache/ranger/unixusersync/process/UnixUserGroupBuilder.java > > Lines 543 (patched) > > <https://reviews.apache.org/r/67694/diff/1/?file=2044228#file2044228line543> > > > > MD5 is not recommended anymore. > > Allen Wittenauer wrote: > Agree that MD5 shouldn't be used for security purposes, but that isn't > the use case here. Instead, it is only used to generate a simple checksum. > Using a more complex (and therefore more CPU intensive) hashing function > doesn't have much value. If someone were to replace /etc/passwd with a file > that had an MD5 collision (the reason why MD5 shouldn't be used in the > majority of use cases) it would defeat the purpose; this code is only > triggered when the MD5s do not match. > > Velmurugan Periasamy wrote: > It is a good idea to use something like sha256Hex, so that source code > analysis tools such as coverity/fortify do not flag md5Hex usage as > vulnerable. Dear Velmurugan and Allen, Thanks for your comments. I also do not think using MD5 impose any security risk in this context, but I will be happy to replace the digestion algorithm with a more stronger one if you think it is really necessary. At first place, I was more concerned about the performance rather than the security of /etc/passwd or /etc/group files. If UnixUserGroupBuilder checks the update too frequently (which is configurable in the Ranger context), using more secure digestion algoritm like SHA256 will be computationally heavier without any additional functional benefits. - Cetin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67694/#review205204 --- On June 21, 2018, 7:16 p.m., Cetin Sahin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67694/ > --- > > (Updated June 21, 2018, 7:16 p.m.) > > > Review request for ranger. > > > Bugs: RANGER-2139 > https://issues.apache.org/jira/browse/RANGER-2139 > > > Repository: ranger > > > Description > --- > > Fixed the update detection issue on consecutive updates in > UnixUserGroupBuilder. The update detection logic is improved by verifying the > checksums in addition to last modification time. > > > Diffs > - > > > ugsync/src/main/java/org/apache/ranger/unixusersync/process/UnixUserGroupBuilder.java > ddab6294a > > > Diff: https://reviews.apache.org/r/67694/diff/1/ > > > Testing > --- > > 1. Applied the patch to the master branch and verified that all unit tests > passed successfully. > > > Thanks, > > Cetin Sahin > >
[jira] [Updated] (RANGER-2139) UnixUserGroupBuilder fails to detect consecutive updates on UNIX passwd and group files
[
https://issues.apache.org/jira/browse/RANGER-2139?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Cetin Sahin updated RANGER-2139:
Affects Version/s: 0.7.1
> UnixUserGroupBuilder fails to detect consecutive updates on UNIX passwd and
> group files
> ---
>
> Key: RANGER-2139
> URL: https://issues.apache.org/jira/browse/RANGER-2139
> Project: Ranger
> Issue Type: Bug
> Components: usersync
>Affects Versions: 1.0.0, master, 0.7.1
>Reporter: Cetin Sahin
>Priority: Critical
> Fix For: master
>
> Attachments: 0001-RANGER-2139-Fix-UnixUserGroupBuilder.patch
>
>
> When Unix based user and group synchronization is enabled in Ranger,
> UnixUserGroupBuilder periodically checks whether one of the /etc/passwd or
> /etc/group files is modified or not to trigger a synchronization.
> However, while checking the modification of a file, the UnixUserGroupBuilder
> uses java.io.File.lastModified() to check whether the file is updated after
> the latest synchronization time.
> {code:java}
> long TempGroupFileModifiedAt = new File(unixGroupFile).lastModified();
> {code}
> java.io.File.lastModified() function, however, returns the latest modified
> timestamp in the time granularity of seconds. That means each timestamp ends
> with 000 independent of the millisecond precision.
> [https://bugs.openjdk.java.net/browse/JDK-8177809]
>
> [http://dev-answers.blogspot.com/2014/11/avoid-using-javaiofilelastmodified-for.html]
> This can cause UnixUserGroupBuilder to fail to detect the update on the file
> if the file modification check happens between the two consecutive updates
> within the same second. Assume the following scenario with the corresponding
> timestamps where UnixUserGroupBuilder checks the updates per minute.
> the latest modification of users and group files are at t0 (00:00:00.111),
> which have a corresponding timestamp of 1529539200111, denoted by T0
> Now, consider the following scenario.
> * At time t1 (01:*00:00.123*), T1 (1529542800123): /etc/group file is
> updated and a new group called group01 is added.
> * At time t2 (01:*00:00.345*), T2 (1529542800345): UnixUserGroupBuilder
> threads wakes up and detects the update on the group file and performed the
> synchronization. After the synchronization, the latest modification time for
> the group is updated from the File.lastModified() function. latest
> modification of group file = File.lastModified(t1) = *152954280*. Please
> note that the last 3 digits corresponding to the milliseconds is truncated to
> 000 with File.lastModified() function.
> * At time t3 (01:*00:00.567*), T3 (1529542800567): /etc/group file is
> updated and a user membership is added to one of the groups (e.g., user
> user01 becomes a member of group group01).
> * At time t4 (01:*01:00.345*), T4 (1529542860345): UnixUserGroupBuilder
> thread wakes up and couldn't detect any changes since the timestamp generated
> from the File.lastModified() function returns the same timestamp for t1 and
> t3. Recall that the latest modification time of the group file becomes
> 152954280 at t2 and File.lastModified(t3) returns *152954280* as
> well. Since both File.lastModified(t1) = File.lastModified(t3),
> UnixUserGroupBuilder could not detect the modification on the file at t4,
> assumes there is no update, and then, sleeps again without syncing the
> changes.
> At time t4, UnixUserGroupBuilder is supposed to sync the user group
> membership but if fails to detect the update. If there is no any further
> update on one of these files, the user01 will never be part of group01 in
> Ranger.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Comment Edited] (RANGER-2139) UnixUserGroupBuilder fails to detect consecutive updates on UNIX passwd and group files
[
https://issues.apache.org/jira/browse/RANGER-2139?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16519700#comment-16519700
]
Cetin Sahin edited comment on RANGER-2139 at 6/21/18 7:25 PM:
--
Requested Review: [RANGER-2139: UnixUserGroupBuilder fails to detect
consecutive updates on UNIX passwd and group
files|https://reviews.apache.org/r/67694/]
[^0001-RANGER-2139-Fix-UnixUserGroupBuilder.patch]
was (Author: cetinsahin):
[^0001-RANGER-2139-Fix-UnixUserGroupBuilder.patch]
> UnixUserGroupBuilder fails to detect consecutive updates on UNIX passwd and
> group files
> ---
>
> Key: RANGER-2139
> URL: https://issues.apache.org/jira/browse/RANGER-2139
> Project: Ranger
> Issue Type: Bug
> Components: usersync
>Affects Versions: 1.0.0, master
>Reporter: Cetin Sahin
>Priority: Critical
> Fix For: master
>
> Attachments: 0001-RANGER-2139-Fix-UnixUserGroupBuilder.patch
>
>
> When Unix based user and group synchronization is enabled in Ranger,
> UnixUserGroupBuilder periodically checks whether one of the /etc/passwd or
> /etc/group files is modified or not to trigger a synchronization.
> However, while checking the modification of a file, the UnixUserGroupBuilder
> uses java.io.File.lastModified() to check whether the file is updated after
> the latest synchronization time.
> {code:java}
> long TempGroupFileModifiedAt = new File(unixGroupFile).lastModified();
> {code}
> java.io.File.lastModified() function, however, returns the latest modified
> timestamp in the time granularity of seconds. That means each timestamp ends
> with 000 independent of the millisecond precision.
> [https://bugs.openjdk.java.net/browse/JDK-8177809]
>
> [http://dev-answers.blogspot.com/2014/11/avoid-using-javaiofilelastmodified-for.html]
> This can cause UnixUserGroupBuilder to fail to detect the update on the file
> if the file modification check happens between the two consecutive updates
> within the same second. Assume the following scenario with the corresponding
> timestamps where UnixUserGroupBuilder checks the updates per minute.
> the latest modification of users and group files are at t0 (00:00:00.111),
> which have a corresponding timestamp of 1529539200111, denoted by T0
> Now, consider the following scenario.
> * At time t1 (01:*00:00.123*), T1 (1529542800123): /etc/group file is
> updated and a new group called group01 is added.
> * At time t2 (01:*00:00.345*), T2 (1529542800345): UnixUserGroupBuilder
> threads wakes up and detects the update on the group file and performed the
> synchronization. After the synchronization, the latest modification time for
> the group is updated from the File.lastModified() function. latest
> modification of group file = File.lastModified(t1) = *152954280*. Please
> note that the last 3 digits corresponding to the milliseconds is truncated to
> 000 with File.lastModified() function.
> * At time t3 (01:*00:00.567*), T3 (1529542800567): /etc/group file is
> updated and a user membership is added to one of the groups (e.g., user
> user01 becomes a member of group group01).
> * At time t4 (01:*01:00.345*), T4 (1529542860345): UnixUserGroupBuilder
> thread wakes up and couldn't detect any changes since the timestamp generated
> from the File.lastModified() function returns the same timestamp for t1 and
> t3. Recall that the latest modification time of the group file becomes
> 152954280 at t2 and File.lastModified(t3) returns *152954280* as
> well. Since both File.lastModified(t1) = File.lastModified(t3),
> UnixUserGroupBuilder could not detect the modification on the file at t4,
> assumes there is no update, and then, sleeps again without syncing the
> changes.
> At time t4, UnixUserGroupBuilder is supposed to sync the user group
> membership but if fails to detect the update. If there is no any further
> update on one of these files, the user01 will never be part of group01 in
> Ranger.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
[jira] [Updated] (RANGER-2139) UnixUserGroupBuilder fails to detect consecutive updates on UNIX passwd and group files
[
https://issues.apache.org/jira/browse/RANGER-2139?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Cetin Sahin updated RANGER-2139:
Attachment: 0001-RANGER-2139-Fix-UnixUserGroupBuilder.patch
> UnixUserGroupBuilder fails to detect consecutive updates on UNIX passwd and
> group files
> ---
>
> Key: RANGER-2139
> URL: https://issues.apache.org/jira/browse/RANGER-2139
> Project: Ranger
> Issue Type: Bug
> Components: usersync
>Affects Versions: 1.0.0, master
>Reporter: Cetin Sahin
>Priority: Critical
> Attachments: 0001-RANGER-2139-Fix-UnixUserGroupBuilder.patch
>
>
> When Unix based user and group synchronization is enabled in Ranger,
> UnixUserGroupBuilder periodically checks whether one of the /etc/passwd or
> /etc/group files is modified or not to trigger a synchronization.
> However, while checking the modification of a file, the UnixUserGroupBuilder
> uses java.io.File.lastModified() to check whether the file is updated after
> the latest synchronization time.
> {code:java}
> long TempGroupFileModifiedAt = new File(unixGroupFile).lastModified();
> {code}
> java.io.File.lastModified() function, however, returns the latest modified
> timestamp in the time granularity of seconds. That means each timestamp ends
> with 000 independent of the millisecond precision.
> [https://bugs.openjdk.java.net/browse/JDK-8177809]
>
> [http://dev-answers.blogspot.com/2014/11/avoid-using-javaiofilelastmodified-for.html]
> This can cause UnixUserGroupBuilder to fail to detect the update on the file
> if the file modification check happens between the two consecutive updates
> within the same second. Assume the following scenario with the corresponding
> timestamps where UnixUserGroupBuilder checks the updates per minute.
> the latest modification of users and group files are at t0 (00:00:00.111),
> which have a corresponding timestamp of 1529539200111, denoted by T0
> Now, consider the following scenario.
> * At time t1 (01:*00:00.123*), T1 (1529542800123): /etc/group file is
> updated and a new group called group01 is added.
> * At time t2 (01:*00:00.345*), T2 (1529542800345): UnixUserGroupBuilder
> threads wakes up and detects the update on the group file and performed the
> synchronization. After the synchronization, the latest modification time for
> the group is updated from the File.lastModified() function. latest
> modification of group file = File.lastModified(t1) = *152954280*. Please
> note that the last 3 digits corresponding to the milliseconds is truncated to
> 000 with File.lastModified() function.
> * At time t3 (01:*00:00.567*), T3 (1529542800567): /etc/group file is
> updated and a user membership is added to one of the groups (e.g., user
> user01 becomes a member of group group01).
> * At time t4 (01:*01:00.345*), T4 (1529542860345): UnixUserGroupBuilder
> thread wakes up and couldn't detect any changes since the timestamp generated
> from the File.lastModified() function returns the same timestamp for t1 and
> t3. Recall that the latest modification time of the group file becomes
> 152954280 at t2 and File.lastModified(t3) returns *152954280* as
> well. Since both File.lastModified(t1) = File.lastModified(t3),
> UnixUserGroupBuilder could not detect the modification on the file at t4,
> assumes there is no update, and then, sleeps again without syncing the
> changes.
> At time t4, UnixUserGroupBuilder is supposed to sync the user group
> membership but if fails to detect the update. If there is no any further
> update on one of these files, the user01 will never be part of group01 in
> Ranger.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
Review Request 67694: RANGER-2139: UnixUserGroupBuilder fails to detect consecutive updates on UNIX passwd and group files
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67694/ --- Review request for ranger. Bugs: RANGER-2139 https://issues.apache.org/jira/browse/RANGER-2139 Repository: ranger Description --- Fixed the update detection issue on consecutive updates in UnixUserGroupBuilder. The update detection logic is improved by verifying the checksums in addition to last modification time. Diffs - ugsync/src/main/java/org/apache/ranger/unixusersync/process/UnixUserGroupBuilder.java ddab6294a Diff: https://reviews.apache.org/r/67694/diff/1/ Testing --- 1. Applied the patch to the master branch and verified that all unit tests passed successfully. Thanks, Cetin Sahin
[jira] [Created] (RANGER-2139) UnixUserGroupBuilder fails to detect consecutive updates on UNIX passwd and group files
Cetin Sahin created RANGER-2139:
---
Summary: UnixUserGroupBuilder fails to detect consecutive updates
on UNIX passwd and group files
Key: RANGER-2139
URL: https://issues.apache.org/jira/browse/RANGER-2139
Project: Ranger
Issue Type: Bug
Components: usersync
Affects Versions: 1.0.0, master
Reporter: Cetin Sahin
When Unix based user and group synchronization is enabled in Ranger,
UnixUserGroupBuilder periodically checks whether one of the /etc/passwd or
/etc/group files is modified or not to trigger a synchronization.
However, while checking the modification of a file, the UnixUserGroupBuilder
uses java.io.File.lastModified() to check whether the file is updated after the
latest synchronization time.
{code:java}
long TempGroupFileModifiedAt = new File(unixGroupFile).lastModified();
{code}
java.io.File.lastModified() function, however, returns the latest modified
timestamp in the time granularity of seconds. That means each timestamp ends
with 000 independent of the millisecond precision.
[https://bugs.openjdk.java.net/browse/JDK-8177809]
[http://dev-answers.blogspot.com/2014/11/avoid-using-javaiofilelastmodified-for.html]
This can cause UnixUserGroupBuilder to fail to detect the update on the file if
the file modification check happens between the two consecutive updates within
the same second. Assume the following scenario with the corresponding
timestamps where UnixUserGroupBuilder checks the updates per minute.
the latest modification of users and group files are at t0 (00:00:00.111),
which have a corresponding timestamp of 1529539200111, denoted by T0
Now, consider the following scenario.
* At time t1 (01:*00:00.123*), T1 (1529542800123): /etc/group file is updated
and a new group called group01 is added.
* At time t2 (01:*00:00.345*), T2 (1529542800345): UnixUserGroupBuilder
threads wakes up and detects the update on the group file and performed the
synchronization. After the synchronization, the latest modification time for
the group is updated from the File.lastModified() function. latest modification
of group file = File.lastModified(t1) = *152954280*. Please note that the
last 3 digits corresponding to the milliseconds is truncated to 000 with
File.lastModified() function.
* At time t3 (01:*00:00.567*), T3 (1529542800567): /etc/group file is updated
and a user membership is added to one of the groups (e.g., user user01 becomes
a member of group group01).
* At time t4 (01:*01:00.345*), T4 (1529542860345): UnixUserGroupBuilder thread
wakes up and couldn't detect any changes since the timestamp generated from the
File.lastModified() function returns the same timestamp for t1 and t3. Recall
that the latest modification time of the group file becomes 152954280 at t2
and File.lastModified(t3) returns *152954280* as well. Since both
File.lastModified(t1) = File.lastModified(t3), UnixUserGroupBuilder could not
detect the modification on the file at t4, assumes there is no update, and
then, sleeps again without syncing the changes.
At time t4, UnixUserGroupBuilder is supposed to sync the user group membership
but if fails to detect the update. If there is no any further update on one of
these files, the user01 will never be part of group01 in Ranger.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
