syncevo-dbus-server: removed special case for unauthenticated Connections
The extra "mustAuthenticate" parameter to the Session::sync() method is replaced with temporarily setting username/password to empty via the Session::m_syncFilter. To catch undesired prototype changes of D-Bus API calls (as it happened when adding that parameter), all these methods now come with a comment that links them to the corresponding D-Bus API call. test-dbus.py tests cover the different credential checks: - TestConnection.testStartSync: invalid MD5 creds, no authorization necessary => client accepted - TestConnection.testCredentialsWrong: invalid MD5 creds, authorization necessary => client rejected - TestConnection.testCredentialsRight: correct basic creds, authorization necessary => client accepted
This commit is contained in:
Patrick Ohly
parent
332a67b8a5
commit
4cbe91d39e
|
|
@ -184,8 +184,10 @@ public:
|
|||
void getReports(uint32_t start, uint32_t count,
|
||||
Reports_t &reports);
|
||||
|
||||
/** Session.CheckSource() */
|
||||
void checkSource(const string &sourceName);
|
||||
|
||||
/** Session.GetDatabases() */
|
||||
void getDatabases(const string &sourceName, SourceDatabases_t &databases);
|
||||
};
|
||||
|
||||
|
|
@ -265,11 +267,14 @@ class DBusServer : public DBusObjectHelper
|
|||
*/
|
||||
std::string getNextSession();
|
||||
|
||||
/** Server.Attach() */
|
||||
void attachClient(const Caller_t &caller,
|
||||
const boost::shared_ptr<Watch> &watch);
|
||||
|
||||
/** Server.Detach() */
|
||||
void detachClient(const Caller_t &caller);
|
||||
|
||||
/** Server.Connect() */
|
||||
void connect(const Caller_t &caller,
|
||||
const boost::shared_ptr<Watch> &watch,
|
||||
const StringMap &peer,
|
||||
|
|
@ -277,11 +282,13 @@ class DBusServer : public DBusObjectHelper
|
|||
const std::string &session,
|
||||
DBusObject_t &object);
|
||||
|
||||
/** Server.StartSession() */
|
||||
void startSession(const Caller_t &caller,
|
||||
const boost::shared_ptr<Watch> &watch,
|
||||
const std::string &server,
|
||||
DBusObject_t &object);
|
||||
|
||||
/** Server.GetConfig() */
|
||||
void getConfig(const std::string &config_name,
|
||||
bool getTemplate,
|
||||
ReadOperations::Config_t &config)
|
||||
|
|
@ -290,6 +297,7 @@ class DBusServer : public DBusObjectHelper
|
|||
ops.getConfig(getTemplate , config);
|
||||
}
|
||||
|
||||
/** Server.GetReports() */
|
||||
void getReports(const std::string &config_name,
|
||||
uint32_t start, uint32_t count,
|
||||
ReadOperations::Reports_t &reports)
|
||||
|
|
@ -298,13 +306,16 @@ class DBusServer : public DBusObjectHelper
|
|||
ops.getReports(start, count, reports);
|
||||
}
|
||||
|
||||
/** Server.CheckPresence() */
|
||||
void checkPresence(const std::string &server,
|
||||
std::string &status,
|
||||
std::vector<std::string> &transports);
|
||||
|
||||
/** Server.SessionChanged */
|
||||
EmitSignal2<const DBusObject_t &,
|
||||
bool> sessionChanged;
|
||||
|
||||
/** Server.PresenceChanged */
|
||||
EmitSignal3<const std::string &,
|
||||
const std::string &,
|
||||
const std::string &> presence;
|
||||
|
|
@ -821,14 +832,18 @@ class Session : public DBusObjectHelper,
|
|||
Timer m_statusTimer;
|
||||
Timer m_progressTimer;
|
||||
|
||||
/** Session.Detach() */
|
||||
void detach(const Caller_t &caller);
|
||||
|
||||
/** Session.SetConfig() */
|
||||
void setConfig(bool update, bool temporary,
|
||||
const ReadOperations::Config_t &config);
|
||||
|
||||
/** Session.GetStatus() */
|
||||
void getStatus(std::string &status,
|
||||
uint32_t &error,
|
||||
SourceStatuses_t &sources);
|
||||
/** Session.GetProgress() */
|
||||
void getProgress(int32_t &progress,
|
||||
SourceProgresses_t &sources);
|
||||
|
||||
|
|
@ -847,9 +862,11 @@ class Session : public DBusObjectHelper,
|
|||
/** like fireStatus() for progress information */
|
||||
void fireProgress(bool flush = false);
|
||||
|
||||
/** Session.StatusChanged */
|
||||
EmitSignal3<const std::string &,
|
||||
uint32_t,
|
||||
const SourceStatuses_t &> emitStatus;
|
||||
/** Session.ProgressChanged */
|
||||
EmitSignal2<int32_t,
|
||||
const SourceProgresses_t &> emitProgress;
|
||||
|
||||
|
|
@ -923,9 +940,11 @@ public:
|
|||
int32_t extra1, int32_t extra2, int32_t extra3);
|
||||
|
||||
typedef StringMap SourceModes_t;
|
||||
void sync(const std::string &mode, const SourceModes_t &source_modes, bool mustAuthenticate);
|
||||
void clientSync(const std::string &mode, const SourceModes_t &source_modes) { sync(mode, source_modes, true); }
|
||||
/** Session.Sync() */
|
||||
void sync(const std::string &mode, const SourceModes_t &source_modes);
|
||||
/** Session.Abort() */
|
||||
void abort();
|
||||
/** Session.Suspend() */
|
||||
void suspend();
|
||||
|
||||
bool isSuspend() { return m_syncStatus == SYNC_SUSPEND; }
|
||||
|
|
@ -998,15 +1017,20 @@ class Connection : public DBusObjectHelper, public Resource
|
|||
*/
|
||||
static std::string buildDescription(const StringMap &peer);
|
||||
|
||||
/** Connection.Process() */
|
||||
void process(const Caller_t &caller,
|
||||
const std::pair<size_t, const uint8_t *> &message,
|
||||
const std::string &message_type);
|
||||
/** Connection.Close() */
|
||||
void close(const Caller_t &caller,
|
||||
bool normal,
|
||||
const std::string &error);
|
||||
/** wrapper around sendAbort */
|
||||
void abort();
|
||||
/** Connection.Abort */
|
||||
EmitSignal0 sendAbort;
|
||||
bool m_abortSent;
|
||||
/** Connection.Reply */
|
||||
EmitSignal5<const std::pair<size_t, const uint8_t *> &,
|
||||
const std::string &,
|
||||
const StringMap &,
|
||||
|
|
@ -1031,6 +1055,9 @@ public:
|
|||
|
||||
/** connection is no longer needed, ensure that it gets deleted */
|
||||
void shutdown();
|
||||
|
||||
/** peer is not trusted, must authenticate as part of SyncML */
|
||||
bool mustAuthenticate() const { return m_mustAuthenticate; }
|
||||
};
|
||||
|
||||
/**
|
||||
|
|
@ -1449,7 +1476,7 @@ void Session::initServer(SharedBuffer data, const std::string &messageType)
|
|||
m_initialMessageType = messageType;
|
||||
}
|
||||
|
||||
void Session::sync(const std::string &mode, const SourceModes_t &source_modes, bool mustAuthenticate)
|
||||
void Session::sync(const std::string &mode, const SourceModes_t &source_modes)
|
||||
{
|
||||
if (!m_active) {
|
||||
SE_THROW_EXCEPTION(InvalidCall, "session is not active, call not allowed at this time");
|
||||
|
|
@ -1464,7 +1491,12 @@ void Session::sync(const std::string &mode, const SourceModes_t &source_modes, b
|
|||
m_sync->initServer(m_sessionID,
|
||||
m_initialMessage,
|
||||
m_initialMessageType);
|
||||
m_sync->setMustAuthenticate(mustAuthenticate);
|
||||
boost::shared_ptr<Connection> c = m_connection.lock();
|
||||
if (c && !c->mustAuthenticate()) {
|
||||
// unsetting username/password disables checking them
|
||||
m_syncFilter["password"] = "";
|
||||
m_syncFilter["username"] = "";
|
||||
}
|
||||
}
|
||||
|
||||
// Apply temporary config filters. The parameters of this function
|
||||
|
|
@ -1626,7 +1658,7 @@ Session::Session(DBusServer &server,
|
|||
add(&m_operations, &ReadOperations::getReports, "GetReports");
|
||||
add(&m_operations, &ReadOperations::checkSource, "CheckSource");
|
||||
add(&m_operations, &ReadOperations::getDatabases, "GetDatabases");
|
||||
add(this, &Session::clientSync, "Sync");
|
||||
add(this, &Session::sync, "Sync");
|
||||
add(this, &Session::abort, "Abort");
|
||||
add(this, &Session::suspend, "Suspend");
|
||||
add(this, &Session::getStatus, "GetStatus");
|
||||
|
|
@ -2372,7 +2404,7 @@ Connection::~Connection()
|
|||
void Connection::ready()
|
||||
{
|
||||
// proceed with sync now that our session is ready
|
||||
m_session->sync(m_syncMode, m_sourceModes, m_mustAuthenticate);
|
||||
m_session->sync(m_syncMode, m_sourceModes);
|
||||
}
|
||||
|
||||
/****************** DBusTransportAgent implementation **************/
|
||||
|
|
|
|||
|
|
@ -132,7 +132,6 @@ SyncContext::SyncContext(const string &server,
|
|||
m_doLogging(doLogging),
|
||||
m_quiet(false),
|
||||
m_dryrun(false),
|
||||
m_mustAuthenticate(true),
|
||||
m_serverMode(false)
|
||||
{
|
||||
}
|
||||
|
|
@ -1487,7 +1486,7 @@ void SyncContext::getConfigXML(string &xml, string &configname)
|
|||
const char *user = getUsername();
|
||||
const char *password = getPassword();
|
||||
|
||||
if (m_mustAuthenticate && (user[0] || password[0])) {
|
||||
if (user[0] || password[0]) {
|
||||
// require authentication with the configured password
|
||||
substTag(xml, "defaultauth",
|
||||
"<requestedauth>md5</requestedauth>\n"
|
||||
|
|
|
|||
|
|
@ -83,7 +83,6 @@ class SyncContext : public SyncConfig, public ConfigUserInterface {
|
|||
bool m_quiet;
|
||||
bool m_dryrun;
|
||||
|
||||
bool m_mustAuthenticate;
|
||||
bool m_serverMode;
|
||||
std::string m_sessionID;
|
||||
SharedBuffer m_initialMessage;
|
||||
|
|
@ -161,10 +160,6 @@ class SyncContext : public SyncConfig, public ConfigUserInterface {
|
|||
bool getDryRun() { return m_dryrun; }
|
||||
void setDryRun(bool dryrun) { m_dryrun = dryrun; }
|
||||
|
||||
/** only for server: does the client have to provide valid credentials? */
|
||||
bool getMustAuthenticate() { return m_mustAuthenticate; }
|
||||
void setMustAuthenticate(bool mustAuthenticate) { m_mustAuthenticate = mustAuthenticate; }
|
||||
|
||||
/** only for server: device ID of peer */
|
||||
void setSyncDeviceID(const std::string &deviceID) { m_syncDeviceID = deviceID; }
|
||||
std::string getSyncDeviceID() const { return m_syncDeviceID; }
|
||||
|
|
|
|||
|
|
@ -308,20 +308,14 @@ TSyError SyncEvolution_Session_Login( CContext sContext, cAppCharP sUsername, ap
|
|||
}
|
||||
TSyError res = DB_Forbidden;
|
||||
string user = sc->getUsername();
|
||||
string password = sc->getPassword();
|
||||
|
||||
if (!sc->getMustAuthenticate()) {
|
||||
// nothing to check, accept peer, without even querying what our password is
|
||||
if (user.empty() && password.empty()) {
|
||||
// nothing to check, accept peer
|
||||
res = LOCERR_OK;
|
||||
} else if (user == sUsername) {
|
||||
*sPassword=StrAlloc(password.c_str());
|
||||
res = LOCERR_OK;
|
||||
} else {
|
||||
string password = sc->getPassword();
|
||||
|
||||
if (user.empty() && password.empty()) {
|
||||
// nothing to check, accept peer
|
||||
res = LOCERR_OK;
|
||||
} else if (user == sUsername) {
|
||||
*sPassword=StrAlloc(password.c_str());
|
||||
res = LOCERR_OK;
|
||||
}
|
||||
}
|
||||
|
||||
SE_LOG_DEBUG(NULL, NULL, "Session_Login usr='%s' expected user='%s' res=%d",
|
||||
|
|
|
|||
|
|
@ -480,7 +480,7 @@ class TestDBusSyncError(unittest.TestCase, DBusUtil):
|
|||
class TestConnection(unittest.TestCase, DBusUtil):
|
||||
"""Tests Server.Connect(). Tests depend on getting one Abort signal to terminate."""
|
||||
|
||||
"""a real message sent to our own server, DevInf stripped"""
|
||||
"""a real message sent to our own server, DevInf stripped, username/password foo/bar"""
|
||||
message1 = '''<?xml version="1.0" encoding="UTF-8"?><SyncML xmlns='SYNCML:SYNCML1.2'><SyncHdr><VerDTD>1.2</VerDTD><VerProto>SyncML/1.2</VerProto><SessionID>255</SessionID><MsgID>1</MsgID><Target><LocURI>http://127.0.0.1:9000/syncevolution</LocURI></Target><Source><LocURI>sc-api-nat</LocURI><LocName>test</LocName></Source><Cred><Meta><Format xmlns='syncml:metinf'>b64</Format><Type xmlns='syncml:metinf'>syncml:auth-md5</Type></Meta><Data>kHzMn3RWFGWSKeBpXicppQ==</Data></Cred><Meta><MaxMsgSize xmlns='syncml:metinf'>20000</MaxMsgSize><MaxObjSize xmlns='syncml:metinf'>4000000</MaxObjSize></Meta></SyncHdr><SyncBody><Alert><CmdID>1</CmdID><Data>200</Data><Item><Target><LocURI>addressbook</LocURI></Target><Source><LocURI>./addressbook</LocURI></Source><Meta><Anchor xmlns='syncml:metinf'><Last>20091105T092757Z</Last><Next>20091105T092831Z</Next></Anchor><MaxObjSize xmlns='syncml:metinf'>4000000</MaxObjSize></Meta></Item></Alert><Final/></SyncBody></SyncML>'''
|
||||
|
||||
def setUp(self):
|
||||
|
|
@ -491,10 +491,10 @@ class TestConnection(unittest.TestCase, DBusUtil):
|
|||
def run(self, result):
|
||||
self.runTest(result, own_xdg=False)
|
||||
|
||||
def getConnection(self):
|
||||
def getConnection(self, must_authenticate=False):
|
||||
conpath = self.server.Connect({'description': 'test-dbus.py',
|
||||
'transport': 'dummy'},
|
||||
False,
|
||||
must_authenticate,
|
||||
"")
|
||||
self.setUpConnectionListeners(conpath)
|
||||
connection = dbus.Interface(bus.get_object('org.syncevolution',
|
||||
|
|
@ -530,6 +530,56 @@ class TestConnection(unittest.TestCase, DBusUtil):
|
|||
# TODO: check events
|
||||
self.failIfEqual(self.reply, None)
|
||||
self.failUnlessEqual(self.reply[1], 'application/vnd.syncml+xml')
|
||||
# credentials should have been accepted because must_authenticate=False
|
||||
# in Connect(); 508 = "refresh required" is normal
|
||||
self.failUnless('<Status><CmdID>2</CmdID><MsgRef>1</MsgRef><CmdRef>1</CmdRef><Cmd>Alert</Cmd><TargetRef>addressbook</TargetRef><SourceRef>./addressbook</SourceRef><Data>508</Data>' in self.reply[0])
|
||||
self.failIf('<Chal>' in self.reply[0])
|
||||
self.failUnlessEqual(self.reply[3], False)
|
||||
self.failIfEqual(self.reply[4], '')
|
||||
connection.Close(False, 'good bye')
|
||||
loop.run()
|
||||
loop.run()
|
||||
self.failUnlessEqual(self.quit_events, ["connection " + conpath + " aborted",
|
||||
"session done"])
|
||||
|
||||
def testCredentialsWrong(self):
|
||||
"""send invalid credentials"""
|
||||
conpath, connection = self.getConnection(must_authenticate=True)
|
||||
connection.Process(TestConnection.message1, 'application/vnd.syncml+xml')
|
||||
loop.run()
|
||||
self.failUnlessEqual(self.quit_events, ["connection " + conpath + " got reply"])
|
||||
self.quit_events = []
|
||||
# TODO: check events
|
||||
self.failIfEqual(self.reply, None)
|
||||
self.failUnlessEqual(self.reply[1], 'application/vnd.syncml+xml')
|
||||
# credentials should have been rejected because of wrong Nonce
|
||||
self.failUnless('<Chal>' in self.reply[0])
|
||||
self.failUnlessEqual(self.reply[3], False)
|
||||
self.failIfEqual(self.reply[4], '')
|
||||
connection.Close(False, 'good bye')
|
||||
# when the login fails, the server also ends the session
|
||||
loop.run()
|
||||
loop.run()
|
||||
loop.run()
|
||||
self.quit_events.sort()
|
||||
self.failUnlessEqual(self.quit_events, ["connection " + conpath + " aborted",
|
||||
"connection " + conpath + " got final reply",
|
||||
"session done"])
|
||||
|
||||
def testCredentialsRight(self):
|
||||
"""send correct credentials"""
|
||||
conpath, connection = self.getConnection(must_authenticate=True)
|
||||
plain_auth = TestConnection.message1.replace("<Type xmlns='syncml:metinf'>syncml:auth-md5</Type></Meta><Data>kHzMn3RWFGWSKeBpXicppQ==</Data>",
|
||||
"<Type xmlns='syncml:metinf'>syncml:auth-basic</Type></Meta><Data>dGVzdDp0ZXN0</Data>")
|
||||
connection.Process(plain_auth, 'application/vnd.syncml+xml')
|
||||
loop.run()
|
||||
self.failUnlessEqual(self.quit_events, ["connection " + conpath + " got reply"])
|
||||
self.quit_events = []
|
||||
self.failIfEqual(self.reply, None)
|
||||
self.failUnlessEqual(self.reply[1], 'application/vnd.syncml+xml')
|
||||
# credentials should have been accepted because with basic auth,
|
||||
# credentials can be replayed; 508 = "refresh required" is normal
|
||||
self.failUnless('<Status><CmdID>2</CmdID><MsgRef>1</MsgRef><CmdRef>1</CmdRef><Cmd>Alert</Cmd><TargetRef>addressbook</TargetRef><SourceRef>./addressbook</SourceRef><Data>508</Data>' in self.reply[0])
|
||||
self.failUnlessEqual(self.reply[3], False)
|
||||
self.failIfEqual(self.reply[4], '')
|
||||
connection.Close(False, 'good bye')
|
||||
|
|
|
|||
Loading…
Reference in New Issue