Fix and tests for security bug #541870.
This commit is contained in:
parent
36b5421c59
commit
d8505624d0
@ -11,6 +11,12 @@ Security:
|
||||
unaffected. Users who have only used the mosquitto_passwd utility to create
|
||||
and modify their password files are unaffected by this vulnerability.
|
||||
Affects version 1.0 to 1.5.5 inclusive.
|
||||
- CVE-2018-xxxxx: If an ACL file is empty, or has only blank lines or
|
||||
comments, then mosquitto treats the ACL file as not being defined, which
|
||||
means that no topic access is denied. Although denying access to all topics
|
||||
is not a useful configuration, this behaviour is unexpected and could lead
|
||||
to access being incorrectly granted in some circumstances. This is now
|
||||
fixed. Affects versions 1.0 to 1.5.5 inclusive.
|
||||
|
||||
Broker:
|
||||
- Fixed comment handling for config options that have optional arguments.
|
||||
|
@ -317,7 +317,7 @@ int mosquitto_acl_check_default(struct mosquitto_db *db, struct mosquitto *conte
|
||||
}else{
|
||||
security_opts = &db->config->security_options;
|
||||
}
|
||||
if(!security_opts->acl_list && !security_opts->acl_patterns){
|
||||
if(!security_opts->acl_file && !security_opts->acl_list && !security_opts->acl_patterns){
|
||||
return MOSQ_ERR_PLUGIN_DEFER;
|
||||
}
|
||||
|
||||
@ -535,6 +535,10 @@ static int aclfile__parse(struct mosquitto_db *db, struct mosquitto__security_op
|
||||
fclose(aclfptr);
|
||||
return 1;
|
||||
}
|
||||
}else{
|
||||
log__printf(NULL, MOSQ_LOG_ERR, "Error: Invalid line in acl_file \"%s\": %s.", security_opts->acl_file, buf);
|
||||
fclose(aclfptr);
|
||||
return 1;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
71
test/broker/09-acl-empty-file.py
Executable file
71
test/broker/09-acl-empty-file.py
Executable file
@ -0,0 +1,71 @@
|
||||
#!/usr/bin/env python
|
||||
|
||||
# Test for CVE-2018-xxxxx
|
||||
|
||||
import inspect, os, sys
|
||||
# From http://stackoverflow.com/questions/279237/python-import-a-module-from-a-folder
|
||||
cmd_subfolder = os.path.realpath(os.path.abspath(os.path.join(os.path.split(inspect.getfile( inspect.currentframe() ))[0],"..")))
|
||||
if cmd_subfolder not in sys.path:
|
||||
sys.path.insert(0, cmd_subfolder)
|
||||
|
||||
import mosq_test
|
||||
import signal
|
||||
|
||||
def write_config(filename, port, per_listener):
|
||||
with open(filename, 'w') as f:
|
||||
f.write("per_listener_settings %s\n" % (per_listener))
|
||||
f.write("port %d\n" % (port))
|
||||
f.write("acl_file %s\n" % (filename.replace('.conf', '.acl')))
|
||||
|
||||
def write_acl(filename):
|
||||
with open(filename, 'w') as f:
|
||||
f.write('#comment\n')
|
||||
f.write('\n')
|
||||
|
||||
|
||||
def do_test(port, per_listener):
|
||||
conf_file = os.path.basename(__file__).replace('.py', '.conf')
|
||||
write_config(conf_file, port, per_listener)
|
||||
|
||||
acl_file = os.path.basename(__file__).replace('.py', '.acl')
|
||||
write_acl(acl_file)
|
||||
|
||||
rc = 1
|
||||
keepalive = 60
|
||||
connect_packet = mosq_test.gen_connect("acl-check", keepalive=keepalive)
|
||||
connack_packet = mosq_test.gen_connack(rc=0)
|
||||
|
||||
mid = 1
|
||||
publish_packet = mosq_test.gen_publish("test/topic", qos=0, payload="message")
|
||||
subscribe_packet = mosq_test.gen_subscribe(mid, "test/topic", 0)
|
||||
suback_packet = mosq_test.gen_suback(mid, 0)
|
||||
|
||||
pingreq_packet = mosq_test.gen_pingreq()
|
||||
pingresp_packet = mosq_test.gen_pingresp()
|
||||
|
||||
broker = mosq_test.start_broker(filename=os.path.basename(__file__), use_conf=True, port=port)
|
||||
|
||||
try:
|
||||
sock = mosq_test.do_client_connect(connect_packet, connack_packet, port=port)
|
||||
mosq_test.do_send_receive(sock, subscribe_packet, suback_packet, "suback")
|
||||
|
||||
sock.send(publish_packet)
|
||||
|
||||
# If we receive the message, this will fail.
|
||||
mosq_test.do_send_receive(sock, pingreq_packet, pingresp_packet, "pingresp")
|
||||
rc = 0
|
||||
|
||||
sock.close()
|
||||
finally:
|
||||
os.remove(conf_file)
|
||||
os.remove(acl_file)
|
||||
broker.terminate()
|
||||
broker.wait()
|
||||
(stdo, stde) = broker.communicate()
|
||||
if rc:
|
||||
print(stde)
|
||||
exit(rc)
|
||||
|
||||
port = mosq_test.get_port()
|
||||
do_test(port, "false")
|
||||
do_test(port, "true")
|
@ -118,6 +118,7 @@ endif
|
||||
endif
|
||||
|
||||
09 :
|
||||
./09-acl-empty-file.py
|
||||
./09-plugin-auth-unpwd-success.py
|
||||
./09-plugin-auth-unpwd-fail.py
|
||||
./09-plugin-auth-acl-sub.py
|
||||
|
@ -90,6 +90,7 @@ tests = [
|
||||
(2, './08-tls-psk-pub.py'),
|
||||
(3, './08-tls-psk-bridge.py'),
|
||||
|
||||
(1, './09-acl-empty-file.py'),
|
||||
(1, './09-plugin-auth-unpwd-success.py'),
|
||||
(1, './09-plugin-auth-unpwd-fail.py'),
|
||||
(1, './09-plugin-auth-acl-sub.py'),
|
||||
|
Loading…
Reference in New Issue
Block a user