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

killall -HUP disconnects clients when using use_identity_as_username true #1402

Closed
twegener-embertec opened this issue Sep 5, 2019 · 3 comments

Comments

@twegener-embertec
Copy link

commented Sep 5, 2019

When sending the HUP signal to mosquitto (for the purpose of log file close/reopen for logrotate), I found that clients using username/password auth remained connected (good), whereas clients of listener using require_certificate / use_identity_as_username were disconnected (bad).

I'm raising this as a separate issue to #657, as I suspect that this is a different special case.

I'm wondering whether the auth checks on reload are not working for the use_identity_as_username case because they may be expecting password to be present when in fact it is not applicable, say.

This is with 1.6.4-0mosquitto1~xenial1 from the mosquitto PPA.

@ralight ralight added this to the 1.6.5 milestone Sep 5, 2019
@ralight

This comment has been minimized.

Copy link
Contributor

commented Sep 5, 2019

Thank you! Are you able to test against the current fixes branch please? I've just pushed a change that fixes this.

@twegener-embertec

This comment has been minimized.

Copy link
Author

commented Sep 6, 2019

Thanks for the very quick response! :-)

I've tried it out to the the extent that I can say:

  • It doesn't disconnect any users on HUP (at least in the normal case where all the client credentials are still valid).
  • General operation seems to be fine (i.e. no glaring regressions in our setting).

However, I haven't tested things like whether it is kicking off clients whose credentials are no longer satisfactory after the config reload via HUP. Can you think of a simple way for me to test that? Otherwise, I'll leave that with you.

@twegener-embertec

This comment has been minimized.

Copy link
Author

commented Sep 6, 2019

Side note: It seems to me that having up-to-date CRL information would align with the intent of the checks that check that client auth crendentials are still applicable. Would it make sense / be practically possible to reload the CRL as part of the config reload on HUP procedure?
I suspect that this would be somewhat simpler to implement than the on-connection CRL autoreload patch proposed in #35.
Also, AFAICT, that proposal doesn't cover the case where one updates the CRL with a new revocation entry, and so would ideally like mosquitto to disconnect any existing client connection that is using that newly revoked cert.

So to summarise my suggestion:
Can/should the config-reload-on-HUP procedure, also reload the CRL file, and ensure that all connections that are using revoked certs are disconnected?
(Cf. e.g. nginx that reloads the CRL on reload (HUP) signal.)

I suppose I should raise a separate issue for that, but just want to check with you first regarding how that fits in with the intent of the current reload-config-on-HUP code, and also w.r.t. #35.

@ralight ralight closed this in f6b22f8 Sep 12, 2019
ralight added a commit that referenced this issue Sep 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.