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

Systemd workloadattestor closes DBus connection too early #4165

Merged
merged 6 commits into from
May 24, 2023

Conversation

rkaippully
Copy link
Contributor

Pull Request check list

  • Commit conforms to CONTRIBUTING.md?
  • Proper tests/regressions included?
  • Documentation updated?

Affected functionality
This PR fixes a bug in the systemd workload attestor. In getSystemdUnitInfo, the DBus connection is closed via a defer. But that connection is required later to retrieve the ID() and FragmentPath() in the Attest function. This causes attestation failures with an error message dbus: connection closed by user.

Description of change

The fix extracts the required selector attributes early and avoids using the dangling connection to DBus.

This PR fixes is a bug in the systemd workload attestor. In
`getSystemdUnitInfo`, the DBus connection is closed via a `defer`. But
that connection is required later to retrieve the `ID()` and
`FragmentPath()` in the `Attest` function. This causes attestation
failures with an error message `dbus: connection closed by user`.

The fix extracts the required selector attributes early and avoids
using the dangling connection to DBus.

Signed-off-by: Raghu Kaippully <rkaippully@gmail.com>
Signed-off-by: Raghu Kaippully <rkaippully@gmail.com>
Copy link
Member

@azdagron azdagron left a comment

Choose a reason for hiding this comment

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

Thank you, @rkaippully! Just a few small nitpicks and this is ready to merge.

Co-authored-by: Andrew Harding <azdagron@gmail.com>
Signed-off-by: Raghu Kaippully <rkaippully@gmail.com>
@rkaippully rkaippully force-pushed the raghu/fix-systemd-defer branch from e8afaad to dfc95a2 Compare May 24, 2023 04:27
@rkaippully
Copy link
Contributor Author

Thanks for the quick review @azdagron!

Copy link
Member

@azdagron azdagron left a comment

Choose a reason for hiding this comment

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

Thanks again!

@azdagron azdagron merged commit 5f75f54 into spiffe:main May 24, 2023
@azdagron azdagron added this to the 1.7.0 milestone May 24, 2023
@rkaippully rkaippully deleted the raghu/fix-systemd-defer branch May 25, 2023 02:41
Neniel pushed a commit to Neniel/spire that referenced this pull request Aug 24, 2023
This PR fixes is a bug in the systemd workload attestor. In
`getSystemdUnitInfo`, the DBus connection is closed via a `defer`. But
that connection is required later to retrieve the `ID()` and
`FragmentPath()` in the `Attest` function. This causes attestation
failures with an error message `dbus: connection closed by user`.

The fix extracts the required selector attributes early and avoids
using the dangling connection to DBus.

Signed-off-by: Raghu Kaippully <rkaippully@gmail.com>
Signed-off-by: Neniel <11655196+Neniel@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants