[ https://issues.jboss.org/browse/SECURITY-659?page=com.atlassian.jira.plug... ] SBS JIRA Integration updated SECURITY-659: ------------------------------------------ Forum Reference: https://community.jboss.org/thread/200652, https://community.jboss.org/message/739801#739801 (was: https://community.jboss.org/thread/200652) -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/SECURITY-659?page=com.atlassian.jira.plug... ] arjan tijms updated SECURITY-659: --------------------------------- Description: In JBoss AS 7.1.1, if a user provided {{ServerAuthModule}} provides a {{GroupPrincipalCallback}}, then this is ignored by {{WebJASPIAuthenticator}}. The provided handler copies the {{GroupPrincipalCallback}}, but the authenticator then does nothing with it. Simulteanously, if the {{ServerAuthModule}} does not provide a {{PasswordValidationCallback}} to the handler, then this will result in a null pointer exception in the authenticator. Regarding the ignored {{GroupPrincipalCallback}}, the problem seems to be in the following code: {code:title=WebJASPIAuthenticator#authenticate} // ... if (result) { PasswordValidationCallback pvc = cbh.getPasswordValidationCallback(); CallerPrincipalCallback cpc = cbh.getCallerPrincipalCallback(); // get the client principal from the callback. Principal clientPrincipal = cpc.getPrincipal(); if (clientPrincipal == null) { clientPrincipal = new SimplePrincipal(cpc.getName()); } // if the client principal is not a jboss generic principal, we need to build one before registering. if (!(clientPrincipal instanceof JBossGenericPrincipal)) clientPrincipal = this.buildJBossPrincipal(clientSubject, clientPrincipal); {code} {{buildJBossPrincipal()}} looks at the "Roles" group in the Subject, but this hasn't been set by either the handler or other code based on what the GroupPrincipalCallback contains. I wonder if changing this into the following would be more correct: {code:title=WebJASPIAuthenticator#authenticate} // ... if (result) { PasswordValidationCallback pvc = cbh.getPasswordValidationCallback(); CallerPrincipalCallback cpc = cbh.getCallerPrincipalCallback(); GroupPrincipalCallback gpc = cbh.getGroupPrincipalCallback(); // ADDED // get the client principal from the callback. Principal clientPrincipal = cpc.getPrincipal(); if (clientPrincipal == null) { clientPrincipal = new SimplePrincipal(cpc.getName()); } // if the client principal is not a jboss generic principal, we need to build one before registering. if (!(clientPrincipal instanceof JBossGenericPrincipal)) clientPrincipal = this.buildJBossPrincipal(clientSubject, clientPrincipal, gpc); // ADDED gpc PARAMETER {code} With {{buildJBossPrincipal()}} implemented as: {code:title=WebJASPIAuthenticator#buildJBossPrincipal} protected Principal buildJBossPrincipal(Subject subject, Principal principal, GroupPrincipalCallback groupPrincipalCallback) { List<String> roles = new ArrayList<String>(); // look for roles in the subject first. for (Principal p : subject.getPrincipals()) { if (p instanceof Group && p.getName().equals("Roles")) { Enumeration<? extends Principal> members = ((Group) p).members(); while (members.hasMoreElements()) roles.add(members.nextElement().getName()); } } // START ADDED if (groupPrincipalCallback != null && groupPrincipalCallback.getGroups() != null) { for (String group : groupPrincipalCallback.getGroups()) { roles.add(group); } } // END ADDED // if the subject didn't contain any roles, look for the roles declared in the deployment descriptor. JBossWebRealm realm = (JBossWebRealm) this.getContainer().getRealm(); Set<String> descriptorRoles = realm.getPrincipalVersusRolesMap().get(principal.getName()); if (roles.isEmpty() && descriptorRoles != null) roles.addAll(descriptorRoles); // build and return the JBossGenericPrincipal. return new JBossGenericPrincipal(realm, principal.getName(), null, roles, principal, null, null, null, subject); } {code} As for the PasswordValidationCallback, WebJASPIAuthenticator now contains the following code in {{authenticate()}}: {code:title=WebJASPIAuthenticator#authenticate} this.register(request, response, clientPrincipal, authMethod, pvc.getUsername(), new String(pvc.getPassword())); {code} The {{register()}} method considers both username and password as optional, but because there's no null check on {{pvc}}, the above line will throw a NPE in case no PasswordValidationCallback is provided. This could perhaps be changed into something like the following: {code:title=WebJASPIAuthenticator#authenticate} this.register(request, response, clientPrincipal, authMethod, pvc != null ? pvc.getUsername() : null, pvc != null && pvc.getPassword() != null ? new String(pvc.getPassword()) : null); {code} was: In JBoss AS 7.1.1, if a user provided {{ServerAuthModule}} provides a {{GroupPrincipalCallback}}, then this is ignored by {{WebJASPIAuthenticator}}. The provided handler copies the {{GroupPrincipalCallback}}, but the authenticator then does nothing with it. Simulteanously, if the {{ServerAuthModule}} does not provide a {{PasswordValidationCallback}} to the handler, then this will result in a null pointer exception in the authenticator. Regarding the ignored {{GroupPrincipalCallback}}, the problem seems to be in the following code: {code:title=WebJASPIAuthenticator#authenticate} // ... if (result) { PasswordValidationCallback pvc = cbh.getPasswordValidationCallback(); CallerPrincipalCallback cpc = cbh.getCallerPrincipalCallback(); // get the client principal from the callback. Principal clientPrincipal = cpc.getPrincipal(); if (clientPrincipal == null) { clientPrincipal = new SimplePrincipal(cpc.getName()); } // if the client principal is not a jboss generic principal, we need to build one before registering. if (!(clientPrincipal instanceof JBossGenericPrincipal)) clientPrincipal = this.buildJBossPrincipal(clientSubject, clientPrincipal); {code} {{buildJBossPrincipal()}} looks at the "Roles" group in the Subject, but this hasn't been set by either the handler or other code based on what the GroupPrincipalCallback contains. I wonder if changing this into the following would be more correct: {code:title=WebJASPIAuthenticator#authenticate} // ... if (result) { PasswordValidationCallback pvc = cbh.getPasswordValidationCallback(); CallerPrincipalCallback cpc = cbh.getCallerPrincipalCallback(); GroupPrincipalCallback gpc = cbh.getGroupPrincipalCallback(); // ADDED // get the client principal from the callback. Principal clientPrincipal = cpc.getPrincipal(); if (clientPrincipal == null) { clientPrincipal = new SimplePrincipal(cpc.getName()); } // if the client principal is not a jboss generic principal, we need to build one before registering. if (!(clientPrincipal instanceof JBossGenericPrincipal)) clientPrincipal = this.buildJBossPrincipal(clientSubject, clientPrincipal, gpc); // ADDED gpc PARAMETER {code} With {{buildJBossPrincipal()}} implemented as: {code:title=WebJASPIAuthenticator#buildJBossPrincipal} protected Principal buildJBossPrincipal(Subject subject, Principal principal, GroupPrincipalCallback groupPrincipalCallback) { List<String> roles = new ArrayList<String>(); // look for roles in the subject first. for (Principal p : subject.getPrincipals()) { if (p instanceof Group && p.getName().equals("Roles")) { Enumeration<? extends Principal> members = ((Group) p).members(); while (members.hasMoreElements()) roles.add(members.nextElement().getName()); } } // START ADDED if (groupPrincipalCallback != null && groupPrincipalCallback.getGroups() != null) { for (String group : groupPrincipalCallback.getGroups()) { roles.add(group); } } // END ADDED // if the subject didn't contain any roles, look for the roles declared in the deployment descriptor. JBossWebRealm realm = (JBossWebRealm) this.getContainer().getRealm(); Set<String> descriptorRoles = realm.getPrincipalVersusRolesMap().get(principal.getName()); if (roles.isEmpty() && descriptorRoles != null) roles.addAll(descriptorRoles); // build and return the JBossGenericPrincipal. return new JBossGenericPrincipal(realm, principal.getName(), null, roles, principal, null, null, null, subject); } {code} As for the PasswordValidationCallback, WebJASPIAuthenticator now contains the following code in {{authenticate()}}: {code:title=WebJASPIAuthenticator#authenticate} this.register(request, response, clientPrincipal, authMethod, pvc.getUsername(), new String(pvc.getPassword())); {code} The {{register()}} method considers both username and password as optional, but because there's no null check on {{pvc}}, the above line will throw a NPE in case no PasswordValidationCallback is provided. This could perhaps be changed into something like the following: {code:title=WebJASPIAuthenticator#authenticate} this.register(request, response, clientPrincipal, authMethod, pvc != null ? pvc.getUsername() : null, pvc != null ? new String(pvc.getPassword()) : null); {code} Forum Reference: https://community.jboss.org/thread/200652, https://community.jboss.org/message/739801#739801 (was: https://community.jboss.org/thread/200652, https://community.jboss.org/message/739801#739801) -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.jboss.org/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://issues.jboss.org/browse/AS7-5735?page=com.atlassian.jira.plugin.s... ] Josef Cacek updated AS7-5735: ----------------------------- Fix Version/s: 7.2.0.Alpha1 7.1.4.Final (EAP) Git Pull Request: https://github.com/jbossas/jboss-as/pull/3245, https://github.com/jbossas/jboss-as/pull/3246 Fix from Stefan is already some time in the repository - both master and 7.1 branch. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira