regression because of CVE-2017-7650: clientid with / character

[wiebeytec]

The / character is not allowed in the client ID since version 1.4.12. This breaks mosquitto_pub and mosquitto_sub, in generate_client_id, because they themselves use a /.

Even if that is changed, all versions installed by distros will break on this.

The / was included in the blacklist because it "may represent an additional risk". Perhaps it should be undone?

[hmvp]

Also plugins that do proper checks are effected by this!
We have to move to custom builds of mosquitto because we cannot disallow '/', '+' and '#' characters.

It would be nice to be able to opt out this unwanted security...

[vavrecan]

What about cleaning up client_id after auth plugin check instead of enforcing policy before?

[ralight]

It was a mistake not to change the autogenerated IDs for mosquitto_pub/sub. This is fixed in the fixes branch.

I think it will be required to allow / in usernames given their fairly widespread use there, but the spec does not expect / to be allowed in client ids.

Plugins that do proper checks are affected by this, yes, but in a survey of plugins that I could find, more were vulnerable than secure, so this was taken as the least worst option.

How about a plugin_supports CVE-2017-7650 option to indicate that the plugin already handles it?

[wiebeytec]

How about a plugin_supports CVE-2017-7650 option to indicate that the plugin already handles it?

That would work for me, although in my case a bit of a misnomer, because the auth plugin we wrote is not susceptible to the problem, i.e. user names or client IDs never appear in topics.

What about auth_plugin_cve_2017_7650_safe true?

[ralight]

auth_plugin_cve_2017_7650_safe true - I was hoping to have a single config option for this sort of situation so if something else comes up then it's just another value to add rather than a new option.

Any thoughts on a better name?

[ralight]

This should be fixed in the upcoming 1.4.13 release, so I'm closing this.