Detect and send kernel metapackages by rptrchv · Pull Request #276 · WoTTsecurity/agent

rptrchv · 2020-03-09T07:12:01Z

Closes #268

vpetersson · 2020-03-09T08:23:18Z

@rptrchv Check the Codacity errors. It's complaining about code quality.

a-martynovich

There are many issues with this.
First, you use regexes for parsing the version from the package name. This will easily break because the package name may contain more dashes than your regex expects (look at https://packages.debian.org/jessie/linux-image-3.16.0-10-amd64 section "similar packages). Second, you're assuming that you can simply compare those packages by one number in their name, which will also break in many cases.

There is an easier and more robust solution. For every installed linux-image-... find its parent (the package which depends on it). Kernels which are not the latest version won't have a parent. Only the kernel of the latest version will have a parent, and that (or a level higher, if Ubuntu) is the meta-package. Also this way you can find out if the currently running kernel package does not match the meta-package, which means reboot is required (the new kernel was just installed).

rptrchv · 2020-03-12T12:11:53Z

@a-martynovich

First, you use regexes for parsing the version from the package name. This will easily break because the package name may contain more dashes than your regex expects (look at https://packages.debian.org/jessie/linux-image-3.16.0-10-amd64 section "similar packages).

Those kernels (that don't match my re pattern) are very exotic. Besides, this re pattern can be easily adjusted to support them too: r'(.+-\d+.\d+.\d+-)(\d+)([\.-].+)'.

Second, you're assuming that you can simply compare those packages by one number in their name, which will also break in many cases.

For kernels of all (available to me for check) platforms and distro versions it works. So this solution is not perfect (will not cover non-standard versions naming) but more than satisfactory.

There is an easier and more robust solution. For every installed linux-image-... find its parent (the package which depends on it). Kernels which are not the latest version won't have a parent. Only the kernel of the latest version will have a parent, and that (or a level higher, if Ubuntu) is the meta-package.

We can't simply check all installed "linux-image-..." packages because there might be installed several different kernel types on a machine, e.g. "linux-image-4.15.0-XXX-aws" and "linux-image-4.15.0-XXX-generic". So we need to check for parents existence only kernel image packages of the same type as our running kernel (that's what my "get_latest_same_kernel_deb" function does now). Which again relies on re patterns matching, which in turn is not 100% reliable (as you pointed out). So your proposed solution has the same potential issue as mine.

a-martynovich · 2020-03-13T08:42:11Z

@rptrchv I see your point and I can't come up with any easier solution now.
I won't ask you to write unit tests because I guess it's going to be more code than the real code itself. But please list which distros and which kernel packages you've tested on.

Closes WoTTsecurity#268

a-martynovich

Codacy shows 12 new issues, some of which you told me to correct in my PRs.
I suggest renaming is_amazon2() to smth like is_rhel() (and changing detection accordingly to match RHEL, CentOS and Amazon Linux) because the logic will most probably be the same for RHEL, CentOS and Amazon Linux when it comes to RPM packages.
Codacy shows 12 new issues, some of which you told me to correct in my PRs.

a-martynovich · 2020-03-19T11:24:16Z

-            # No systemd - probably yum-cron is not running
-            # TODO: use "service" executable which also works without systemd and on older systems
-            return False
+        from sh import systemctl


I wrote this so it wouldn't fail if running in container. Why did you change this?

I suggest renaming is_amazon2()

I don't agree. We can do that only after testing everything on RHEL and CentOS, not beforehand.

you replied to a different comment

a-martynovich · 2020-03-19T11:27:14Z

+            if match:
+                name_parts = match.groups()  # E.g. ('linux-image-4.4.0-', '174', '-generic')
+                latest_kernel_pkg = get_latest_same_kernel_deb(name_parts[0], name_parts[2])
+                return apt_pkg.version_compare(latest_kernel_pkg.installed.version, kernel_pkg.installed.version) > 0


why only newer? Can't we install older version? I think this should be != 0 i.e. not equal.

We need reboot only when we have a newer version of the running kernel package installed. That exactly what my code does. I don't catch your point.

a-martynovich · 2020-03-19T11:32:54Z

+        if kernel_pkg is not None:
+            match = DEBIAN_KERNEL_PKG_NAME_RE.match(kernel_pkg.name)
+            if match:
+                name_parts = match.groups()  # E.g. ('linux-image-4.4.0-', '174', '-generic')


How about we change this regex to linux-image-\d+\.\d+\.\d+\.-\d+-.+ since we actually expect it to look like that? This way we don't accidentally catch packets which we don't know.

a-martynovich · 2020-03-19T11:53:16Z

+            if __file__.encode() in package_header[rpm.RPMTAG_FILENAMES]:
+                return Installation.RPM
    # Other.
+    import agent


from agent import __version__?

a-martynovich · 2020-03-19T11:55:40Z

+                'version': kernel_pkg.installed.version,
+                'source_name': kernel_pkg.installed.source_name,
+                'source_version': kernel_pkg.installed.source_version,
+                'arch': kernel_pkg.installed.architecture


you don't need .installed because get_kernel_deb_package already checks for that (is_installed).

.installed here is a quite different thing

a-martynovich · 2020-03-19T12:07:24Z

+            match = DEBIAN_KERNEL_PKG_NAME_RE.match(kernel_pkg.name)
+            if match:
+                name_parts = match.groups()  # E.g. ('linux-image-4.4.0-', '174', '-generic')
+                latest_kernel_pkg = get_latest_same_kernel_deb(name_parts[0], name_parts[2])


this may break in case of linux-gcp to linux-gcp-edge upgrade. Basically it will be linux-image-5.0.0-xxxx-gcp -> linux-image-5.3.0-xxxx-gcp.

Yes. But it's not that simple to properly handle it. That's why I believe the major kernel version upgrade case is out of the scope of this task and deserves a separate task.

a-martynovich

I still do not agree with the way this task was done, but it should work, so I'm not holding it any longer.

vpetersson · 2020-03-19T16:08:35Z

@a-martynovich can you give me a tl;dr how you think this should be redesigned? In the meantime, I'm merging this.

a-martynovich · 2020-03-19T16:11:43Z

I think it should build a dependency tree from any known kernel meta-package down to a concrete kernel package and not rely on package names. This also has some edge cases, though.

vpetersson · 2020-03-19T16:21:45Z

Ok, I'll create a new ticket for that.

a-martynovich · 2020-03-19T16:23:45Z

@vpetersson we still need to expand this on Debian support, so yes, a ticket is needed.
On Debian everything is completely different, as we've recently learned.

cla-bot Bot added the cla-signed label Mar 9, 2020

rptrchv requested a review from a-martynovich March 9, 2020 08:08

a-martynovich suggested changes Mar 11, 2020

View reviewed changes

pythonpro added 9 commits March 18, 2020 11:35

Detect and send kernel metapacakages

3fafb7f

Closes WoTTsecurity#268

Fixed flake8 warning

cbc97f2

Fix Codacy warnings

adc9d8d

Fix one more Codacity warning

235476b

Added support of some known exotic kernel naming schemas

20d2f9f

Fixed newly found errors in RE pattern

d65c70f

Return not only meta-package's name but its full info

07ff5ca

Pass info about reboot required

253780b

Full support of Ubuntu and Amazon Linux 2

ffdeed3

rptrchv force-pushed the 750-show-kernel-meta-package branch from 69e04f3 to ffdeed3 Compare March 19, 2020 06:25

Fixed unused import

cf9f1a5

rptrchv requested a review from a-martynovich March 19, 2020 06:53

a-martynovich suggested changes Mar 19, 2020

View reviewed changes

a-martynovich reviewed Mar 19, 2020

View reviewed changes

pythonpro added 3 commits March 19, 2020 18:42

Post-review fixes/improvements

81bc6aa

Fixed 1 Codacy warning

3e0bfdf

2nd attempt to fix the same Codacy warning

2aad0be

a-martynovich self-requested a review March 19, 2020 16:01

a-martynovich approved these changes Mar 19, 2020

View reviewed changes

vpetersson merged commit 9e3184c into WoTTsecurity:master Mar 19, 2020

vpetersson mentioned this pull request Mar 19, 2020

Resolve kernel detection #283

Open

2 tasks

Uh oh!

Conversation

rptrchv commented Mar 9, 2020

Uh oh!

vpetersson commented Mar 9, 2020

Uh oh!

a-martynovich left a comment

Choose a reason for hiding this comment

Uh oh!

rptrchv commented Mar 12, 2020

Uh oh!

a-martynovich commented Mar 13, 2020

Uh oh!

a-martynovich left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a-martynovich Mar 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a-martynovich left a comment

Choose a reason for hiding this comment

Uh oh!

vpetersson commented Mar 19, 2020

Uh oh!

a-martynovich commented Mar 19, 2020

Uh oh!

vpetersson commented Mar 19, 2020

Uh oh!

a-martynovich commented Mar 19, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

a-martynovich left a comment •

edited

Loading

a-martynovich Mar 19, 2020 •

edited

Loading