[jboss-jira] [JBoss JIRA] Commented: (SECURITY-339) ClientLoginModule improperly handles SecurityAssociation stack in abort()
[
https://jira.jboss.org/jira/browse/SECURITY-339?page=com.atlassian.jira.p...
]
Marco Schulze commented on SECURITY-339:
----------------------------------------
I just took a look at
http://fisheye.jboss.org/browse/JBossAS/projects/security/security-jboss-...
and the code is still wrong!
You wrote in your comment "On abort, the principal information if any should be
popped from the Security Context." This is *NOT* correct, because the login module
must only pop things which it pushed itself before. Otherwise it pops sth. that has been
pushed by *ANOTHER* login process before.
I have - before submitting this issue - stepped through the code thoroughly and saw that
the methods called by a normal login are as follows:
A) if successful:
1) ClientLoginModule.login()
2) ClientLoginModule.commit()
B) on error:
1) ClientLoginModule.login()
2) ClientLoginModule.abort()
Since the current code pushes the principal onto the stack in the commit() method (i.e.
only if successful) but the abort() method pops it even though no commit() happened, the
stack is wrong after one single authentication failure!
ClientLoginModule improperly handles SecurityAssociation stack in
abort()
-------------------------------------------------------------------------
Key: SECURITY-339
URL: https://jira.jboss.org/jira/browse/SECURITY-339
Project: JBoss Security and Identity Management
Issue Type: Bug
Security Level: Public(Everyone can see)
Components: JBossSX
Reporter: Marco Schulze
Assignee: Anil Saldhana
Fix For: JBossSecurity_2.0.4
The abort() method calls SecurityAssociationActions.popPrincipalInfo() even though the
corresponding push happens in commit() [via
SecurityAssociationActions.setPrincipalInfo(loginPrincipal, loginCredential, subject)].
That means, whenever a login fails, the commit is not called (thus nothing pushed), but
the abort pops out an element from the stack. This should not be done. IMHO the abort()
method should look like this:
public boolean abort() throws LoginException
{
if( trace )
log.trace("abort");
if( restoreLoginIdentity == false )
{
// Clear the entire security association stack
SecurityAssociationActions.clear();
}
return true;
}
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
https://jira.jboss.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira