Projects tigase _server server-core Issues #1434
Database inconsistency errors during in-band registration (#1434)
Unknown opened 4 years ago

Describe the bug

A client may leave the server database in inconsistent state.

  1. If the database url doesn't have 'autoCreateUser=true', in-band creation will not work at all, but the server still returns a success response.

  2. If the database url has 'autoCreateUser=true', the behaviour is as follows:

  • on the first attempt, a user entry will be created in tig_users, but password will be unset
  • on the next attempts, the password entry will be set / changed.

The server returns a success response in every case.

To Reproduce

import asyncio
import slixmpp

class Connection(slixmpp.ClientXMPP):
	def __init__(self, jid, password):
		self.__username = jid.split('@')[0]
		self.__password = password
		self.__login_failed = False
		
		super().__init__(jid, password)
		
		self.register_plugin('xep_0077')
		self.add_event_handler('connected', self.on_connected)
		self.add_event_handler('disconnected', self.on_disconnected)
		self.add_event_handler('failed_auth', self.on_failed_auth)
		self.add_event_handler('session_start', self.on_session_start)
		self.add_event_handler('session_end', self.on_session_end)
	
	def exception(self, exception):
		print("exception", exception)
		raise exception
	
	def on_connected(self, event):
		print("connected", event)
	
	def on_disconnected(self, event):
		print("disconnected", event)
	
	def on_failed_auth(self, event):
		print("failed_auth", event)
		self.__login_failed = True
	
	async def on_session_start(self, event):
		print("session_start", event)
		if self.__login_failed:
			print(" login failed, registering")
			
			iq = await (self['xep_0077'].get_registration())
			print(" recv:", iq)
			registration_form = iq['register']
			
			registration_request = self['xep_0077'].stanza.Register()
			registration_request.set_fields(registration_form.get_fields())
			for field in registration_request.get_fields():
				if field == 'username':
					registration_request[field] = self.__username
				elif field == 'password':
					registration_request[field] = self.__password
				elif field == 'email':
					registration_request[field] = 'bobby@example.net'
				else:
					print(" unsupported field:", field)
			iq = self['xep_0077'].xmpp.make_iq_set(registration_request)
			print(" send:", iq)
			print(" recv:", (await iq.send()))
			self.disconnect() # disconnect
		
		else:
			self.send_presence()
			self.get_roster()
			
			print(" login successful, unregistering")
			
			iq = await (self['xep_0077'].get_registration())
			print(" recv:", iq)
			registration_form = iq['register']
			
			registration_request = self['xep_0077'].stanza.Register()
			registration_request.set_fields(['remove'])
			iq = self['xep_0077'].xmpp.make_iq_set(registration_request)
			print(" send:", iq)
			await iq.send()
			# the server will disconnect
	
	def on_session_end(self, event):
		print("session_end", event)
		quit()
	
if __debug__ and __name__ == '__main__':
	connection = Connection('bobby@tigase.example.net/x12345', 'abcdef') # use tigase address server here, change username
	
	connection.connect()
	try:
		connection.process()
	except KeyboardInterrupt:
		connection.disconnect()
		connection.process(timeout=5)

Expected behavior

In-band registration works in a single step, the database stays consistent even when the client violates protocol.

Details (please complete the following information):

  • Tigase version: [e.g. 8.0.0]
  • JVM flavour and version [e.g. OpenJDK11]
  • Operating system/distribution/version [e.g. Linux Ubuntu 20.04]

Additional context

I believe the problem lays in the file JabberIqRegister.java line 689:

if (session.isAuthorized()) {

The server treats the first registration as password change, not account creation.

Proposed workaround:

if (session.isAuthorized() && ! session.isAnonymous()) {
Unknown commented 4 years ago

First of all:

even when the client violates protocol.

We can't guarantee that things work well if user violates the specification...

ad rem: could you share more details?

  1. which exact tigase-server version do you use?
  2. is it your deployment or some public server?
  3. could you share complete configuration file (obfuscated)?
  4. do you receive any error during registration?
  5. what do you mean by "inconsistent state"?
Unknown commented 4 years ago
  • We can't guarantee that things work well if user violates the specification...

The server still has to return the right error code and maintain the database in the right state.

  1. which exact tigase-server version do you use?

8.0.0

  1. is it your deployment or some public server?

My private deployment.

  1. could you share complete configuration file (obfuscated)?
admins = [
    'admin@localhost'
]
'config-type' = 'default'
debug = [ 'server' ]
'default-virtual-host' = 'localhost'
dataSource () {
    default () {
        uri = 'jdbc:postgresql://localhost/tigase?user=tigase&password=2938hkjd230scvsd&useSSL=false&autoCreateUser=true'
    }
}
'ext-man' () {}
http () {}
pubsub () {
    trusted = [ 'http@{clusterNode}' ]
}
s2s (active: false) {}
'sess-man' () {
    'presence-offline' () {}
}
socks5 () {}
upload () {}

I also tried the database url without autoCreateUser=true.

  1. do you receive any error during registration?

Without the autoCreateUser=true option:

2021-03-22 13:11:36.167 [jabber:iq:register Queue Worker 1]  RepositoryAccess.setRegistration()  WARNING: Problem accessing reposiotry: 
tigase.db.UserNotFoundException: User does not exist: ziom_3@localnet
	at tigase.db.jdbc.JDBCRepository.getUserUID(JDBCRepository.java:1174)
	at tigase.db.jdbc.JDBCRepository.setData(JDBCRepository.java:587)
	at tigase.db.jdbc.JDBCRepository.setData(JDBCRepository.java:637)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at tigase.stats.StatisticsInvocationHandler.invoke(StatisticsInvocationHandler.java:75)
	at com.sun.proxy.$Proxy35.setData(Unknown Source)
	at tigase.db.UserRepositoryMDImpl.setData(UserRepositoryMDImpl.java:320)
	at tigase.xmpp.RepositoryAccess.setRegistration(RepositoryAccess.java:721)
	at tigase.xmpp.impl.JabberIqRegister.doRegisterNewAccount(JabberIqRegister.java:686)
	at tigase.xmpp.impl.JabberIqRegister.process(JabberIqRegister.java:339)
	at tigase.server.xmppsession.SessionManager$ProcessorWorkerThread.process(SessionManager.java:2587)
	at tigase.util.processing.WorkerThread.run(WorkerThread.java:68)
  • what do you mean by "inconsistent state"?

If I add the autoCreateUser=true option:

  • Upon the first attempt of registration there is a user entry created in the table tig_users, but no corresponding password entry in the table tig_user_credentials.

  • After the second registration attempt the credentials get created, probably because it is interpreted as password change request.

  • After user unregistration the entries from tig_user_credentials are deleted, but the entry in tig_users stays there. Consequently, another attempt at registration works, because it is interpreted as password change.

I believe the problem lies in the check session.isAuthorized(), because it returns true even when the password check failed.

Unknown commented 4 years ago

Thank you for all the details. Could you try reproduce it with the latest stable: 8.1.x?

Unknown commented 4 years ago

The last version uses data forms instead of plain fields, so please give me some time to write the test.

Unknown commented 4 years ago

I can confirm the bug is present in the last version. The server returns the success response but the user is not created. No error is printed on the console.

Expected behavior: user is created and the server returns success, or user is not created and the server returns error.

issue 1 of 1
Type
Bug
Issue Votes (0)
Watchers (0)
Reference
tigase/_server/server-core#1434
Please wait...
Page is in error, reload to recover