Re: [os-libsynthesis] server login without credential check
Hi Patrick, On Nov 10, 2009, at 18:43 , Patrick Ohly wrote: > There are quite a bit of changes in your "luz" branch and some unmerged > ones in our "master". Should we merge the changes back and forth so that > we are in sync again? Probably yes. Now that unilib has become the main line, I'm back with the old scheme - my current work on "luz" (with a little bit of risk that something in there might not yet work in different contexts than mine) and "master" being pulled up from time to time as a more official and more conservative "release" branch. >> Now for your change - I disagree because it makes it impossible for a >> DB plugin to handle invalid, anonymous AND regular logins. > > Okay. I reverted the change. > >> The purpose >> of is just an extra security barrier to set a minimal >> login requirement. Setting it to "none" should not mean that >> necessarily *all* sync attempts need no credentials, but allows that >> *some* might not need them. In any case, the DB layer (or the >> scripts) decides about allowing a session or not on a case- >> by-case basis. > > I didn't see a possibility to do that in the DB layer, at least not with > the old code. Indeed, the really anonymous case (no credentials at all) was impossible to handle in the plugin. >> For a server that does not need any checking, just put a "return TRUE" >> into . > > That works. I added it to SyncEvolution "master". :-) Lukas Zeller (l...@synthesis.ch) - Synthesis AG, SyncML Solutions & Sustainable Software Concepts i...@synthesis.ch, http://www.synthesis.ch ___ os-libsynthesis mailing list os-libsynthesis@synthesis.ch http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis
Re: [os-libsynthesis] server login without credential check
On Mon, 2009-11-09 at 10:07 +, Lukas Zeller wrote: > I just pushed this as "d468580dac (anonymous login: improved passing > anonymous login attempts to DB Api)" to luz on indefero. There are quite a bit of changes in your "luz" branch and some unmerged ones in our "master". Should we merge the changes back and forth so that we are in sync again? > Now for your change - I disagree because it makes it impossible for a > DB plugin to handle invalid, anonymous AND regular logins. Okay. I reverted the change. > The purpose > of is just an extra security barrier to set a minimal > login requirement. Setting it to "none" should not mean that > necessarily *all* sync attempts need no credentials, but allows that > *some* might not need them. In any case, the DB layer (or the > scripts) decides about allowing a session or not on a case- > by-case basis. I didn't see a possibility to do that in the DB layer, at least not with the old code. > For a server that does not need any checking, just put a "return TRUE" > into . That works. I added it to SyncEvolution "master". -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter. ___ os-libsynthesis mailing list os-libsynthesis@synthesis.ch http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis
Re: [os-libsynthesis] server login without credential check
Hello Patrick, There are two levels. and define what is requested from the client and what is accepted from the client on the protocol level. The second level is the authentication against the backend, which in a multi-user environment normally needs to know at least a user name (or the absence of it), even if no real credential check is performed. So the DB-Plugin's SessionLogin() is normally always called. It can be prevented by explicitly returning true or false in (so a simple return TRUE in the script will let pass any login and never call SessionLogin()). Now there's a special case when the SyncML message does not provide any credentials. Even in this case, the engine does a login towards the database with a pseudo user name of "anonymous". I checked the handling of this case in the plugin API and found that it was not handled very nicely in all cases (Password_ modes). So I added better handling as follows for anonymous login: - for the modes that pass a credential *into* the plugin to check (Password_ClrText_IN and Password_MD5_IN), a empty string will be passed along with user name "anonymous". - for the modes where the plugin *returns* a credential string for the engine to check (Password_ClrText_OUT and Password_MD5_OUT), the engine checks if that returned string is empty; if it is not, anonymous login is rejected. I just pushed this as "d468580dac (anonymous login: improved passing anonymous login attempts to DB Api)" to luz on indefero. Now for your change - I disagree because it makes it impossible for a DB plugin to handle invalid, anonymous AND regular logins. The purpose of is just an extra security barrier to set a minimal login requirement. Setting it to "none" should not mean that necessarily *all* sync attempts need no credentials, but allows that *some* might not need them. In any case, the DB layer (or the scripts) decides about allowing a session or not on a case- by-case basis. For a server that does not need any checking, just put a "return TRUE" into . Best Regards, Lukas On Nov 3, 2009, at 12:01 , Patrick Ohly wrote: For the case that a client contacts the server via Bluetooth (and thus must have paired successfully) I'd like to accept the connection request regardless which credentials the client provides. I'm running with the following configuration: SyncEvolution yes yes none none yes This redirects session and device handling calls into SyncEvolution code. It always returns Password_ClrText_OUT as password mode. I see that our Session_Login() is called from here: #0 sysync::TCustomImplAgent::SessionLogin (this=0x2b3d110, aUserName=0x2aca500 "test", aAuthString=0x2aca670 "rKOc35gVO51AvKnMKMIskw==", aAuthStringType=sysync::sectyp_md5_V11, aDeviceID=0x2acaa18 "sc-pim-117cdd45-3892-4a2d-ae62-4c98cb0f3eae") at /home/pohly/syncevolution/libsynthesis/src/sysync/ customimplagent.cpp:815 #1 0x00683f81 in sysync::TSyncSession::checkCredentials (this=0x2b3d110, aUserName=0x2aca500 "test", aCred=0x2aca670 "rKOc35gVO51AvKnMKMIskw==", aAuthType=sysync::auth_md5) at /home/pohly/syncevolution/libsynthesis/src/sysync/ syncsession.cpp:4048 #2 0x00685657 in sysync::TSyncSession::checkCredentials (this=0x2b3d110, aUserName=0x2aca500 "test", aCredP=0x2aca520, astatuscomma...@0x2aca840) at /home/pohly/syncevolution/libsynthesis/src/sysync/ syncsession.cpp:3981 #3 0x00771726 in sysync::TSyncAgent::ServerMessageStarted (this=0x2b3d110, aContentP=0x2aca2d0, astatuscomma...@0x2aca840, aBad=false) at /home/pohly/syncevolution/libsynthesis/src/sysync/ syncagent.cpp:1814 #4 0x0076d7b4 in sysync::TSyncAgent::MessageStarted (this=0x2b3d110, aContentP=0x2aca2d0, astatuscomma...@0x2aca840, aBad=false) In other words, despite none none credentials are checked and have to match. Should these settings have an effect when using the DB to authenticate users? It doesn't seem that they do, but I also don't see any other way to disable the login check. Hmm, perhaps I misunderstood the meaning of these settings. In TSyncAgent::isAuthTypeAllowed() I see that fRequiredAuth is checked only to see whether the client's credentials are "strong" enough. If the client provides credentials, they still have to be correct. In TSyncSession::checkCredentials(), the relevant code is this: // first check credentials // NOTE: This must be done first, to force calling SessionLogin // in all cases authok=checkCredentials(aUserName,authdata,authtype); // now check result if (!isAuthTypeAllowed(authtype)) { aStatusCommand.setStatusCode(401); // we need another auth type, tell client which one authok=false; // anyway, reject PDEBUGPRINTFX(DBG_ERROR,("Authorization failed (wrong type of creds), sending 401 + chal")); } else if (!authok) {
[os-libsynthesis] server login without credential check
Hello! For the case that a client contacts the server via Bluetooth (and thus must have paired successfully) I'd like to accept the connection request regardless which credentials the client provides. I'm running with the following configuration: SyncEvolution yes yes none none yes This redirects session and device handling calls into SyncEvolution code. It always returns Password_ClrText_OUT as password mode. I see that our Session_Login() is called from here: #0 sysync::TCustomImplAgent::SessionLogin (this=0x2b3d110, aUserName=0x2aca500 "test", aAuthString=0x2aca670 "rKOc35gVO51AvKnMKMIskw==", aAuthStringType=sysync::sectyp_md5_V11, aDeviceID=0x2acaa18 "sc-pim-117cdd45-3892-4a2d-ae62-4c98cb0f3eae") at /home/pohly/syncevolution/libsynthesis/src/sysync/customimplagent.cpp:815 #1 0x00683f81 in sysync::TSyncSession::checkCredentials (this=0x2b3d110, aUserName=0x2aca500 "test", aCred=0x2aca670 "rKOc35gVO51AvKnMKMIskw==", aAuthType=sysync::auth_md5) at /home/pohly/syncevolution/libsynthesis/src/sysync/syncsession.cpp:4048 #2 0x00685657 in sysync::TSyncSession::checkCredentials (this=0x2b3d110, aUserName=0x2aca500 "test", aCredP=0x2aca520, astatuscomma...@0x2aca840) at /home/pohly/syncevolution/libsynthesis/src/sysync/syncsession.cpp:3981 #3 0x00771726 in sysync::TSyncAgent::ServerMessageStarted (this=0x2b3d110, aContentP=0x2aca2d0, astatuscomma...@0x2aca840, aBad=false) at /home/pohly/syncevolution/libsynthesis/src/sysync/syncagent.cpp:1814 #4 0x0076d7b4 in sysync::TSyncAgent::MessageStarted (this=0x2b3d110, aContentP=0x2aca2d0, astatuscomma...@0x2aca840, aBad=false) In other words, despite none none credentials are checked and have to match. Should these settings have an effect when using the DB to authenticate users? It doesn't seem that they do, but I also don't see any other way to disable the login check. Hmm, perhaps I misunderstood the meaning of these settings. In TSyncAgent::isAuthTypeAllowed() I see that fRequiredAuth is checked only to see whether the client's credentials are "strong" enough. If the client provides credentials, they still have to be correct. In TSyncSession::checkCredentials(), the relevant code is this: // first check credentials // NOTE: This must be done first, to force calling SessionLogin // in all cases authok=checkCredentials(aUserName,authdata,authtype); // now check result if (!isAuthTypeAllowed(authtype)) { aStatusCommand.setStatusCode(401); // we need another auth type, tell client which one authok=false; // anyway, reject PDEBUGPRINTFX(DBG_ERROR,("Authorization failed (wrong type of creds), sending 401 + chal")); } else if (!authok) { // auth type allowed, but auth itself not ok aStatusCommand.setStatusCode(401); // unauthorized, bad credentials PDEBUGPRINTFX(DBG_ERROR,("Authorization failed (invalid credentials) sending 401 + chal")); } if (!authok) { // - add challenge aStatusCommand.setChallenge(newSessionChallenge()); } Would it be okay to add something like this before "else if (!authok)"? It solves the problem for me. if (!authok && isAuthTypeAllowed(auth_none)) { authok = true; } That changes the meaning of none from "all kinds of authentication methods allowed, including none" to "all kinds of credentials allowed, including invalid ones". Because a client with invalid credentials was already allowed to connect by not sending them, this doesn't make a server setup less secure. It has the advantage that the client setup becomes easier. I added my patch to the "master" branch on moblin.org, because it is needed for SyncEvolution. Please merge if you agree with this change. -- Best Regards, Patrick Ohly The content of this message is my personal opinion only and although I am an employee of Intel, the statements I make here in no way represent Intel's position on the issue, nor am I authorized to speak on behalf of Intel on this matter. ___ os-libsynthesis mailing list os-libsynthesis@synthesis.ch http://lists.synthesis.ch/mailman/listinfo/os-libsynthesis