Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Auditbeat] Package: Disable librpm signal handlers #10694

Merged
merged 7 commits into from
Feb 18, 2019

Conversation

cwurm
Copy link
Contributor

@cwurm cwurm commented Feb 12, 2019

Librpm installs its own signal handlers, preventing Beats from running its own Go handlers and causing an unclean shutdown. This summarily disables librpm's signal handlers. See #10633 (comment) for a detailed description of what is happening.

Resolves #10633.

@cwurm cwurm added bug review needs_backport PR is waiting to be backported to other branches. Auditbeat SecOps labels Feb 12, 2019
@cwurm cwurm requested a review from a team as a code owner February 12, 2019 13:05
@elasticmachine
Copy link
Collaborator

Pinging @elastic/secops

@cwurm cwurm force-pushed the package_disable_librpm_signal_handlers branch from 01eab52 to 77a855b Compare February 12, 2019 13:07
Copy link
Contributor

@adriansr adriansr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cwurm
Copy link
Contributor Author

cwurm commented Feb 18, 2019

I did some testing on Fedora 29 (which has a newer version of librpm) and noticed the code didn't work.

So I pushed two more commits:

  1. ae5006e to correctly disable librpm's signal traps by setting rpmsqSetInterruptSafety to false, i.e. 0.
  2. 5862064 When the tests still failed I looked further into librpm and noticed it's now using a thread-local variable to disable its signal traps (here). So I added runtime.LockOSThread/UnlockOSThread to make sure Go runs all librpm C calls in one data collection cycle in the same thread, and moved the disabling logic to that function as well.

Together with the changes in #10796 this should hopefully now work on all distros.

@adriansr do you mind taking another look?

Copy link
Contributor

@adriansr adriansr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 on the new changes

@cwurm cwurm merged commit 6fbcbff into elastic:master Feb 18, 2019
cwurm pushed a commit to cwurm/beats that referenced this pull request Feb 18, 2019
Disable librpm signal handlers.

Resolves elastic#10633.

(cherry picked from commit 6fbcbff)
@cwurm cwurm added v7.2.0 and removed needs_backport PR is waiting to be backported to other branches. labels Feb 18, 2019
cwurm pushed a commit to cwurm/beats that referenced this pull request Feb 18, 2019
Disable librpm signal handlers.

Resolves elastic#10633.

(cherry picked from commit 6fbcbff)
@cwurm cwurm added the v7.0.0 label Feb 18, 2019
@cwurm cwurm added the v6.7.0 label Feb 18, 2019
cwurm pushed a commit to cwurm/beats that referenced this pull request Feb 18, 2019
Disable librpm signal handlers.

Resolves elastic#10633.

