Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WindowsLoginModule version 1.8.x and later does not work with Tomcat JAASRealm to identify roles from Windows securitygroups #1114

Closed
eekodeerder opened this issue Dec 11, 2020 · 3 comments
Labels
Milestone

Comments

@eekodeerder
Copy link
Contributor

eekodeerder commented Dec 11, 2020

The documented configuration for Tomcat with the JAASRealm and WindowsLoginModule does not work. No RolePrincipal(s) are found in the JAASRealm. This is due to the fact that the WindowsLoginModule packages all Windows Groups as RolePrincipals in a list which is then added as a GroupPrincipal.
The JAASRealm (configured to accept waffle.jaas.RolePrinciple as RolePrinciples), fails to identify any roles.

Tomcat 9. Waffle for Tomcat 9 (and earliers). It seems the problem was introduced in version 1.8.x according to the code, as it was not there in 1.7.x.
The bug was introduced in by devnullpointer on October 16 2016. He removed the line which added the RolePrinciples when he added code to add a GroupPrincipal with all the RolePrinciples.

@eekodeerder eekodeerder changed the title WindowsLoginModule does not work with Tomcat JAASRealm to identify roles from Windows securitygroups WindowsLoginModule version 1.8.x and later does not work with Tomcat JAASRealm to identify roles from Windows securitygroups Dec 11, 2020
@dblock dblock added the bug? label Dec 11, 2020
@eekodeerder
Copy link
Contributor Author

I am a bit new to this. How do I determine the process for fixing this bug? I can fix it, but need to understand the process
We have successfuly used Waffle in a project for Windows Authentication with Tomcat 5 about 8 years ago. The project has gone through several phases, but the Windows authentication option has now re-appeared. It took me two days to pinpoint the problem, but I need to fix it and I have time pressure.
My options are:

  1. to locally branch the project and bring it under our source control with CI/CD. In this case I would probably make a custom LoginModule class which looks like the previous WindowsLoginModule. I don't like the idea of locally branching from an open-source project, if we only need to use the functionality provided (if it was provided -> which it is not anymore).
  2. modify the WindowsLoginModule by adding the removed line again. Unfortunately my knowledge of Waffle and its uses are not wide enough to know if this will now break somebody else's use case of the class (as the previous modification did). I also don't know how to get the code into a release for the project. My take is that a use case will either use the GroupPrincipal "Roles" in the Subject to find the roles (I am not sure which use cases use this), or the RolePrincipal(s) themselves (such as Tomcat JAASRealm), but cannot think of a use cases which will try both (my humble opinion).
  3. Introduce the original WindowsLoginModule as a new OriginalWindowsLoginModule. This will break documentation (including Tomcat documentation for many versions of Tomcat), but suffers from the same levels of my ignorance on the process.
    Is it possible for me to commit changes to the project or to request changes to be committed? How long will it typically take to get this into a build? How can I get the change into the project, as I believe currently it does not work as advertised (especially for Tomcat).

@dblock
Copy link
Collaborator

dblock commented Dec 12, 2020

Let's not focus too much on which line to add/remove. This is a bug and we should do (2) and do our best to not break someone else.

eekodeerder added a commit to eekodeerder/waffle that referenced this issue Dec 19, 2020
The existing code for WindowsLoginModule (since October 2016) does not bind the Windows (or Active DIrectory) group (names) to the Subject. It binds an object named "Roles" to the Subject. It therefore does not fulfill the basic function required of a JAAS WindowsLoginModule.  Refer to Issue Waffle#1114.

The packaging into a GroupPrincipal (which did not exist before the breaking change), was made to get it to work with WildFly (earlier versions as it does not work with latest versions in any case). Every Windows Group to which the User belongs, was placed into a new list of Principals (named "Roles") which was then packaged as a Principal (the GroupPrincipal) and added to the Subject.Principals list. This is just plain wrong, as it assumes that any user of the subject will know about the custom "Roles", and be able to extract the Principals from this list, This assumption is obviously invalid. The JAAS specified way is to obtain it from the Subject.Principles.getName(). The code modification also removed the ability to obtain the Windows Group (or JAAS roles) from the Subject.Principles.getName() as specified by JAAS.
eekodeerder added a commit to eekodeerder/waffle that referenced this issue Dec 19, 2020
eekodeerder added a commit to eekodeerder/waffle that referenced this issue Dec 19, 2020
…d Windows Groups as Principles

The existing code for WindowsLoginModule (since October 2016) does not bind the Windows (or Active DIrectory) group (names) to the Subject. It binds an object named "Roles" to the Subject. It therefore does not fulfill the basic function required of a JAAS WindowsLoginModule.  Refer to Issue Waffle#1114

