Fix MQTT v5 clients not being able to specify a password without a us…

…ername.

Thanks to Erik Moqvist.

Closes #1274.

ChangeLog.txt

@@ -28,6 +28,8 @@ Clients:
 - Fix mosquitto_pub exiting with error code 0 when an error occurred.
  Closes #1285.
 - Fix mosquitto_pub not using the `-c` option. Closes #1273.
+- Fix MQTT v5 clients not being able to specify a password without a username.
+ Closes #1274.
 
 
 1.6.2 - 20190430

client/client_shared.c

@@ -325,9 +325,6 @@ int client_config_load(struct mosq_config *cfg, int pub_or_sub, int argc, char *
  fprintf(stderr, "Error: Will retain given, but no will topic given.\n");
  return 1;
  }
- if(cfg->password && !cfg->username){
- err_printf(cfg, "Warning: Not using password since username not set.\n");
- }
 #ifdef WITH_TLS
  if((cfg->certfile && !cfg->keyfile) || (cfg->keyfile && !cfg->certfile)){
  fprintf(stderr, "Error: Both certfile and keyfile must be provided if one of them is set.\n");
@@ -1084,8 +1081,8 @@ int client_opts_set(struct mosquitto *mosq, struct mosq_config *cfg)
  }
  cfg->will_props = NULL;
 