(cherry picked from commit 6fbcbff)
cwurm pushed a commit that referenced this pull request Feb 19, 2019
… handlers (#10797)

Cherry-pick of PR #10694 to 7.x branch. Original message: 

Disable librpm signal handlers.

Resolves #10633.
cwurm pushed a commit that referenced this pull request Feb 19, 2019
… handlers (#10798)

* [Auditbeat] Package: Disable librpm signal handlers (#10694)

Disable librpm signal handlers.

Resolves #10633.

(cherry picked from commit 6fbcbff)
cwurm pushed a commit that referenced this pull request Feb 19, 2019
… handlers (#10799)

Cherry-pick of PR #10694 to 6.7 branch. Original message: 

Disable librpm signal handlers.

Resolves #10633.
cwurm pushed a commit that referenced this pull request Feb 20, 2019
…S 6.x, 7.x, and Fedora 29 (#10796)

Librpm version 4.14.2.1 on Fedora 29 no longer contains the `headerGetEntry` method we are currently using. It was deprecated and then removed in version 4.14 (rpm-software-management/rpm@c68fa9a).

Also, the much older version 4.8.0 of librpm on CentOS 6.10 (Final) does not yet contain newer data structures for tags like `rpm_tag_t/rpmTag/rpmTagVal`.

This PR makes two changes that should allow this code to work on all three distros (CentOS 6.x, 7.x, Fedora 29 - and hopefully anything in between):

1. Use `headerGetString/headerGetNumber` instead of `headerGetEntry`.
2. Use `int32_t` instead of `rpm_tag_t/rpmTag/rpmTagVal`. Luckily, this seems to work on all three distros. I'd prefer something like a typedef, but unfortunately, C99 does not allow repeating a typedef (C11 does) and so backporting them is not easily possible.

It also makes the code more lenient with errors during data collection: Only when no package name can be found do we return an error.

Together with #10694 this will hopefully allow RPM package collection to work well.
cwurm pushed a commit to cwurm/beats that referenced this pull request Feb 20, 2019
…S 6.x, 7.x, and Fedora 29 (elastic#10796)

Librpm version 4.14.2.1 on Fedora 29 no longer contains the `headerGetEntry` method we are currently using. It was deprecated and then removed in version 4.14 (rpm-software-management/rpm@c68fa9a).

Also, the much older version 4.8.0 of librpm on CentOS 6.10 (Final) does not yet contain newer data structures for tags like `rpm_tag_t/rpmTag/rpmTagVal`.

This PR makes two changes that should allow this code to work on all three distros (CentOS 6.x, 7.x, Fedora 29 - and hopefully anything in between):

1. Use `headerGetString/headerGetNumber` instead of `headerGetEntry`.
2. Use `int32_t` instead of `rpm_tag_t/rpmTag/rpmTagVal`. Luckily, this seems to work on all three distros. I'd prefer something like a typedef, but unfortunately, C99 does not allow repeating a typedef (C11 does) and so backporting them is not easily possible.

It also makes the code more lenient with errors during data collection: Only when no package name can be found do we return an error.

Together with elastic#10694 this will hopefully allow RPM package collection to work well.

(cherry picked from commit e7ea5d7)
cwurm pushed a commit to cwurm/beats that referenced this pull request Feb 20, 2019
…S 6.x, 7.x, and Fedora 29 (elastic#10796)

Librpm version 4.14.2.1 on Fedora 29 no longer contains the `headerGetEntry` method we are currently using. It was deprecated and then removed in version 4.14 (rpm-software-management/rpm@c68fa9a).

Also, the much older version 4.8.0 of librpm on CentOS 6.10 (Final) does not yet contain newer data structures for tags like `rpm_tag_t/rpmTag/rpmTagVal`.

This PR makes two changes that should allow this code to work on all three distros (CentOS 6.x, 7.x, Fedora 29 - and hopefully anything in between):

1. Use `headerGetString/headerGetNumber` instead of `headerGetEntry`.
2. Use `int32_t` instead of `rpm_tag_t/rpmTag/rpmTagVal`. Luckily, this seems to work on all three distros. I'd prefer something like a typedef, but unfortunately, C99 does not allow repeating a typedef (C11 does) and so backporting them is not easily possible.

It also makes the code more lenient with errors during data collection: Only when no package name can be found do we return an error.

Together with elastic#10694 this will hopefully allow RPM package collection to work well.

(cherry picked from commit e7ea5d7)
cwurm pushed a commit to cwurm/beats that referenced this pull request Feb 22, 2019
…S 6.x, 7.x, and Fedora 29 (elastic#10796)

Librpm version 4.14.2.1 on Fedora 29 no longer contains the `headerGetEntry` method we are currently using. It was deprecated and then removed in version 4.14 (rpm-software-management/rpm@c68fa9a).

Also, the much older version 4.8.0 of librpm on CentOS 6.10 (Final) does not yet contain newer data structures for tags like `rpm_tag_t/rpmTag/rpmTagVal`.

This PR makes two changes that should allow this code to work on all three distros (CentOS 6.x, 7.x, Fedora 29 - and hopefully anything in between):

1. Use `headerGetString/headerGetNumber` instead of `headerGetEntry`.
2. Use `int32_t` instead of `rpm_tag_t/rpmTag/rpmTagVal`. Luckily, this seems to work on all three distros. I'd prefer something like a typedef, but unfortunately, C99 does not allow repeating a typedef (C11 does) and so backporting them is not easily possible.

It also makes the code more lenient with errors during data collection: Only when no package name can be found do we return an error.

Together with elastic#10694 this will hopefully allow RPM package collection to work well.

(cherry picked from commit e7ea5d7)
cwurm pushed a commit that referenced this pull request Feb 25, 2019
…ode compatible across CentOS 6.x, 7.x, and Fedora 29 (#10907)

Cherry-pick of PR #10796 to 6.7 branch. Original message: 

Librpm version 4.14.2.1 on Fedora 29 no longer contains the `headerGetEntry` method we are currently using. It was deprecated and then removed in version 4.14 (rpm-software-management/rpm@c68fa9a).

Also, the much older version 4.8.0 of librpm on CentOS 6.10 (Final) does not yet contain newer data structures for tags like `rpm_tag_t/rpmTag/rpmTagVal`.

This PR makes two changes that should allow this code to work on all three distros (CentOS 6.x, 7.x, Fedora 29 - and hopefully anything in between):

1. Use `headerGetString/headerGetNumber` instead of `headerGetEntry`.
2. Use `int32_t` instead of `rpm_tag_t/rpmTag/rpmTagVal`. Luckily, this seems to work on all three distros. I'd prefer something like a typedef, but unfortunately, C99 does not allow repeating a typedef (C11 does) and so backporting them is not easily possible.

It also makes the code more lenient with errors during data collection: Only when no package name can be found do we return an error.

Together with #10694 this will hopefully allow RPM package collection to work well.
cwurm pushed a commit that referenced this pull request Feb 25, 2019
…S 6.x, 7.x, and Fedora 29 (#10796) (#10843)

Librpm version 4.14.2.1 on Fedora 29 no longer contains the `headerGetEntry` method we are currently using. It was deprecated and then removed in version 4.14 (rpm-software-management/rpm@c68fa9a).

Also, the much older version 4.8.0 of librpm on CentOS 6.10 (Final) does not yet contain newer data structures for tags like `rpm_tag_t/rpmTag/rpmTagVal`.

This PR makes two changes that should allow this code to work on all three distros (CentOS 6.x, 7.x, Fedora 29 - and hopefully anything in between):

1. Use `headerGetString/headerGetNumber` instead of `headerGetEntry`.
2. Use `int32_t` instead of `rpm_tag_t/rpmTag/rpmTagVal`. Luckily, this seems to work on all three distros. I'd prefer something like a typedef, but unfortunately, C99 does not allow repeating a typedef (C11 does) and so backporting them is not easily possible.

It also makes the code more lenient with errors during data collection: Only when no package name can be found do we return an error.

Together with #10694 this will hopefully allow RPM package collection to work well.

(cherry picked from commit e7ea5d7)
cwurm pushed a commit that referenced this pull request Feb 25, 2019
…ode compatible across CentOS 6.x, 7.x, and Fedora 29 (#10842)

Cherry-pick of PR #10796 to 7.x branch. Original message: 

Librpm version 4.14.2.1 on Fedora 29 no longer contains the `headerGetEntry` method we are currently using. It was deprecated and then removed in version 4.14 (rpm-software-management/rpm@c68fa9a).

Also, the much older version 4.8.0 of librpm on CentOS 6.10 (Final) does not yet contain newer data structures for tags like `rpm_tag_t/rpmTag/rpmTagVal`.

This PR makes two changes that should allow this code to work on all three distros (CentOS 6.x, 7.x, Fedora 29 - and hopefully anything in between):

1. Use `headerGetString/headerGetNumber` instead of `headerGetEntry`.
2. Use `int32_t` instead of `rpm_tag_t/rpmTag/rpmTagVal`. Luckily, this seems to work on all three distros. I'd prefer something like a typedef, but unfortunately, C99 does not allow repeating a typedef (C11 does) and so backporting them is not easily possible.

It also makes the code more lenient with errors during data collection: Only when no package name can be found do we return an error.

Together with #10694 this will hopefully allow RPM package collection to work well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants