> 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. > > Cetin Sahin wrote: > 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.
Thanks Cetin. Sorry for the delayed response. I agree it would be computationally heavier, my suggestion is to go with that anyway since Coverity/Fortify scans might flag any MD5 usage as potential issue. - Velmurugan ----------------------------------------------------------- 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 > >