The packaging into a GroupPrincipal (which did not exist before the breaking change), was made to get it to work with WildFly (earlier versions as it does not work with latest versions in any case). Every Windows Group to which the User belongs, was placed into a new list of Principals (named "Roles") which was then packaged as a Principal (the GroupPrincipal) and added to the Subject.Principals list. This is just plain wrong, as it assumes that any user of the subject will know about the custom "Roles", and be able to extract the Principals from this list, This assumption is obviously invalid. The JAAS specified way is to obtain it from the Subject.Principles.getName(). The code modification also removed the ability to obtain the Windows Group (or JAAS roles) from the Subject.Principles.getName() as specified by JAAS.
@hazendaz
Copy link
Member

Fixed in 3.0.0 and released tonight. The binaries should show up in central in 2 hours.

@hazendaz hazendaz added this to the 3.0.0 milestone Dec 29, 2020
eekodeerder added a commit to eekodeerder/waffle that referenced this issue Jan 5, 2021
eekodeerder added a commit to eekodeerder/waffle that referenced this issue Jan 6, 2021
The existing code for WindowsLoginModule (since October 2016) does not bind the Windows (or Active DIrectory) group (names) to the Subject. It binds an object named "Roles" to the Subject. It therefore does not fulfill the basic function required of a JAAS WindowsLoginModule.  Refer to Issue Waffle#1114.

The packaging into a GroupPrincipal (which did not exist before the breaking change), was made to get it to work with WildFly (earlier versions as it does not work with latest versions in any case). Every Windows Group to which the User belongs, was placed into a new list of Principals (named "Roles") which was then packaged as a Principal (the GroupPrincipal) and added to the Subject.Principals list. This is just plain wrong, as it assumes that any user of the subject will know about the custom "Roles", and be able to extract the Principals from this list, This assumption is obviously invalid. The JAAS specified way is to obtain it from the Subject.Principles.getName(). The code modification also removed the ability to obtain the Windows Group (or JAAS roles) from the Subject.Principles.getName() as specified by JAAS.
hazendaz pushed a commit that referenced this issue Jan 18, 2021
* Removed the incorrect GroupPrincipal "Roles" from the code

The existing code for WindowsLoginModule (since October 2016) does not bind the Windows (or Active DIrectory) group (names) to the Subject. It binds an object named "Roles" to the Subject. It therefore does not fulfill the basic function required of a JAAS WindowsLoginModule.  Refer to Issue #1114.

The packaging into a GroupPrincipal (which did not exist before the breaking change), was made to get it to work with WildFly (earlier versions as it does not work with latest versions in any case). Every Windows Group to which the User belongs, was placed into a new list of Principals (named "Roles") which was then packaged as a Principal (the GroupPrincipal) and added to the Subject.Principals list. This is just plain wrong, as it assumes that any user of the subject will know about the custom "Roles", and be able to extract the Principals from this list, This assumption is obviously invalid. The JAAS specified way is to obtain it from the Subject.Principles.getName(). The code modification also removed the ability to obtain the Windows Group (or JAAS roles) from the Subject.Principles.getName() as specified by JAAS.

* Fixed the tests to exclude the incorrect GroupPrinciple named "Roles"

* Update WindowsLoginModuleTest.java

* Corrected the tests to accept Windows groups as (Role)Principles

Refer to issue #1114.

* Fixed the changed name in the assert.

* Fixed the failing tests

* Delete GroupPrincipal.java

GroupPrincipal breaks the JAAS Principal and should be removed.

* Delete the GroupPrincipal tests as as the GroupPrincipal was deleted

* Removed the incorrect GroupPrincipal "Roles" from the code

The existing code for WindowsLoginModule (since October 2016) does not bind the Windows (or Active DIrectory) group (names) to the Subject. It binds an object named "Roles" to the Subject. It therefore does not fulfill the basic function required of a JAAS WindowsLoginModule.  Refer to Issue #1114.

The packaging into a GroupPrincipal (which did not exist before the breaking change), was made to get it to work with WildFly (earlier versions as it does not work with latest versions in any case). Every Windows Group to which the User belongs, was placed into a new list of Principals (named "Roles") which was then packaged as a Principal (the GroupPrincipal) and added to the Subject.Principals list. This is just plain wrong, as it assumes that any user of the subject will know about the custom "Roles", and be able to extract the Principals from this list, This assumption is obviously invalid. The JAAS specified way is to obtain it from the Subject.Principles.getName(). The code modification also removed the ability to obtain the Windows Group (or JAAS roles) from the Subject.Principles.getName() as specified by JAAS.

* Fixed the tests to exclude the incorrect GroupPrinciple named "Roles"

* Removed the warnings on specific desrialization tests.

* Disabled dserialization ban on serializable test.

* Suppressed warnings when testing specifically for Serializability.

Co-authored-by: Frederick Peter Eek <48279754+FreekEek@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants