acl check returns MOSQ_ERR_PLUGIN_DEFER when require_certificate=true

[rcadena]

mosquitto: 1.5.3 mosquitto_auth_plug: Commit # 4e7fe9a
backend: http

mosquitto.conf configured with:

require_certificate true
use_identity_as_username true
use_username_as_clientid true

Issue: The ACL check on auth-plug.c:611 fails when a client is "authenticated" via a certificate.

It looks like the plugin's auth method is not called when the above config options are used, so the client is not "cached" by auth-plug, which causes the ACL call to fail getting the username and id from the hash.

Details:

1. Test auth_plug is used: I tested that the http backend worked by disabling require_certificate and using a username and password with mosquitto_pub. Ensure that the auth_plug's http backend has the auth and acl URIs called. auth_plug works.
2. Test publishing works without auth_plug: update the mosquitto configuration as describe above. Disable auth_plug so and attempt to connect and publish a message by using mosquitto_sub with certificates, etc.
3. Enable auth_plug as above.
4. The client connects, but the plugin's auth function is not called.
5. The client publishes a message, the plugin's acl function is called, but it exists at line 616 because it did not find the client in the cache.

A quick fix I implemented was as follows:

# FILE: auth-plug.c
...
#include <mosquitto_broker.h>
...
int mosquitto_auth_acl_check( ... ) 
    const char *clientid = NULL;
    const char *username = NULL;
    const char *topic = msg->topic;
    HASH_FIND(hh, ud->clients, &client, sizeof(void *), e); 
    if (e) {
        clientid = e->clientid;
        username = e->username;
    } else {
        clientid = mosquitto_client_id(client);
        username = mosquitto_client_username(client);
        // return MOSQ_ERR_PLUGIN_DEFER;
    }   

This extracts the clientid and username using mosquitto's built in functions. I'm not sure this is the correct way to fix this, however. Thoughts are welcome.

NOTE This is the same issue as: https://github.com/jpmens/mosquitto-auth-plug/issues/331 however, the fix there was first to set the cache time to 0, and to make sure the superusers return 40X... However, I would like to not have to set the cache time to 0 because I could be making lots of ACL checks.

[jpmens]

@rcadena thank you for your report. I have, based on your suggested patch, added support for this and I'd appreciate you testing it.

The way it's supposed to work is that when we detect that a client has authenticated with a TLS client certificate that we then use the functions you propose to extract clientId and username. Should these not be available we DEFER as we've done so far.

Does that sound reasonable? And more importantly: does it actually work for you?

[rcadena]

@jpmens apologies for the delay in replying. I will give it a test and report back. Thanks!

[rcadena]

@jpmens I just tried it and ran the same tests and it works perfectly! Thanks for the patch.

[jpmens]

Thank you, @rcadena for the hint and the fix, and for reporting back.