Fix clients using use_identity_as_* being disconnected on SIGHUP.

Closes #1402. Thanks to twegener-embertec.

ChangeLog.txt

@@ -4,6 +4,8 @@ Broker:
 - Fix support for libwebsockets 3.x.
 - Fix slow websockets performance when sending large messages. Closes #1390.
 - Fix bridges potentially not connecting on Windows. Closes #478.
+- Fix clients authorised using `use_identity_as_username` or
+ `use_subject_as_username` being disconnected on SIGHUP. Closes #1402.
 
 Client library:
 - Fix reconnect backoff for the situation where connections are dropped rather

src/security_default.c

@@ -21,6 +21,8 @@ and the Eclipse Distribution License is available at
 
 #include "mosquitto_broker_internal.h"
 #include "memory_mosq.h"
+#include "mqtt_protocol.h"
+#include "send_mosq.h"
 #include "util_mosq.h"
 
 static int aclfile__parse(struct mosquitto_db *db, struct mosquitto__security_options *security_opts);
@@ -939,6 +941,18 @@ static int unpwd__cleanup(struct mosquitto__unpwd **root, bool reload)
  return MOSQ_ERR_SUCCESS;
 }
 
+
+#ifdef WITH_TLS
+static void security__disconnect_auth(struct mosquitto_db *db, struct mosquitto *context)
+{
+ if(context->protocol == mosq_p_mqtt5){
+ send__disconnect(context, MQTT_RC_ADMINISTRATIVE_ACTION, NULL);
+ }
+ context__set_state(context, mosq_cs_disconnecting);
+ do_disconnect(db, context, MOSQ_ERR_AUTH);
+}
+#endif
+
 /* Apply security settings after a reload.
  * Includes:
  * - Disconnecting anonymous users if appropriate
@@ -951,6 +965,13 @@ int mosquitto_security_apply_default(struct mosquitto_db *db)
  struct mosquitto__acl_user *acl_user_tail;
  bool allow_anonymous;
  struct mosquitto__security_options *security_opts = NULL;
+#ifdef WITH_TLS
+ int i;
+ X509 *client_cert = NULL;
+ X509_NAME *name;
+ X509_NAME_ENTRY *name_entry;
+ ASN1_STRING *name_asn1 = NULL;
+#endif
 
  if(!db) return MOSQ_ERR_INVAL;
 
@@ -973,12 +994,116 @@ int mosquitto_security_apply_default(struct mosquitto_db *db)
  do_disconnect(db, context, MOSQ_ERR_AUTH);
  continue;
  }
+
  /* Check for connected clients that are no longer authorised */
