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

Include D-Bus sender in User-Agent http header #2329

Merged
merged 1 commit into from
Aug 24, 2021
Merged

Conversation

jirihnidek
Copy link
Contributor

  • When some application call D-Bus method, then the application
    can be identified using sender argument of the method.
    This argument used to be unused. The sender ID (e.g. ':1:3003')
    can be used for finding of PID and this PID can be used for
    finding command line used for starting the application that
    sent the D-Bus method. This could be e.g.: 'dbus-send',
    'cockpit-bridge', etc. (note: all command line options are
    stripped for security reason to not send username nor password).
    The command line of D-Bus sender is stored in singleton and
    then this command line is added to User-Agent http header
    as 'dbus_sender=cockpit-bridge'
  • The sender argument is saved to singleton using decorator
    dbus_handle_sender implemented in rhsmlib.dbus.util.
    When it will not be desired to store sender for some method
    then just don't use this decorator.
  • When methods of DomainSocketRegisterDBusObject are used,
    then it is little bit complicated, because system bus
    is not used there, but custom unix socket is used. Thus
    sender argument is not available. It is necessary to save
    sender, when DomainSocketRegisterDBusObject is created using
    com.redha.RHSM.Register.Start method
  • It will be possible to do statistics of applications using
    our D-Bus API
  • Added implementation of singleton
  • Modified some unit tests

@jirihnidek
Copy link
Contributor Author

This PR depend on #2328

@jirihnidek
Copy link
Contributor Author

The http headers could look like this:

{
    "Content-type": "application/json",
    "Accept": "application/json",
    "x-subscription-manager-version": "RPM_VERSION",
    "Accept-Language": "cs-cz",
    "X-Correlation-ID": "70859bc7ffbd4f0b836a7c1b2946414a",
    "User-Agent": "RHSM/1.0 (cmd=rhsm_service.py) subscription-manager/RPM_VERSION dbus_sender=cockpit-bridge",
    "Content-Length": "0"
}

@jirihnidek jirihnidek force-pushed the jhnidek/dbus_sender branch 2 times, most recently from f7ac889 to b57a161 Compare September 23, 2020 14:08
@wottop wottop self-assigned this Oct 2, 2020
Singleton and parent for singletons
"""
_instance = None
_initialized = False
Copy link
Member

Choose a reason for hiding this comment

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

Where is this used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I realized that I do something wrong here and we will need to use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So it is used in decorator no_reinitialization.

@wottop wottop removed their assignment Oct 7, 2020
@cnsnyder
Copy link
Member

Looks like there are a few failing flake8 issues:

test/test_managercli.py:1605:37: E231 missing whitespace after ','
test/test_managercli.py:1605:45: E231 missing whitespace after ','

spoiled_child = SpoiledChild()
self.assertEqual(id(spoiled_child), spoiled_child_id)

def test_signleton_and_treading(self):
Copy link
Member

@cnsnyder cnsnyder Oct 12, 2020

Choose a reason for hiding this comment

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

test_signleton_and_treading -> test_singleton_and_threading

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

elif len(args) > 0:
sender = args[-1]

if sender is not None:
Copy link
Member

Choose a reason for hiding this comment

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

If we want to ensure that the sender isn't changed for a given call, shouldn't we acquire the lock for the DbusSender singleton here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point!

try:
return func(*args, **kwargs)
finally:
dbus_reset_sender_cmd_line()
Copy link
Member

Choose a reason for hiding this comment

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

And if we acquire the lock before the function call, I'd suggest we release it here.

Admittedly this is effectively serializing all our dbus calls which may be worth more discussion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I implemented this using __enter__() and __exit__() methods and with DBusSender as dbus_sender statement. It is more clear solution.

@jirihnidek jirihnidek force-pushed the jhnidek/dbus_sender branch 2 times, most recently from 6da5b37 to d738c64 Compare October 15, 2020 09:19
@wottop
Copy link
Contributor

wottop commented Jul 27, 2021

We need to fix the test issue and rebase if this is on track for inclusion. Otherwise, we should close the PR until is it on the path.

* When some application call D-Bus method, then the application
  can be identified using sender argument of the method.
  This argument used to be unused. The sender ID (e.g. ':1:3003')
  can be used for finding of PID and this PID can be used for
  finding command line used for starting the application that
  sent the D-Bus method. This could be e.g.: 'dbus-send',
  'cockpit-bridge', etc. (note: all command line options are
  stripped for security reason to not send username nor password).
  The command line of D-Bus sender is stored in singleton and
  then this command line is added to User-Agent http header
  as 'dbus_sender=cockpit-bridge'
* The sender argument is saved to singleton using decorator
  dbus_handle_sender implemented in rhsmlib.dbus.util.
  When it will not be desired to store sender for some method
  then just don't use this decorator.
* When methods of DomainSocketRegisterDBusObject are used,
  then it is little bit complicated, because system bus
  is not used there, but custom unix socket is used. Thus
  sender argument is not available. It is necessary to save
  sender, when DomainSocketRegisterDBusObject is created using
  com.redha.RHSM.Register.Start method
* Fix one issue with initialization of CPPProvider
* It will be possible to do statistics of applications using
  our D-Bus API
* Modified some unit tests
* Added implementation of singleton
* Added unit tests for singleton
* Create new connection for GetStatus, otherwise the dbus_sender
  is not set correctly and value from previous call is used
* Fix issue with printing traceback to the file
* Do not use wild import in rhsmlib.dbus package, because it
  caused circular import of DBusSender
@jirihnidek
Copy link
Contributor Author

@wottop I updated the PR.

@cnsnyder cnsnyder merged commit 29167f9 into main Aug 24, 2021
@m-horky m-horky deleted the jhnidek/dbus_sender branch September 30, 2021 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants