search

linux-application-whitelisting / fapolicyd

File Access Policy Daemon
GNU General Public License v3.0
192 stars 55 forks source link

Extract MD5 logic for reuse #277

Closed Kangie closed 8 months ago

Kangie commented 8 months ago

Gentoo's portage also uses MD5s to track which files have changed since installation.

Added some debug statements for later.

Kangie commented 8 months ago

~I'm going to have to rely on CI/CD to make sure that I didn't break anything on *buntu, I can't get Gentoo's dpkg package to spit out a libdpkg that we can link against. Changes are pretty innocuous IMO.~

I'll spin up a build container at least to catch any warnings unique to the deb path.

stevegrubb commented 8 months ago

OK, looking at the change to configure.ac, it adds AC_USE_SYSTEM_EXTENSIONS which was introduced in autoconf-2.60. But we are declaring conformance to 2.50. As far as I know, fapolicy only runs on Linux. So, we can add _GNU_SOURCE on a case by case basis. until there's a need to rebase to a newer autoconf. Looks like RHEL 8 has autoconf-2.69 available. I don't know about other Linux distributions. Still reviewing...

stevegrubb commented 8 months ago

It's probably better to include uthash.h only in the .c files that call HASH_* functions.

test_file() leaks the fd it opens. After fstat() it could close it's fd, but the next thing that happens if test_file returns success is it re-opens the path. It would be better to open it once and pass the fd to test_file. But don't close it in test_file, do that in the same function it's opened in for clarity. That said, the stat buffer should also have the file size in the st_size member which could avoid the need to call to lseek. But if you do that, maybe it's better not to separate the code in test_file() from add_file_to_backend_by_md5() ?

Kangie commented 8 months ago

Looks like RHEL 8 has autoconf-2.69 available.

2.60 came out in 2006, while kernel 4.20 came out in Dec 2018; I think we're fine here—I can't imagine a reasonable system running a kernel that supports FANOTIFY and not having 2.60. I will update that to target 2.60 though if you're happy with that?

... But if you do that, maybe it's better not to separate the code in test_file() from add_file_to_backend_by_md5() ?

I'll rework that, thanks.

stevegrubb commented 8 months ago

Thanks. Looks good to me.

Kangie commented 8 months ago

I'm happy if you are. I haven't addressed your feedback about putting the uthash stuff in each C file that calls it; ran out of time before work - If that's fine, great, if not I'm equally happy to make those changes :smile:

Edit: Latest force-push is a whitespace fix in md5-backend.h that was supposed to make it last time...