- if(mosquitto_unpwd_check(db, context, context->username, context->password) != MOSQ_ERR_SUCCESS){
- context__set_state(context, mosq_cs_disconnecting);
- do_disconnect(db, context, MOSQ_ERR_AUTH);
- continue;
+#ifdef WITH_TLS
+ if(context->listener->ssl_ctx && (context->listener->use_identity_as_username || context->listener->use_subject_as_username)){
+ /* Client must have either a valid certificate, or valid PSK used as a username. */
+ if(!context->ssl){
+ if(context->protocol == mosq_p_mqtt5){
+ send__disconnect(context, MQTT_RC_ADMINISTRATIVE_ACTION, NULL);
+ }
+ context__set_state(context, mosq_cs_disconnecting);
+ do_disconnect(db, context, MOSQ_ERR_AUTH);
+ continue;
+ }
+#ifdef FINAL_WITH_TLS_PSK
+ if(context->listener->psk_hint){
+ /* Client should have provided an identity to get this far. */
+ if(!context->username){
+ security__disconnect_auth(db, context);
+ continue;
+ }
+ }else
+#endif /* FINAL_WITH_TLS_PSK */
+ {
+ client_cert = SSL_get_peer_certificate(context->ssl);
+ if(!client_cert){
+ security__disconnect_auth(db, context);
+ continue;
+ }
+ name = X509_get_subject_name(client_cert);
+ if(!name){
+ X509_free(client_cert);
+ client_cert = NULL;
+ security__disconnect_auth(db, context);
+ continue;
+ }
+ if (context->listener->use_identity_as_username) { //use_identity_as_username
+ i = X509_NAME_get_index_by_NID(name, NID_commonName, -1);
+ if(i == -1){
+ X509_free(client_cert);
+ client_cert = NULL;
+ security__disconnect_auth(db, context);
+ continue;
+ }
+ name_entry = X509_NAME_get_entry(name, i);
+ if(name_entry){
+ name_asn1 = X509_NAME_ENTRY_get_data(name_entry);
+ if (name_asn1 == NULL) {
+ X509_free(client_cert);
+ client_cert = NULL;
+ security__disconnect_auth(db, context);
+ continue;
+ }
+#if OPENSSL_VERSION_NUMBER < 0x10100000L
+ context->username = mosquitto__strdup((char *) ASN1_STRING_data(name_asn1));
+#else
+ context->username = mosquitto__strdup((char *) ASN1_STRING_get0_data(name_asn1));
+#endif
+ if(!context->username){
+ X509_free(client_cert);
+ client_cert = NULL;
+ security__disconnect_auth(db, context);
+ continue;
+ }
+ /* Make sure there isn't an embedded NUL character in the CN */
+ if ((size_t)ASN1_STRING_length(name_asn1) != strlen(context->username)) {
+ X509_free(client_cert);
+ client_cert = NULL;
+ security__disconnect_auth(db, context);
+ continue;
+ }
+ }
+ } else { // use_subject_as_username
+ BIO *subject_bio = BIO_new(BIO_s_mem());
+ X509_NAME_print_ex(subject_bio, X509_get_subject_name(client_cert), 0, XN_FLAG_RFC2253);
+ char *data_start = NULL;
+ long name_length = BIO_get_mem_data(subject_bio, &data_start);
+ char *subject = mosquitto__malloc(sizeof(char)*name_length+1);
+ if(!subject){
+ BIO_free(subject_bio);
+ X509_free(client_cert);
+ client_cert = NULL;
+ security__disconnect_auth(db, context);
+ continue;
+ }
+ memcpy(subject, data_start, name_length);
+ subject[name_length] = '\0';
+ BIO_free(subject_bio);
+ context->username = subject;
+ }
+ if(!context->username){
+ X509_free(client_cert);
+ client_cert = NULL;
+ security__disconnect_auth(db, context);
+ continue;
+ }
+ X509_free(client_cert);
+ client_cert = NULL;
+ }
+ }else
+#endif
+ {
+ /* Username/password check only if the identity/subject check not used */
+ if(mosquitto_unpwd_check(db, context, context->username, context->password) != MOSQ_ERR_SUCCESS){
+ context__set_state(context, mosq_cs_disconnecting);
+ do_disconnect(db, context, MOSQ_ERR_AUTH);
+ continue;
+ }
  }
+
+
  /* Check for ACLs and apply to user. */
  if(db->config->per_listener_settings){
  if(context->listener){

test/broker/08-ssl-hup-disconnect.py

@@ -0,0 +1,72 @@
+#!/usr/bin/env python3
+
+# Test whether a client connected with a client certificate when
+# use_identity_as_username is true is then disconnected when a SIGHUP is
+# received.
+# https://github.com/eclipse/mosquitto/issues/1402
+
+from mosq_test_helper import *
+import signal
+
+if sys.version < '2.7':
+ print("WARNING: SSL not supported on Python 2.6")
+ exit(0)
+
+def write_config(filename, pw_file, port, option):
+ with open(filename, 'w') as f:
+ f.write("listener %d\n" % (port))
+ f.write("cafile ../ssl/all-ca.crt\n")
+ f.write("certfile ../ssl/server.crt\n")
+ f.write("keyfile ../ssl/server.key\n")
+ f.write("require_certificate true\n")
+ f.write("%s true\n" % (option))
+ f.write("password_file %s\n" % (pw_file))
+
+def write_pwfile(filename):
+ with open(filename, 'w') as f:
+ # Username "test client", password test
+ f.write('test client:$6$njERlZMi/7DzNB9E$iiavfuXvUm8iyDZArTy7smTxh07GXXOrOsqxfW6gkOYVXHGk+W+i/8d3xDxrMwEPygEBhoA8A/gjQC0N2M4Lkw==\n')
+
+def do_test(option):
+ port = mosq_test.get_port()
+ conf_file = os.path.basename(__file__).replace('.py', '.conf')
+ pw_file = os.path.basename(__file__).replace('.py', '.pwfile')
+ write_config(conf_file, pw_file, port, option)
+ write_pwfile(pw_file)
+
+ rc = 1
+ keepalive = 10
+ connect_packet = mosq_test.gen_connect("connect-success-test", keepalive=keepalive)
+ connack_packet = mosq_test.gen_connack(rc=0)
+
+ broker = mosq_test.start_broker(filename=os.path.basename(__file__), port=port, use_conf=True)
+
+ try:
+ sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
+ ssock = ssl.wrap_socket(sock, ca_certs="../ssl/test-root-ca.crt", certfile="../ssl/client.crt", keyfile="../ssl/client.key", cert_reqs=ssl.CERT_REQUIRED)
+ ssock.settimeout(20)
+ ssock.connect(("localhost", port))
+ mosq_test.do_send_receive(ssock, connect_packet, connack_packet, "connack")
+
+ broker.send_signal(signal.SIGHUP)
+ time.sleep(1)
+
+ # This will fail if we've been disconnected
+ mosq_test.do_ping(ssock)
+ rc = 0
+
+ ssock.close()
+ finally:
+ os.remove(conf_file)
+ os.remove(pw_file)
+ broker.terminate()
+ broker.wait()
+ (stdo, stde) = broker.communicate()
+ if rc:
+ print(stde.decode('utf-8'))
+ exit(rc)
+
+do_test("use_identity_as_username")
+do_test("use_subject_as_username")
+exit(0)
+

test/broker/Makefile

@@ -160,6 +160,7 @@ ifeq ($(WITH_TLS),yes)
  ./08-ssl-connect-no-auth-wrong-ca.py
  ./08-ssl-connect-no-auth.py
  ./08-ssl-connect-no-identity.py
+ ./08-ssl-hup-disconnect.py
 ifeq ($(WITH_TLS_PSK),yes)
  ./08-tls-psk-pub.py
  ./08-tls-psk-bridge.py

test/broker/test.py

@@ -132,6 +132,7 @@
  (2, './08-ssl-connect-no-auth-wrong-ca.py'),
  (2, './08-ssl-connect-no-auth.py'),
  (2, './08-ssl-connect-no-identity.py'),
+ (1, './08-ssl-hup-disconnect.py'),
  (2, './08-tls-psk-pub.py'),
  (3, './08-tls-psk-bridge.py'),