- if(cfg->username && mosquitto_username_pw_set(mosq, cfg->username, cfg->password)){
- err_printf(cfg, "Error: Problem setting username and password.\n");
+ if((cfg->username || cfg->password) && mosquitto_username_pw_set(mosq, cfg->username, cfg->password)){
+ err_printf(cfg, "Error: Problem setting username and/or password.\n");
  mosquitto_lib_cleanup();
  return 1;
  }

client/pub_client.c

@@ -329,7 +329,7 @@ void print_usage(void)
  mosquitto_lib_version(&major, &minor, &revision);
  printf("mosquitto_pub is a simple mqtt client that will publish a message on a single topic and exit.\n");
  printf("mosquitto_pub version %s running on libmosquitto %d.%d.%d.\n\n", VERSION, major, minor, revision);
- printf("Usage: mosquitto_pub {[-h host] [-p port] [-u username [-P password]] -t topic | -L URL}\n");
+ printf("Usage: mosquitto_pub {[-h host] [-p port] [-u username] [-P password] -t topic | -L URL}\n");
  printf(" {-f file | -l | -n | -m message}\n");
  printf(" [-c] [-k keepalive] [-q qos] [-r] [--repeat N] [--repeat-delay time]\n");
 #ifdef WITH_SRV

client/rr_client.c

@@ -156,7 +156,7 @@ void print_usage(void)
  printf(" Defaults to MQTT v5, where the Request-Response feature will be used, but v3.1.1 can also be used\n");
  printf(" with v3.1.1 brokers.\n");
  printf("mosquitto_rr version %s running on libmosquitto %d.%d.%d.\n\n", VERSION, major, minor, revision);
- printf("Usage: mosquitto_rr {[-h host] [-p port] [-u username [-P password]] -t topic | -L URL} -e response-topic\n");
+ printf("Usage: mosquitto_rr {[-h host] [-p port] [-u username] [-P password] -t topic | -L URL} -e response-topic\n");
  printf(" [-c] [-k keepalive] [-q qos] [-R]\n");
  printf(" [-F format]\n");
 #ifndef WIN32

client/sub_client.c

@@ -168,7 +168,7 @@ void print_usage(void)
  mosquitto_lib_version(&major, &minor, &revision);
  printf("mosquitto_sub is a simple mqtt client that will subscribe to a set of topics and print all messages it receives.\n");
  printf("mosquitto_sub version %s running on libmosquitto %d.%d.%d.\n\n", VERSION, major, minor, revision);
- printf("Usage: mosquitto_sub {[-h host] [-p port] [-u username [-P password]] -t topic | -L URL [-t topic]}\n");
+ printf("Usage: mosquitto_sub {[-h host] [-p port] [-u username] [-P password] -t topic | -L URL [-t topic]}\n");
  printf(" [-c] [-k keepalive] [-q qos]\n");
  printf(" [-C msg_count] [-E] [-R] [--retained-only] [--remove-retained] [-T filter_out] [-U topic ...]\n");
  printf(" [-F format]\n");

lib/options.c

@@ -69,6 +69,12 @@ int mosquitto_username_pw_set(struct mosquitto *mosq, const char *username, cons
 {
  if(!mosq) return MOSQ_ERR_INVAL;
 
+ if(mosq->protocol == mosq_p_mqtt311 || mosq->protocol == mosq_p_mqtt31){
+ if(password != NULL && username == NULL){
+ return MOSQ_ERR_INVAL;
+ }
+ }
+
  mosquitto__free(mosq->username);
  mosq->username = NULL;
 
@@ -81,13 +87,14 @@ int mosquitto_username_pw_set(struct mosquitto *mosq, const char *username, cons
  }
  mosq->username = mosquitto__strdup(username);
  if(!mosq->username) return MOSQ_ERR_NOMEM;
- if(password){
- mosq->password = mosquitto__strdup(password);
- if(!mosq->password){
- mosquitto__free(mosq->username);
- mosq->username = NULL;
- return MOSQ_ERR_NOMEM;
- }
+ }
+
+ if(password){
+ mosq->password = mosquitto__strdup(password);
+ if(!mosq->password){
+ mosquitto__free(mosq->username);
+ mosq->username = NULL;
+ return MOSQ_ERR_NOMEM;
  }
  }
  return MOSQ_ERR_SUCCESS;

lib/send_connect.c

@@ -111,11 +111,21 @@ int send__connect(struct mosquitto *mosq, uint16_t keepalive, bool clean_session
  payloadlen += will_proplen + varbytes;
  }
  }
+
+ /* After this check we can be sure that the username and password are
+ * always valid for the current protocol, so there is no need to check
+ * username before checking password. */
+ if(mosq->protocol == mosq_p_mqtt31 || mosq->protocol == mosq_p_mqtt311){
+ if(password != NULL && username == NULL){
+ return MOSQ_ERR_INVAL;
+ }
+ }
+
  if(username){
  payloadlen += 2+strlen(username);
-  if(password){
-  payloadlen += 2+strlen(password);
- }
+ }
+ if(password){
+ payloadlen += 2+strlen(password);
  }
 
  packet->command = CMD_CONNECT;
@@ -145,9 +155,9 @@ int send__connect(struct mosquitto *mosq, uint16_t keepalive, bool clean_session
  }
  if(username){
  byte = byte | 0x1<<7;
-  if(mosq->password){
-  byte = byte | 0x1<<6;
- }
+ }
+ if(mosq->password){
+ byte = byte | 0x1<<6;
  }
  packet__write_byte(packet, byte);
  packet__write_uint16(packet, keepalive);
@@ -173,11 +183,12 @@ int send__connect(struct mosquitto *mosq, uint16_t keepalive, bool clean_session
  packet__write_string(packet, mosq->will->msg.topic, strlen(mosq->will->msg.topic));
  packet__write_string(packet, (const char *)mosq->will->msg.payload, mosq->will->msg.payloadlen);
  }
+
  if(username){
  packet__write_string(packet, username, strlen(username));
-  if(password){
-  packet__write_string(packet, password, strlen(password));
- }
+ }
+ if(password){
+ packet__write_string(packet, password, strlen(password));
  }
 
  mosq->keepalive = keepalive;

man/mosquitto_pub.1.xml

@@ -21,10 +21,8 @@
  <arg choice='plain'>
  <arg><option>-h</option> <replaceable>hostname</replaceable></arg>
  <arg><option>-p</option> <replaceable>port-number</replaceable></arg>
- <arg>
- <arg><option>-u</option> <replaceable>username</replaceable></arg>
- <arg><option>-P</option> <replaceable>password</replaceable></arg>
- </arg>
+ <arg><option>-u</option> <replaceable>username</replaceable></arg>
+ <arg><option>-P</option> <replaceable>password</replaceable></arg>
  <arg choice='plain' rep='repeat'><option>-t</option> <replaceable>message-topic</replaceable></arg>
  </arg>
  <arg choice='plain'><option>-L</option> <replaceable>URL</replaceable></arg>
@@ -366,8 +364,8 @@
  <listitem>
  <para>Provide a password to be used for authenticating with
  the broker. Using this argument without also specifying
- a username is invalid. See also the
- <option>--username</option> option.</para>
+ a username is invalid when using MQTT v3.1 or v3.1.1.
+ See also the <option>--username</option> option.</para>
  </listitem>
  </varlistentry>
  <varlistentry>

man/mosquitto_rr.1.xml

@@ -21,10 +21,8 @@
  <arg choice='plain'>
  <arg><option>-h</option> <replaceable>hostname</replaceable></arg>
  <arg><option>-p</option> <replaceable>port-number</replaceable></arg>
- <arg>
- <arg><option>-u</option> <replaceable>username</replaceable></arg>
- <arg><option>-P</option> <replaceable>password</replaceable></arg>
- </arg>
+ <arg><option>-u</option> <replaceable>username</replaceable></arg>
+ <arg><option>-P</option> <replaceable>password</replaceable></arg>
  <arg choice='plain' rep='repeat'><option>-t</option> <replaceable>message-topic</replaceable></arg>
  </arg>
  <arg choice='plain'>
@@ -387,8 +385,8 @@
  <listitem>
  <para>Provide a password to be used for authenticating with
  the broker. Using this argument without also specifying
- a username is invalid. See also the
- <option>--username</option> option.</para>
+ a username is invalid when using MQTT v3.1 or v3.1.1.
+ See also the <option>--username</option> option.</para>
  </listitem>
  </varlistentry>
  <varlistentry>

man/mosquitto_sub.1.xml

@@ -21,10 +21,8 @@
  <arg choice='plain'>
  <arg><option>-h</option> <replaceable>hostname</replaceable></arg>
  <arg><option>-p</option> <replaceable>port-number</replaceable></arg>
- <arg>
- <arg><option>-u</option> <replaceable>username</replaceable></arg>
- <arg><option>-P</option> <replaceable>password</replaceable></arg>
- </arg>
+ <arg><option>-u</option> <replaceable>username</replaceable></arg>
+ <arg><option>-P</option> <replaceable>password</replaceable></arg>
  <arg choice='plain' rep='repeat'><option>-t</option> <replaceable>message-topic</replaceable></arg>
  </arg>
  <arg choice='plain'>
@@ -396,8 +394,8 @@
  <listitem>
  <para>Provide a password to be used for authenticating with
  the broker. Using this argument without also specifying
- a username is invalid. See also the
- <option>--username</option> option.</para>
+ a username is invalid when using MQTT v3.1 or v3.1.1.
+ See also the <option>--username</option> option.</para>
  </listitem>
  </varlistentry>
  <varlistentry>

src/handle_connect.c

@@ -586,25 +586,10 @@ int handle__connect(struct mosquitto_db *db, struct mosquitto *context)
 
  if(username_flag){
  rc = packet__read_string(&context->in_packet, &username, &slen);
- if(rc == MOSQ_ERR_SUCCESS){
- if(password_flag){
- rc = packet__read_binary(&context->in_packet, (uint8_t **)&password, &slen);
- if(rc == MOSQ_ERR_NOMEM){
- rc = MOSQ_ERR_NOMEM;
- goto handle_connect_error;
- }else if(rc == MOSQ_ERR_PROTOCOL){
- if(context->protocol == mosq_p_mqtt31){
- /* Password flag given, but no password. Ignore. */
- }else{
- rc = MOSQ_ERR_PROTOCOL;
- goto handle_connect_error;
- }
- }
- }
- }else if(rc == MOSQ_ERR_NOMEM){
+ if(rc == MOSQ_ERR_NOMEM){
  rc = MOSQ_ERR_NOMEM;
  goto handle_connect_error;
- }else{
+ }else if(rc != MOSQ_ERR_SUCCESS){
  if(context->protocol == mosq_p_mqtt31){
  /* Username flag given, but no username. Ignore. */
  username_flag = 0;
@@ -614,7 +599,7 @@ int handle__connect(struct mosquitto_db *db, struct mosquitto *context)
  }
  }
  }else{
- if(context->protocol == mosq_p_mqtt311 || context->protocol == mosq_p_mqtt5){
+ if(context->protocol == mosq_p_mqtt311 || context->protocol == mosq_p_mqtt31){
  if(password_flag){
  /* username_flag == 0 && password_flag == 1 is forbidden */
  log__printf(NULL, MOSQ_LOG_ERR, "Protocol error from %s: password without username, closing connection.", client_id);
@@ -623,6 +608,21 @@ int handle__connect(struct mosquitto_db *db, struct mosquitto *context)
  }
  }
  }
+ if(password_flag){
+ rc = packet__read_binary(&context->in_packet, (uint8_t **)&password, &slen);
+ if(rc == MOSQ_ERR_NOMEM){
+ rc = MOSQ_ERR_NOMEM;
+ goto handle_connect_error;
+ }else if(rc == MOSQ_ERR_PROTOCOL){
+ if(context->protocol == mosq_p_mqtt31){
+ /* Password flag given, but no password. Ignore. */
+ }else{
+ rc = MOSQ_ERR_PROTOCOL;
+ goto handle_connect_error;
+ }
+ }
+ }
+
  if(context->in_packet.pos != context->in_packet.remaining_length){
  /* Surplus data at end of packet, this must be an error. */
  rc = MOSQ_ERR_PROTOCOL;
@@ -755,7 +755,7 @@ int handle__connect(struct mosquitto_db *db, struct mosquitto *context)
 #endif /* FINAL_WITH_TLS_PSK */
  }else{
 #endif /* WITH_TLS */
- if(username_flag){
+ if(username_flag || password_flag){
  /* FIXME - these ensure the mosquitto_client_id() and
  * mosquitto_client_username() functions work, but is hacky */
  context->id = client_id;
@@ -786,9 +786,7 @@ int handle__connect(struct mosquitto_db *db, struct mosquitto *context)
  context->password = password;
  username = NULL; /* Avoid free() in error: below. */
  password = NULL;
- }
-
- if(!username_flag){
+ }else{
  if((db->config->per_listener_settings && context->listener->security_options.allow_anonymous == false)
  || (!db->config->per_listener_settings && db->config->security_options.allow_anonymous == false)){