Add capability to extract application role names from Windows Group Names in specified domains in JAAS implementation (resolves #1145) #1160
Conversation
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.
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.
…main information.
eekodeerder
changed the title
eekodeerder
changed the title
| @@ -77,6 +77,7 @@ The following options are supported by the module. | |||
| * debug: Set to "true" to enable debug mode. In debug mode the module will output information about successful logins, including group memberships. | |||
| * principalFormat: Specifies the name format for the principal. | |||
| * roleFormat: Specifies the name format for the role. | |||
| * mapRolesFromDomainGroups: Specifies domains for which security group names will be mapped to roles without including the domain in the role name. These roles will added in addition to roles added through the roleFormat option. In most uses cases, the need for both the roleFormat and the mapRolesFromDomainGroups should not be required. | |||
| @@ -103,6 +107,10 @@ public void initialize(final Subject initSubject, final CallbackHandler initCall | |||
| .valueOf(((String) option.getValue()).toUpperCase(Locale.ENGLISH)); | |||
| } else if ("roleFormat".equalsIgnoreCase(option.getKey())) { | |||
| this.roleFormat = PrincipalFormat.valueOf(((String) option.getValue()).toUpperCase(Locale.ENGLISH)); | |||
| } else if ("mapRolesFromDomainGroups".equalsIgnoreCase(option.getKey())) { | |||
| // Remove quotation marks as well as whitespace for robustness | |||
| this.domains = ((String) option.getValue()).replaceAll("(\")", "").replaceAll("(^\\s*)|(\\s*$)", "") | |||
There was a problem hiding this comment.
There is a test for this. I modified initialize_withOptions in waffle-jna\src\test\java\waffle\jaas\WindowsLoginModuleTest to simultaneously test this.
There was a problem hiding this comment.
I meant specifically for the stripping part. NBD
| // add Windows Groups as role principles | ||
| for (final IWindowsAccount group : windowsIdentity.getGroups()) { | ||
| this.principals.addAll(getRolePrincipals(group, this.roleFormat)); | ||
| if (domainlist.contains(group.getDomain())) { | ||
| this.principals.add(new RolePrincipal(group.getName())); | ||
| } |
There was a problem hiding this comment.
Shouldn't getRolePrincipals be responsible for adding those instead of doing it here?
There was a problem hiding this comment.
It would make sense to move it GetRolePrincipals. This method would then either not be static anymore, or require the domains to be passed as parameter (which kind of defeats the object of making it static). The better implementation will move lines 170 to 184 into a seperate method. I left it here as this is where the previous GroupPrincipal was also added.
In essence we have an WindowsIdentity which has Groups which is a set of Account. This needs to be translated into Java Principles. In essence we have UserPrincipals, GoupPrincipals (not the previous waffle GroupPrincipal, but a Principal representing the Windows Group in a domain) and RolePrincipal, which would be the security roles required by the application. Somewhere Windows groups have to be mapped to application roles.
A transformation method takes a WindowsIdentity and generates the Principals for a Subject from this identity. This includes generating the UserPrincipal and the one or more translations/mapings from the Groups to Roles. The Principles describe the "identities" of the WindowsIdentity which includes role and/or group memberships.
Doing it correctly would then actually move the creation of all Principals (or for the jna-tomcat Windowsrealm the Strings (Names/Identities) from a single WindowsIdentity into a seperate method.
A transformation method could in theory be re-used in JAAS, and the various jna-tomcat* implementations.
We could implement different LoginModules for different types of mapping or make them options on one LoginModule.
What I have implemented now, makes it possible to interchange the Tomcat UserDatabaseRealm, DataSourceRealm and JAASRealm as realms for the same application which uses Tomcat for authentication and passes the roles to a web-app, or for Tomcat itself. Without the modification, this is no possible,
|
Thought about this some more. I worry that we're adding both |
|
I mean why don't we just add a If we need more combinations, we should introduce |
The difference between the method I implemented and just adding another option is that in my implemented method, you can control the domains that specify the roles. As an example: I might want the role names (group) to be managed in the organization's domain, or as groups on the hosting server. By not specifying the domain to use, it means that if the application should be managed in the domain, I could bypass the system by creating a group with the same name on the server, or vice versa. This will create a security risk. |
I see, and agree, my suggestion was not a good one. That said, if the app wants "manager-gui", you're now enabling "DOMAIN\manager-gui" to become "manager-gui" by stripping domain. I can't help but be worried about this because "ONE\manager-gui" can become the same thing as "TWO\manager-gui" as long as we specified "ONE" and "TWO" in this magical pattern, and not using names was really what I was going for in the original implementation - those can change, while SIDs cannot. Would you consider something more specific? For example an ability to specify a mapping? Possibly as a separate filter on top of Waffle? <group-to-role-mappings>
<group-to-role-mapping>
<group-name>ONE\manager-gui</group-name>
<role-name>manager-gui</role-name>
</group-to-role-mapping>
<group-to-role-mapping>
<group-sid>S-...</group-sid>
<role-name>manager-gui</role-name>
</group-to-role-mapping>
</group-to-role-mappings> |
Yes (and no). I believe having a specific mapping is definitely a good option. Many environments provide a mapping method (Tomcat does not). I see such a mapping as an additional method that can be supported to extend Waffle capabilities. This method has the following constraints:
I basically wanted to solve a problem: We were using Waffle as one of the options (interchangeably) to do authentication for Tomcat. It worked, and now it does not work. I identified the first problem (waffle bug) and fixed that. Then (and I still cannot pinpoint where the problem originates from as operating system, jna, waffle, our own code changed) I found that I still cannot use the Tomcat JAASRealm with the WinodwsLoginModule as we were getting the domain names in there. I have implemented one option, which works for certain use cases (ours in particular). It is an extension of the existing solution and does not break or modify anything else. It does not exclude adding additional methods of mapping inside the WindowsLoginModule; it does not exclude having mappings on top of the Waffle JAAS implementation; it does not exclude any existing capabilities. |
To summarise I believe the problem is not: "How do I expose Windows Authentication (which there are no arguments about currently) and Authorisation (specifically Windows identities) to the application". The problem is: "How do I use Windows Authentication and Authorisation to manage the roles a user can play in an application?". The latter question results in a superset solution, which allows not only exposing roles as used by Windows, but more. |
|
First, thank you. I really appreciate your work here, and I come from a place of wanting to do the right thing (and not the fastest thing :)). I think I am against the magical "strip domain" version as in this PR - it introduces an option that is not generic enough and I can't help but think it's a one-way door decision of adding a feature that will bite us in the future when someone runs into wanting to do a more complex "mapping". In a mapping solution you could, for example, implement a regex version, Btw, @hazendaz ultimately makes this call as the current benevolent dictator.
Yes.
Originally I did, but you have a real problem and Waffle should solve it.
Yes.
Agreed.
Agreed.
Agreed.
Do you think the mapping implementation is better/more generic/solves more problems here? If so, give it a shot and we can compare? I know it's a lot more work, but feels like it's worth it (to someone not doing the work like me :))). |
With this I totally agree. I wrote my first program in 1982. I have been in product development since 1987. I have been doing object oriented design since 1992. Unfortunately, in terms of experience, Java ranks between the 13th to 17th language in terms of my programming experience. W.r.t. our projects that also use Java components, I am responsible for the system design. our products are long-lived and have to be supported and improved for many years. Taking short-cuts does not work, but this does not mean that the path wih less development work is the wrong one. In terms of a map versus just mapping the names from one or more domains:
For the above reasons, I do not believe mapping is more flexible or the "right" solution. It is an alternative which may be viable when you have personnel with more IT knowledge (as it is more difficult to configure/install), but there is no use case that I can think of that you cannot handle by just mapping the domain. Even replacing the Tomcat JAASRealm with a subclassed Realm which can use an object to define the map, will suffer from exactly the same constraint. You have to do part of your "mapping" of "group rights" outside of the official editor for your Group access rights (in this case Windows). You are effectively introducing an additional authorization mapping from the one we are supporting (Windows Group and User management). I still do not understand why you would want to do that, but there may be reasons outside of my reference frame. I became involved in this as nobody on our time had the time to support the client to determine why they could not get Waffle authentication working again. I have spent some of my vacation time and some time paid for by the client to get to where I am. As explained, we have no need for a mapping. In our case the client supplies, installs and maintains the system at their clients' locations. These installers are NOT computer programmers. |
|
I probably need to get a cup of coffee and read through this again. The code is rather net new. It might be useful to provide a bit more in documentation of general setup and more descriptive use case. Currently I think this PR is the descriptive part which would be lost otherwise. Is it possible @eekodeerder you could add a bit more to documentation to assist others? One unrelated item I spotted here was the README.md move. That part is tied to the .net build. Way back when, the java piece only had one pom file and using it to copy things around caused many copies. Today, we have a launcher which isn't coupled into the main build. As such, I saw that as an opportunity to clean that up so regardless of running the .net side or the java side, that file is updated if it was touched in the root of the project since the underlying piece needs it for the actual site build via maven. I'm sending that through now. So what that means to this PR, once you see that on master, please go ahead and just merge back in. That way it filters that change out and we are left just with the changes here to continue to look at. |
|
I've added this as a milestone for 3.1.0 and moved out 3.1.0 until end of February to give time to get this addressed upon further discussion. |
|
Noted, thanks. I will update the documentation. As primarily a user (not developer) of Waffle, where Windows Authentication and Authorisation is offered as one of the options to end-user organisations, I understand the decisions to be made at installation/deployment for an oranisation to be:
Some (less sophisticated) want to administer the users and roles through an application supplied interface (using an application database for user and role management set up as a Tomcat DataSourceRealm, (or even just through the tomcat-users.xml file -> the Tomcat UserDatabaseRealm). Others want to move access authorization as well as user authentication into their domain (which for all clients is a Windows environment. It is for these end-users that Waffle is used. For the sophisticated environment, access is controlled by a specific organizational unit, which in the main does not use the application, interface with the application or even maintain the application. This "application" includes Tomcat. The simple requirement is: "How do we manage which user(s) can perform the which security roles incorporated in the "application" using the tools supplied in Windows for managing access. |
|
I apologise if I have missed some of the message chain and this may be
irrelevant.
Not JAAS related, however, my usages of waffle requires the group to be
populated with the domain ie. DOMAIN\SAMACCOUNT
This allows a legacy application application to continue supporting
multiple domains.
DOMAIN01\USER01
DOMAIN02\USER01
1. stripping the domain would cause authorization failures.
2. Keeping the domain continues to let the application to continue working
with no issues.
3. For others stripping the domain loses the uniqueness of the 2 users.
…On Thu, 21 Jan 2021, 20:19 Daniel Doubrovkine (dB.) @dblockdotorg, < ***@***.***> wrote:
***@***.**** commented on this pull request.
Seems sensible, minor comments only.
------------------------------
In Docs/tomcat/TomcatWindowsLoginJAASAuthenticator.md
<#1160 (comment)>:
> @@ -77,6 +77,7 @@ The following options are supported by the module.
* debug: Set to "true" to enable debug mode. In debug mode the module will output information about successful logins, including group memberships.
* principalFormat: Specifies the name format for the principal.
* roleFormat: Specifies the name format for the role.
+* mapRolesFromDomainGroups: Specifies domains for which security group names will be mapped to roles without including the domain in the role name. These roles will added in addition to roles added through the roleFormat option. In most uses cases, the need for both the roleFormat and the mapRolesFromDomainGroups should not be required.
will *be* added
------------------------------
In Source/JNA/waffle-jna/src/main/java/waffle/jaas/WindowsLoginModule.java
<#1160 (comment)>:
> @@ -103,6 +107,10 @@ public void initialize(final Subject initSubject, final CallbackHandler initCall
.valueOf(((String) option.getValue()).toUpperCase(Locale.ENGLISH));
} else if ("roleFormat".equalsIgnoreCase(option.getKey())) {
this.roleFormat = PrincipalFormat.valueOf(((String) option.getValue()).toUpperCase(Locale.ENGLISH));
+ } else if ("mapRolesFromDomainGroups".equalsIgnoreCase(option.getKey())) {
+ // Remove quotation marks as well as whitespace for robustness
+ this.domains = ((String) option.getValue()).replaceAll("(\")", "").replaceAll("(^\\s*)|(\\s*$)", "")
needs tests
------------------------------
In Docs/tomcat/TomcatWindowsLoginJAASAuthenticator.md
<#1160 (comment)>:
> @@ -77,6 +77,7 @@ The following options are supported by the module.
* debug: Set to "true" to enable debug mode. In debug mode the module will output information about successful logins, including group memberships.
* principalFormat: Specifies the name format for the principal.
* roleFormat: Specifies the name format for the role.
+* mapRolesFromDomainGroups: Specifies domains for which security group names will be mapped to roles without including the domain in the role name. These roles will added in addition to roles added through the roleFormat option. In most uses cases, the need for both the roleFormat and the mapRolesFromDomainGroups should not be required.
backtickquote roleFormat
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#1160 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAQAQNAGBPEB4WDXFPWDORTS3CD47ANCNFSM4WMV6HRQ>
.
|
|
@pedroneil The feature as implemented lets users specify for which domain(s) to add a named role (which happens to not include the domain name), so it's not changing any existing roles unless I didn't read the code right. Since it's net new I don't think there's a problem with existing implementations. My comments are really about the approach of how to achieve that, I disagree that the option should be a list of domains, it will lead to people having exactly the problems that you have listed inadvertently, IMHO. |
... and with as little side effects or surprise behaviors as possible
I forgot this is JAAS. I don't think we need to overthink it with external files and "serialize" a map, e.g. Subsequent change can add regex support,
I think at the lowest level users will want to exercise the lowest level of control, which I think is group -> role where the names are unrelated. That's an application concern and belongs in the application configuration. You can do bulk domain* -> *, but I find that extremely dangerous and wouldn't want to impose this on users by default. |
It does not have to be a list of domain. The way I see it: domain name is the name of where the application roles are defined as Windows Groups. It identifies the "authority" the application should rely on. Providing a list allows "multiple authorities". |
After finding a lot of good reasons why not to do this, I found a valid use case (which makes sense) which will be addressed by such a Regex solution: I will try to make some time to implement. As I said, Java is not even close to my first language. |
This change does not influence users or user domains at all. Two users in different domains will never be confused. It also does not influence existing Group/Role capabilities. DOMAIN\SAMACCOUNT (even for Groups) is still available exactly as you use it now, without change, using the existing options. When using a new option, it just adds the capability to (also) provide the application role name without a domain or computer name, allowing you to get SAMACCOUNT as a role from a Windows Group DOMAIN\SAMACCOUNT. This "group" DOMAIN obviously does not have to be the same as the domain for your user. |
I am ready to change my mind (and you convinced me of various things above that I got wrong), so don't hesitate to push back. I just want the best outcome in the library.
I think people will end up using this without any regex to make things more predictable. This is telling me that they will want to not use regex and just have a file with the explicit domain name mappings, but potentially many entries. If I was coding this I'd only allow a text/tab file for the mappings, so
My first language is Russian :) I think you're doing great! Thank you. |
|
@dblock I'm kind of with you on the regex. Its too easy to mess up and hard to read. It does serve a purpose much more easily though if done correctly but I think here maybe error on side of caution in general. This is security so its a little more dicey with regex. @eekodeerder I think I fully understand now but will re-read this star to finish this weekend. |
|
I have looked at the (agree messy) Regex option. One of the problems of doing mapping like that is how to seperate the Regex (or just plain string) mappings. The proposed ";" will/may cause problems, as the ";" is used as a seperator for specifications in the Tomcat login.conf. Becomes very messy and becomes dependent on the parser in Tomcat. I also vote against a RegEx, but I do need a simple way to specify multiple application roles from one domain without having to copy it multiple times. Maybe instead of a regex, a simple wildcard can be specified: roleMappings=ONE\tomcat-two-* ? A plain domain map could be specified as roleMappings=ONE*, or just roleMappings=ONE as shorthand. Another application in the same Tomcat will have a different LoginModule configuration in login.conf. |
Always using a file and reading it line-by-line with a |
|
I think you're trying to find a perfect solution to an imperfect problem. I would implement a mapping without regex, then try to use it in production and come back for an incremental solution, regex or not, once you see all the problems - divide and conquer. |
|
Thank. you. The code I have submitted is already running in the field with end-users. |
As I said I'm against introducing |
|
closing as not originally accepted. |
In a typical application environment, the application implements security roles (by name) which is used to control access to application functions. These security role names are often application specific and should be portable if the application is to run in multiple organizational domains. As an example: Tomcat access control uses a role: "manager-gui".
To use a JAAS LoginModule and Windows Active Directory to authorise the users who can perform this Tomcat role, the Waffle WindowsLoginModule should be able to translate AD groups to which an autenticated User belongs to a RolePrincipal wih the name "manager-gui".
This pull request adds this capability to the Waffle JAAS WindowsLoginModule. A new option is added in which an installer configuring an application can specify a comma-delimited list of domain names for which the WindowsLoginModule should create RolePrinciples using only the group name without the domain name (not the fqn).
This feature does not affect any existing deployments, as the FQN and/or the SID for Windows Groups can still be included in the list of RolePrinciples delivered, in addition or instead of the new simple role names.
How do I link this pull request to Issue #1145?