Welcome to the new JSR-375 mailing list


Will Hopkins
 

Hi Arjan,

On 05/16/2017 06:35 PM, Arjan Tijms wrote:
Hi Will,

On Tue, May 16, 2017 at 11:26 PM, Will Hopkins <will.hopkins@...> wrote:

OK, yhat seems like a good approach, I'll make sure it's spec'd that way. I don't think I'll spec the idea about requiring the container to use an app-supplied IdentityStore -- you've convinced me about the FORM and BASIC annotations, so that's how the app can leverage an IdentityStore for FORM and BASIC.

How do you feel about specifing that HAMs are only required to be enabled when the app is deployed with <auth-method>AUTHMECH</auth-method>, though, to ensure that containers can continue support legacy apps using the existing code paths, and to ensure deterministic behavior when, for example, two implementations of FORM or BASIC are still present in the container. Containers could still choose to provide just one impl, and to implement all functionality via HAM, and apps could ensure they always got HAM by specifying the AUTHMECH, but legacy behavior got be safely preserved if a container vendor chose.

By itself this would be okay, but there are a few caveats.

First is that the CDI bean representing the HAM is initially enabled via a CDI extension. Despite the best efforts of both the CDI and Servlet EGs, nobody succeeded in finding a solution for the fact that currently CDI extensions are not guaranteed to be able to read web.xml.

So without <auth-method>AUTHMECH</auth-method> we should be prepared for the beans still being around, but just not actually used for authentication (when it comes time to install the JASPIC bridge, web.xml can be read).

Second is that specifically simpler applications are trending to a no-XML/no web.xml solution. Requiring a web.xml only to activate JSR 375 is a bit of a step back then. This caveat could be reduced though by introducing an annotation to set the auth mechanism and/or extending the javax.servlet.ServletContext with an option to set the auth mechanism programmatically.

Hmm. I see the problem(s). Let me give that some thought.

So the light-bulb went off a little while ago. I had been thinking in terms of bank-level security, general-purpose architecture for token issuance, integration with container session management,etc. -- then I remembered that my bank won't let me stay logged in longer than 20 minutes, but lots of other sites have little "Remember Me" checkboxes, and I understood the use case. It's really not a distinct architectural function, it's more like a feature added onto authentication, so the current implementation makes sense. Sorry for being so dense about it.

No worries, I'm happy it's clear now. I've had the light-bulb moment myself on multiple occasions too ;)

 
Let's not add anything else new.  ;)

Okay, sure I agree with that.

 
I guess I see them all as functionally distinct now. The first analogy that comes to mind is the hierarchy of storage for a CPU -- the CPU operates on data in registers; data is loaded into registers from the L2 cache, when there's an L2 cache miss the data is loaded from core memory, and data that's not in core memory is read from disk.

Indeed, that's a totally fitting analogy.

 
So, on to the next topic -- there are a couple other interfaces I think need to be moved, removed, and/or tweaked:
  • There was discussion on the list a while back about EmbeddedIdentityStore, and whether it represented a security risk and/or an anti-pattern. Although there was not complete agreement within the EG, there was support for the idea that it's not a good practice, and that we shouldn't encourage it by including it in the API. It has value for some development use cases, but those use cases could be easily satisfied by including it in the RI, and ultimately it's pretty trivial to write an actual IdentityStore implementation that has hard-coded user data and/or reads from a file, keystore, or other embedded repository.
I agree with the concern, and intend to remove it from the spec (i.e., API javadoc).

Well, as mentioned I don't 100% share the concern, although I do see the issue. The fact is that every other application server out there already provides the equivalent to the embedded identity store, though mostly using some sort of file (property or annotation mostly). But since there indeed was not complete agreement about this in the EG, we probably should give this some more thought.

But since we unfortunately don't have the time for that, I (somewhat reluctantly) agree to move this to the RI then. It's late at my place now, but I'll make the attempt tomorrow then.

Great, thanks.

 
  • For ServletContext, I intend to:
    • Remove the newly added methods (getAllDeclaredCallerRoles() and hasAccessToXXX())
(p.s. I think this is about SecurityContext and not ServletContext, but that's probably just a typo)

Yes, SecurityContext. (I was tired when I wrote this -- there were several typos and I missed that HttpMessageContextWrapper actually did wrap something.)

I'm not sure what the need is for removing those, specifically the hasAccessTo methods. The latter ones are quite often requested by users. Even so much that over at OmniFaces we implemented those by parsing web.xml. This feature has been received with much enthusiasm by our users and is often used. We introduced it a while back so have experience with it for some time as has the general public and wider community.

It's quite important to have the hasAccessTo() methods for e.g. rendering menus in web applications (omit links users don't have access to), or to render links to protected pages in a different way (e.g. in red or with a lock symbol).

At a high level I don't disagree with the usefulness of the functionality, but I do have some concerns about the specifics:
  • The methods were added to the API only recently, at a point in the schedule where we needed to be constraining scope, not adding to it.
  • Depending on the container, the getAllDeclaredRoles() method could be difficult to implement, might perform poorly, or might provide inconsistent or unhelpful results. I'm thinking specifically of environments that do dynamic role mapping, such that generating the list would require evaluating every applicable role-mapping policy, and where the set of roles returned might be meaningless because the question "what roles?" was asked in an abstract or arbitrary context, rather than the context of an actual access control query, for a specific resource, in a specific context.
I also have mixed feelings about the usefulness of getting all of a user's roles, outside of a user- or role-management application. What the caller probably wants to know is, "what set of resources does this user have access to". An alternative way to get that answer is to query which of a set of resources the user can access. (Or query a set of roles -- a method isUserInRoles(Set<String> roles) might make sense.)
  • Some concerns with hasAccessToXXX():
    • The first concern has to do with the web-container-centric naming and signatures. As I understand it, SecurityContext is supposed to be more or less generic, and provide methods that are broadly applicable across all container types in which SecurityContext is available. The authenticate() method is already an exception to this, unfortunately, but if we continue down this path, we'll have a single SecurityContext for all containers, but it will be a fat, kitchen-sink API, with a proliferation of hasAccesstoEJBMethod(), hasAccessToRestResource(), hasAccessToJMSQueue(), etc., methods.
    • That brings me to the second concern -- there is no uniform representation of a resource that can be authorized. A string-based representation may make sense, but, if so, we need to define what the format of that string is, such that it can uniquely and unambiguously identify any Java EE resource, allows for flexible binding of authorization policies to resources, and efficient evaluation of policy at runtime, while remaining human readable and reasonably easy for developers to manipulate in code. Alternatively, we may be better served by an attribute-based model such that, for example, a URI and an HTTP method can be represented as separate (string) attributes as part of a "Resource" object. I'm not trying to argue for a specific representation, I'm just saying that we need one. Whatever that looks like, we should ultimately end up with, at most, one or two methods for authorization:
hasAccessToResource(Resource resource)
hasAccessToResources(Set<Resource> resources)
    • The question of *who* hasAccessToXXX() isn't really addressed. Implicitly, it's the Subject currently on the stack, and maybe that's all we need, but we should consider the question. What happens if there is no authenticated user in the context? What if we want to find out if User A has access to Resource X, where User A is not the currently authenticated user? (Asking that question should probably be allowed only for privileged users, but we should consider whether it's appropriate for the API.)
In short, while the functionality would be very useful, and I don't doubt that similar methods have worked well for, and been well-received by, your users, I think more thought needs to go into these APIs before they're standardized.

See:


Note that in OmniFaces we don't use JACC. I know there's a concern about JACC since it's not enabled by default in WLS (which as I think I mentioned is actually a spec violation), but for this feature JACC is not required. It's just trivial to implement using JACC, and I used that in the RI. But WLS and Liberty (the other server that doesn't have JACC enabled by default) can use proprietary code there, or even copy the code from OmniFaces if they want. 

Alternatively I could also provide the OmniFaces implementation in the RI for the hasAccessTo() methods.

 
    • As previously agreed, remove the "response only" signature for authenticate (authenticate(HttpServletResponse, AuthenticationParameters)).

Okay, I'll remove that one tomorrow too then.

Thanks!

 
    • Specify that ServletContext is supported only in EJB and Servlet containers, or possibly a *short* list of additional containers (likely the web-based containers) where it's known that the getCallerPrincipal() and isCallerInRole() methods can be implemented by layering on existing container APIs.
Okay, with the note that you probably mean SecurityContext here and not ServletContext.

Yes.

  • AuthenticationStatus -- weren't we going to align that with the JASPIC return codes, so that the correspondence would be completely clear even though we weren't re-using the actual type/constants?

Okay, I'm somewhat more in favour of the current names, but we indeed discussed that, so I'm going to make that change too then.

Thanks!

  • AuthenticationContext -- this is a bit of a kitchen sink. I guess we can live with that, but we at least need to provide javadoc for all of the getters/setters. As the author, you're best positioned to do so, I think.
Which class would that exactly be? There's no AuthenticationContext, but there is a HttpMessageContext. Did you meant that one?

Sorry, I meant javax/security/authentication/mechanism/http/AuthenticationParameters.java.

  • Would BaseMessageContext be a better name for MessageContextWrapper? The class doesn't actually wrap anything, it simply implements the MessageContext interface. (Both classes could use some javadoc.)

HttpMessageContextWrapper actually does wrapping, see e.g.

Yes, my mistake.

  • I'll probably add some fields to the LdapIdentityStoreDefinition annotation to allow developers to specify a particular search string (LDAP syntax) and map a particular LDAP attribute to the caller principal name.

Okay, thanks!

Thanks, Arjan. Appreciate all your help and your patience with my questions!

Regards,

Will

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803


Will Hopkins
 

A couple additional questions/concerns:
  • Should IdentityStores (and/or IdentityStoreHandler) be made available in containers other than the Servlet container? They're intended for use by HAMs, which are only present in the Servlet container, but they could in theory be useful for login to EJBs, etc.
I'm inclined to say Servlet-only, to keep things simple, and expand to other containers in a future JSR if there's demand for it.
  • Some issues with credentials:
  • The Password class, and several related credentials (UsernamePasswordCredential, BasicAuthenticationCredential) do not handle passwords securely -- there are constructors and methods that accept or return passwords as strings. Passwords should always be handled as char[], since String values can't be cleared from memory; they can hang around indefinitely. This would apply also to a base64-decoded basic auth string -- it should be decoded to a char[] to avoid having the password portion exposed in memory.
  • The TokenCredential and CallerOnlyCredential are not referenced anywhere in the API or the RI. Given that, we should consider whether they're really needed.
    • I have mixed feelings about the value of a CallerOnlyCredential (and whether it should be called CallerCredential, or CallerNameCredential). If we do keep it, we may want to provide a constructor/accessors that take CallerPrincipal as an argument.
    • It's not clear what value TokenCredential is providing. It doesn't represent any real token type, and it makes an assumption that any token can be usefully (and safely) represented as a string. Wouldn't it be better to let those implementing support for a particular type of token write their own Credential type?
  • I'm not sure the getCaller() method should be on the Credential interface, as it may not be possible to directly obtain a caller for all Credential types. It's probably not needed for most credentials, and I suspect the vast majority -- UsernamePasswordCredential and related types aside -- will end up just returning "" (or null -- the correct behavior isn't clear). The Credential interface should only have operations that can be generically performed for any Credential type, and should make no assumptions about the form, structure, or content of any Credential.
There are only 3 calls to getCaller() in all of the code (test code excluded), and they're all invocations on UsernamePasswordCredential. Since IdentityStores explicitly choose the types of credentials they handle, they'll be prepared to deal with specific types, like UsernamePasswordCredential. It makes more sense to me to remove getCaller() from Credential, and add getUsername to UsernamePasswordCredential, rather than force all Credential types to implement a method they don't need and possibly can't support.
Thoughts?

Will


On 05/17/2017 12:28 AM, Will Hopkins wrote:
Hi Arjan,

On 05/16/2017 06:35 PM, Arjan Tijms wrote:
Hi Will,

On Tue, May 16, 2017 at 11:26 PM, Will Hopkins <will.hopkins@...> wrote:

OK, yhat seems like a good approach, I'll make sure it's spec'd that way. I don't think I'll spec the idea about requiring the container to use an app-supplied IdentityStore -- you've convinced me about the FORM and BASIC annotations, so that's how the app can leverage an IdentityStore for FORM and BASIC.

How do you feel about specifing that HAMs are only required to be enabled when the app is deployed with <auth-method>AUTHMECH</auth-method>, though, to ensure that containers can continue support legacy apps using the existing code paths, and to ensure deterministic behavior when, for example, two implementations of FORM or BASIC are still present in the container. Containers could still choose to provide just one impl, and to implement all functionality via HAM, and apps could ensure they always got HAM by specifying the AUTHMECH, but legacy behavior got be safely preserved if a container vendor chose.

By itself this would be okay, but there are a few caveats.

First is that the CDI bean representing the HAM is initially enabled via a CDI extension. Despite the best efforts of both the CDI and Servlet EGs, nobody succeeded in finding a solution for the fact that currently CDI extensions are not guaranteed to be able to read web.xml.

So without <auth-method>AUTHMECH</auth-method> we should be prepared for the beans still being around, but just not actually used for authentication (when it comes time to install the JASPIC bridge, web.xml can be read).

Second is that specifically simpler applications are trending to a no-XML/no web.xml solution. Requiring a web.xml only to activate JSR 375 is a bit of a step back then. This caveat could be reduced though by introducing an annotation to set the auth mechanism and/or extending the javax.servlet.ServletContext with an option to set the auth mechanism programmatically.

Hmm. I see the problem(s). Let me give that some thought.

So the light-bulb went off a little while ago. I had been thinking in terms of bank-level security, general-purpose architecture for token issuance, integration with container session management,etc. -- then I remembered that my bank won't let me stay logged in longer than 20 minutes, but lots of other sites have little "Remember Me" checkboxes, and I understood the use case. It's really not a distinct architectural function, it's more like a feature added onto authentication, so the current implementation makes sense. Sorry for being so dense about it.

No worries, I'm happy it's clear now. I've had the light-bulb moment myself on multiple occasions too ;)

 
Let's not add anything else new.  ;)

Okay, sure I agree with that.

 
I guess I see them all as functionally distinct now. The first analogy that comes to mind is the hierarchy of storage for a CPU -- the CPU operates on data in registers; data is loaded into registers from the L2 cache, when there's an L2 cache miss the data is loaded from core memory, and data that's not in core memory is read from disk.

Indeed, that's a totally fitting analogy.

 
So, on to the next topic -- there are a couple other interfaces I think need to be moved, removed, and/or tweaked:
  • There was discussion on the list a while back about EmbeddedIdentityStore, and whether it represented a security risk and/or an anti-pattern. Although there was not complete agreement within the EG, there was support for the idea that it's not a good practice, and that we shouldn't encourage it by including it in the API. It has value for some development use cases, but those use cases could be easily satisfied by including it in the RI, and ultimately it's pretty trivial to write an actual IdentityStore implementation that has hard-coded user data and/or reads from a file, keystore, or other embedded repository.
I agree with the concern, and intend to remove it from the spec (i.e., API javadoc).

Well, as mentioned I don't 100% share the concern, although I do see the issue. The fact is that every other application server out there already provides the equivalent to the embedded identity store, though mostly using some sort of file (property or annotation mostly). But since there indeed was not complete agreement about this in the EG, we probably should give this some more thought.

But since we unfortunately don't have the time for that, I (somewhat reluctantly) agree to move this to the RI then. It's late at my place now, but I'll make the attempt tomorrow then.

Great, thanks.

 
  • For ServletContext, I intend to:
    • Remove the newly added methods (getAllDeclaredCallerRoles() and hasAccessToXXX())
(p.s. I think this is about SecurityContext and not ServletContext, but that's probably just a typo)

Yes, SecurityContext. (I was tired when I wrote this -- there were several typos and I missed that HttpMessageContextWrapper actually did wrap something.)

I'm not sure what the need is for removing those, specifically the hasAccessTo methods. The latter ones are quite often requested by users. Even so much that over at OmniFaces we implemented those by parsing web.xml. This feature has been received with much enthusiasm by our users and is often used. We introduced it a while back so have experience with it for some time as has the general public and wider community.

It's quite important to have the hasAccessTo() methods for e.g. rendering menus in web applications (omit links users don't have access to), or to render links to protected pages in a different way (e.g. in red or with a lock symbol).

At a high level I don't disagree with the usefulness of the functionality, but I do have some concerns about the specifics:
  • The methods were added to the API only recently, at a point in the schedule where we needed to be constraining scope, not adding to it.
  • Depending on the container, the getAllDeclaredRoles() method could be difficult to implement, might perform poorly, or might provide inconsistent or unhelpful results. I'm thinking specifically of environments that do dynamic role mapping, such that generating the list would require evaluating every applicable role-mapping policy, and where the set of roles returned might be meaningless because the question "what roles?" was asked in an abstract or arbitrary context, rather than the context of an actual access control query, for a specific resource, in a specific context.
I also have mixed feelings about the usefulness of getting all of a user's roles, outside of a user- or role-management application. What the caller probably wants to know is, "what set of resources does this user have access to". An alternative way to get that answer is to query which of a set of resources the user can access. (Or query a set of roles -- a method isUserInRoles(Set<String> roles) might make sense.)
  • Some concerns with hasAccessToXXX():
    • The first concern has to do with the web-container-centric naming and signatures. As I understand it, SecurityContext is supposed to be more or less generic, and provide methods that are broadly applicable across all container types in which SecurityContext is available. The authenticate() method is already an exception to this, unfortunately, but if we continue down this path, we'll have a single SecurityContext for all containers, but it will be a fat, kitchen-sink API, with a proliferation of hasAccesstoEJBMethod(), hasAccessToRestResource(), hasAccessToJMSQueue(), etc., methods.
    • That brings me to the second concern -- there is no uniform representation of a resource that can be authorized. A string-based representation may make sense, but, if so, we need to define what the format of that string is, such that it can uniquely and unambiguously identify any Java EE resource, allows for flexible binding of authorization policies to resources, and efficient evaluation of policy at runtime, while remaining human readable and reasonably easy for developers to manipulate in code. Alternatively, we may be better served by an attribute-based model such that, for example, a URI and an HTTP method can be represented as separate (string) attributes as part of a "Resource" object. I'm not trying to argue for a specific representation, I'm just saying that we need one. Whatever that looks like, we should ultimately end up with, at most, one or two methods for authorization:
hasAccessToResource(Resource resource)
hasAccessToResources(Set<Resource> resources)
    • The question of *who* hasAccessToXXX() isn't really addressed. Implicitly, it's the Subject currently on the stack, and maybe that's all we need, but we should consider the question. What happens if there is no authenticated user in the context? What if we want to find out if User A has access to Resource X, where User A is not the currently authenticated user? (Asking that question should probably be allowed only for privileged users, but we should consider whether it's appropriate for the API.)
In short, while the functionality would be very useful, and I don't doubt that similar methods have worked well for, and been well-received by, your users, I think more thought needs to go into these APIs before they're standardized.

See:


Note that in OmniFaces we don't use JACC. I know there's a concern about JACC since it's not enabled by default in WLS (which as I think I mentioned is actually a spec violation), but for this feature JACC is not required. It's just trivial to implement using JACC, and I used that in the RI. But WLS and Liberty (the other server that doesn't have JACC enabled by default) can use proprietary code there, or even copy the code from OmniFaces if they want. 

Alternatively I could also provide the OmniFaces implementation in the RI for the hasAccessTo() methods.

 
    • As previously agreed, remove the "response only" signature for authenticate (authenticate(HttpServletResponse, AuthenticationParameters)).

Okay, I'll remove that one tomorrow too then.

Thanks!

 
    • Specify that ServletContext is supported only in EJB and Servlet containers, or possibly a *short* list of additional containers (likely the web-based containers) where it's known that the getCallerPrincipal() and isCallerInRole() methods can be implemented by layering on existing container APIs.
Okay, with the note that you probably mean SecurityContext here and not ServletContext.

Yes.

  • AuthenticationStatus -- weren't we going to align that with the JASPIC return codes, so that the correspondence would be completely clear even though we weren't re-using the actual type/constants?

Okay, I'm somewhat more in favour of the current names, but we indeed discussed that, so I'm going to make that change too then.

Thanks!

  • AuthenticationContext -- this is a bit of a kitchen sink. I guess we can live with that, but we at least need to provide javadoc for all of the getters/setters. As the author, you're best positioned to do so, I think.
Which class would that exactly be? There's no AuthenticationContext, but there is a HttpMessageContext. Did you meant that one?

Sorry, I meant javax/security/authentication/mechanism/http/AuthenticationParameters.java.

  • Would BaseMessageContext be a better name for MessageContextWrapper? The class doesn't actually wrap anything, it simply implements the MessageContext interface. (Both classes could use some javadoc.)

HttpMessageContextWrapper actually does wrapping, see e.g.

Yes, my mistake.

  • I'll probably add some fields to the LdapIdentityStoreDefinition annotation to allow developers to specify a particular search string (LDAP syntax) and map a particular LDAP attribute to the caller principal name.

Okay, thanks!

Thanks, Arjan. Appreciate all your help and your patience with my questions!

Regards,

Will

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803


Arjan Tijms
 

Hi,

On Wed, May 17, 2017 at 11:53 PM, Will Hopkins <will.hopkins@...> wrote:
A couple additional questions/concerns:
  • Should IdentityStores (and/or IdentityStoreHandler) be made available in containers other than the Servlet container? They're intended for use by HAMs, which are only present in the Servlet container, but they could in theory be useful for login to EJBs, etc.
I'm inclined to say Servlet-only, to keep things simple, and expand to other containers in a future JSR if there's demand for it.
There's nothing in the IdentityStore or the IdentityStoreHandler that makes it Servlet specific, or specific to any other container. The initial design was largely by Alex, with some revisions from many in the EG, and it being a protocol (message layer) independent artefact was the one thing everybody always agreed upon.

At the moment the only artefact that uses it is indeed from the Servlet container, since we only happen to have specified the HttpAuthenticationMechanism.

The SOAP authentication mechanism (based on EJB) could just as well use it, as would the existing container proprietary EJB remote login systems, and even the JAX-RS non-Servlet variant.

However early on the decision was made (mostly as stated by Alex again) to emphasise Servlet usage since SOAP as you know is not that much uses anymore for new projects, and neither is remote EJB (plus there isn't any kind of message layer SPI standardised for remote EJB), and my guess is that 99.99% of the JAX-RS implementations out there just use Servlet, so there's no really big need there either.

Long story short, the IdentityStore and IdentityStoreHandler are just CDI artefacts and there's no specific spec text needed I think to either demand them being available in other containers than Servlet or to restrict them to Servlet. Just saying it's an enabled CDI bean should be all that's needed.


 
  • Some issues with credentials:
  • The Password class, and several related credentials (UsernamePasswordCredential, BasicAuthenticationCredential) do not handle passwords securely -- there are constructors and methods that accept or return passwords as strings. Passwords should always be handled as char[], since String values can't be cleared from memory; they can hang around indefinitely. This would apply also to a base64-decoded basic auth string -- it should be decoded to a char[] to avoid having the password portion exposed in memory.
Well, we have been thinking about that, but I'm personally not 100% convinced you're really protecting against anything by doing that for server side apps. If an attacker would ever get that level of access so that the attacker is able to snoop into server memory, I think all bets are already off.

Plus, the HtppServletRequest.getParameter is a String already too, so what are you really protecting against?

The advise seems to originate from the old idea of the AS as a kind of OS managing a diverse set of apps that are potentially hostile to each other and to the AS.

But IMHO server apps that are potentially hostile to each other should be isolated at an entirely different level, e.g. at least via OS processes, but if they are truly hostile via virtual machines on a hypervisor.

When we discussed this earlier pretty much everyone agreed that char[] doesn't buy you much for server side apps. The char[] rule for passwords, while well known, doesn't seem to be universally accepted as a best practice at all for server side apps. (even Bill Shanon uses strings for passwords in GF, I noticed ;))

That said, I'm not against them, but I don't see much value in them either.
 
  • The TokenCredential and CallerOnlyCredential are not referenced anywhere in the API or the RI. Given that, we should consider whether they're really needed.

True, there were several ideas around TokenCredential including using them as base for the JWT support that we hoped to be able to get in, but we never got to them (Rudy made a proof of concept though, but it wasn't included in the spec proper).
 
    • I have mixed feelings about the value of a CallerOnlyCredential (and whether it should be called CallerCredential, or CallerNameCredential). If we do keep it, we may want to provide a constructor/accessors that take CallerPrincipal as an argument.
The concept around CallerOnlyCredential is incredible useful and addresses a very real requirement in real world applications, namely to be able to re-authenticate (refresh authentication) within a request, e.g. in response to callers changing their name or performing an action that gives them extra groups and thus roles. For several RunAS situations these are quite important as well, since a system not rarely has to run code for a specific caller, but does not have the credential of that user.

Indeed, we currently don't reference it, so I'm a bit torn here. It's definitely something that custom authentication mechanisms could use, it's just that the ones we currently ship don't use it.

So I agree, if we keep it, let's provide a validate method for them that use them. If somehow this fails (e.g. because we run out of time), then let's remove it for now.
 
    • It's not clear what value TokenCredential is providing. It doesn't represent any real token type, and it makes an assumption that any token can be usefully (and safely) represented as a string. Wouldn't it be better to let those implementing support for a particular type of token write their own Credential type?
Maybe, I'm a bit torn here too, but since we have to make decisions and can't think forever about everything I'll remove this one then.

 
  • I'm not sure the getCaller() method should be on the Credential interface, as it may not be possible to directly obtain a caller for all Credential types. It's probably not needed for most credentials, and I suspect the vast majority -- UsernamePasswordCredential and related types aside -- will end up just returning "" (or null -- the correct behavior isn't clear). The Credential interface should only have operations that can be generically performed for any Credential type, and should make no assumptions about the form, structure, or content of any Credential.
There are only 3 calls to getCaller() in all of the code (test code excluded), and they're all invocations on UsernamePasswordCredential. Since IdentityStores explicitly choose the types of credentials they handle, they'll be prepared to deal with specific types, like UsernamePasswordCredential. It makes more sense to me to remove getCaller() from Credential, and add getUsername to UsernamePasswordCredential, rather than force all Credential types to implement a method they don't need and possibly can't support.

True, I vaguely remember some deeper ideas behind this, but this one originates from the very first days of the JSR, possibly the oldest code of all. I think you're right here, so let me just remove this one too then.

Should anyone really object against this later it's easy enough to put it back before the FR is filed.

Kind regards,
Arjan Tijms

 
Thoughts?

Will



On 05/17/2017 12:28 AM, Will Hopkins wrote:
Hi Arjan,

On 05/16/2017 06:35 PM, Arjan Tijms wrote:
Hi Will,

On Tue, May 16, 2017 at 11:26 PM, Will Hopkins <will.hopkins@...> wrote:

OK, yhat seems like a good approach, I'll make sure it's spec'd that way. I don't think I'll spec the idea about requiring the container to use an app-supplied IdentityStore -- you've convinced me about the FORM and BASIC annotations, so that's how the app can leverage an IdentityStore for FORM and BASIC.

How do you feel about specifing that HAMs are only required to be enabled when the app is deployed with <auth-method>AUTHMECH</auth-method>, though, to ensure that containers can continue support legacy apps using the existing code paths, and to ensure deterministic behavior when, for example, two implementations of FORM or BASIC are still present in the container. Containers could still choose to provide just one impl, and to implement all functionality via HAM, and apps could ensure they always got HAM by specifying the AUTHMECH, but legacy behavior got be safely preserved if a container vendor chose.

By itself this would be okay, but there are a few caveats.

First is that the CDI bean representing the HAM is initially enabled via a CDI extension. Despite the best efforts of both the CDI and Servlet EGs, nobody succeeded in finding a solution for the fact that currently CDI extensions are not guaranteed to be able to read web.xml.

So without <auth-method>AUTHMECH</auth-method> we should be prepared for the beans still being around, but just not actually used for authentication (when it comes time to install the JASPIC bridge, web.xml can be read).

Second is that specifically simpler applications are trending to a no-XML/no web.xml solution. Requiring a web.xml only to activate JSR 375 is a bit of a step back then. This caveat could be reduced though by introducing an annotation to set the auth mechanism and/or extending the javax.servlet.ServletContext with an option to set the auth mechanism programmatically.

Hmm. I see the problem(s). Let me give that some thought.

So the light-bulb went off a little while ago. I had been thinking in terms of bank-level security, general-purpose architecture for token issuance, integration with container session management,etc. -- then I remembered that my bank won't let me stay logged in longer than 20 minutes, but lots of other sites have little "Remember Me" checkboxes, and I understood the use case. It's really not a distinct architectural function, it's more like a feature added onto authentication, so the current implementation makes sense. Sorry for being so dense about it.

No worries, I'm happy it's clear now. I've had the light-bulb moment myself on multiple occasions too ;)

 
Let's not add anything else new.  ;)

Okay, sure I agree with that.

 
I guess I see them all as functionally distinct now. The first analogy that comes to mind is the hierarchy of storage for a CPU -- the CPU operates on data in registers; data is loaded into registers from the L2 cache, when there's an L2 cache miss the data is loaded from core memory, and data that's not in core memory is read from disk.

Indeed, that's a totally fitting analogy.

 
So, on to the next topic -- there are a couple other interfaces I think need to be moved, removed, and/or tweaked:
  • There was discussion on the list a while back about EmbeddedIdentityStore, and whether it represented a security risk and/or an anti-pattern. Although there was not complete agreement within the EG, there was support for the idea that it's not a good practice, and that we shouldn't encourage it by including it in the API. It has value for some development use cases, but those use cases could be easily satisfied by including it in the RI, and ultimately it's pretty trivial to write an actual IdentityStore implementation that has hard-coded user data and/or reads from a file, keystore, or other embedded repository.
I agree with the concern, and intend to remove it from the spec (i.e., API javadoc).

Well, as mentioned I don't 100% share the concern, although I do see the issue. The fact is that every other application server out there already provides the equivalent to the embedded identity store, though mostly using some sort of file (property or annotation mostly). But since there indeed was not complete agreement about this in the EG, we probably should give this some more thought.

But since we unfortunately don't have the time for that, I (somewhat reluctantly) agree to move this to the RI then. It's late at my place now, but I'll make the attempt tomorrow then.

Great, thanks.

 
  • For ServletContext, I intend to:
    • Remove the newly added methods (getAllDeclaredCallerRoles() and hasAccessToXXX())
(p.s. I think this is about SecurityContext and not ServletContext, but that's probably just a typo)

Yes, SecurityContext. (I was tired when I wrote this -- there were several typos and I missed that HttpMessageContextWrapper actually did wrap something.)

I'm not sure what the need is for removing those, specifically the hasAccessTo methods. The latter ones are quite often requested by users. Even so much that over at OmniFaces we implemented those by parsing web.xml. This feature has been received with much enthusiasm by our users and is often used. We introduced it a while back so have experience with it for some time as has the general public and wider community.

It's quite important to have the hasAccessTo() methods for e.g. rendering menus in web applications (omit links users don't have access to), or to render links to protected pages in a different way (e.g. in red or with a lock symbol).

At a high level I don't disagree with the usefulness of the functionality, but I do have some concerns about the specifics:
  • The methods were added to the API only recently, at a point in the schedule where we needed to be constraining scope, not adding to it.
  • Depending on the container, the getAllDeclaredRoles() method could be difficult to implement, might perform poorly, or might provide inconsistent or unhelpful results. I'm thinking specifically of environments that do dynamic role mapping, such that generating the list would require evaluating every applicable role-mapping policy, and where the set of roles returned might be meaningless because the question "what roles?" was asked in an abstract or arbitrary context, rather than the context of an actual access control query, for a specific resource, in a specific context.
I also have mixed feelings about the usefulness of getting all of a user's roles, outside of a user- or role-management application. What the caller probably wants to know is, "what set of resources does this user have access to". An alternative way to get that answer is to query which of a set of resources the user can access. (Or query a set of roles -- a method isUserInRoles(Set<String> roles) might make sense.)
  • Some concerns with hasAccessToXXX():
    • The first concern has to do with the web-container-centric naming and signatures. As I understand it, SecurityContext is supposed to be more or less generic, and provide methods that are broadly applicable across all container types in which SecurityContext is available. The authenticate() method is already an exception to this, unfortunately, but if we continue down this path, we'll have a single SecurityContext for all containers, but it will be a fat, kitchen-sink API, with a proliferation of hasAccesstoEJBMethod(), hasAccessToRestResource(), hasAccessToJMSQueue(), etc., methods.
    • That brings me to the second concern -- there is no uniform representation of a resource that can be authorized. A string-based representation may make sense, but, if so, we need to define what the format of that string is, such that it can uniquely and unambiguously identify any Java EE resource, allows for flexible binding of authorization policies to resources, and efficient evaluation of policy at runtime, while remaining human readable and reasonably easy for developers to manipulate in code. Alternatively, we may be better served by an attribute-based model such that, for example, a URI and an HTTP method can be represented as separate (string) attributes as part of a "Resource" object. I'm not trying to argue for a specific representation, I'm just saying that we need one. Whatever that looks like, we should ultimately end up with, at most, one or two methods for authorization:
hasAccessToResource(Resource resource)
hasAccessToResources(Set<Resource> resources)
    • The question of *who* hasAccessToXXX() isn't really addressed. Implicitly, it's the Subject currently on the stack, and maybe that's all we need, but we should consider the question. What happens if there is no authenticated user in the context? What if we want to find out if User A has access to Resource X, where User A is not the currently authenticated user? (Asking that question should probably be allowed only for privileged users, but we should consider whether it's appropriate for the API.)
In short, while the functionality would be very useful, and I don't doubt that similar methods have worked well for, and been well-received by, your users, I think more thought needs to go into these APIs before they're standardized.

See:


Note that in OmniFaces we don't use JACC. I know there's a concern about JACC since it's not enabled by default in WLS (which as I think I mentioned is actually a spec violation), but for this feature JACC is not required. It's just trivial to implement using JACC, and I used that in the RI. But WLS and Liberty (the other server that doesn't have JACC enabled by default) can use proprietary code there, or even copy the code from OmniFaces if they want. 

Alternatively I could also provide the OmniFaces implementation in the RI for the hasAccessTo() methods.

 
    • As previously agreed, remove the "response only" signature for authenticate (authenticate(HttpServletResponse, AuthenticationParameters)).

Okay, I'll remove that one tomorrow too then.

Thanks!

 
    • Specify that ServletContext is supported only in EJB and Servlet containers, or possibly a *short* list of additional containers (likely the web-based containers) where it's known that the getCallerPrincipal() and isCallerInRole() methods can be implemented by layering on existing container APIs.
Okay, with the note that you probably mean SecurityContext here and not ServletContext.

Yes.

  • AuthenticationStatus -- weren't we going to align that with the JASPIC return codes, so that the correspondence would be completely clear even though we weren't re-using the actual type/constants?

Okay, I'm somewhat more in favour of the current names, but we indeed discussed that, so I'm going to make that change too then.

Thanks!

  • AuthenticationContext -- this is a bit of a kitchen sink. I guess we can live with that, but we at least need to provide javadoc for all of the getters/setters. As the author, you're best positioned to do so, I think.
Which class would that exactly be? There's no AuthenticationContext, but there is a HttpMessageContext. Did you meant that one?

Sorry, I meant javax/security/authentication/mechanism/http/AuthenticationParameters.java.

  • Would BaseMessageContext be a better name for MessageContextWrapper? The class doesn't actually wrap anything, it simply implements the MessageContext interface. (Both classes could use some javadoc.)

HttpMessageContextWrapper actually does wrapping, see e.g.

Yes, my mistake.

  • I'll probably add some fields to the LdapIdentityStoreDefinition annotation to allow developers to specify a particular search string (LDAP syntax) and map a particular LDAP attribute to the caller principal name.

Okay, thanks!

Thanks, Arjan. Appreciate all your help and your patience with my questions!

Regards,

Will

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803



Ivar Grimstad
 



On Thu, May 18, 2017 at 1:49 AM Arjan Tijms <arjan.tijms@...> wrote:
Hi,

On Wed, May 17, 2017 at 11:53 PM, Will Hopkins <will.hopkins@...> wrote:
A couple additional questions/concerns:
  • Should IdentityStores (and/or IdentityStoreHandler) be made available in containers other than the Servlet container? They're intended for use by HAMs, which are only present in the Servlet container, but they could in theory be useful for login to EJBs, etc.
I'm inclined to say Servlet-only, to keep things simple, and expand to other containers in a future JSR if there's demand for it.
There's nothing in the IdentityStore or the IdentityStoreHandler that makes it Servlet specific, or specific to any other container. The initial design was largely by Alex, with some revisions from many in the EG, and it being a protocol (message layer) independent artefact was the one thing everybody always agreed upon.

At the moment the only artefact that uses it is indeed from the Servlet container, since we only happen to have specified the HttpAuthenticationMechanism.

The SOAP authentication mechanism (based on EJB) could just as well use it, as would the existing container proprietary EJB remote login systems, and even the JAX-RS non-Servlet variant.

The MVC Specification is based on JAX-RS, so that would apply to us as well. Our RI uses Servlet, but another implementation could possibly be non-Servlet.
 

However early on the decision was made (mostly as stated by Alex again) to emphasise Servlet usage since SOAP as you know is not that much uses anymore for new projects, and neither is remote EJB (plus there isn't any kind of message layer SPI standardised for remote EJB), and my guess is that 99.99% of the JAX-RS implementations out there just use Servlet, so there's no really big need there either.

Long story short, the IdentityStore and IdentityStoreHandler are just CDI artefacts and there's no specific spec text needed I think to either demand them being available in other containers than Servlet or to restrict them to Servlet. Just saying it's an enabled CDI bean should be all that's needed.


 
  • Some issues with credentials:
  • The Password class, and several related credentials (UsernamePasswordCredential, BasicAuthenticationCredential) do not handle passwords securely -- there are constructors and methods that accept or return passwords as strings. Passwords should always be handled as char[], since String values can't be cleared from memory; they can hang around indefinitely. This would apply also to a base64-decoded basic auth string -- it should be decoded to a char[] to avoid having the password portion exposed in memory.
Well, we have been thinking about that, but I'm personally not 100% convinced you're really protecting against anything by doing that for server side apps. If an attacker would ever get that level of access so that the attacker is able to snoop into server memory, I think all bets are already off.

Plus, the HtppServletRequest.getParameter is a String already too, so what are you really protecting against?

The advise seems to originate from the old idea of the AS as a kind of OS managing a diverse set of apps that are potentially hostile to each other and to the AS.

But IMHO server apps that are potentially hostile to each other should be isolated at an entirely different level, e.g. at least via OS processes, but if they are truly hostile via virtual machines on a hypervisor.

When we discussed this earlier pretty much everyone agreed that char[] doesn't buy you much for server side apps. The char[] rule for passwords, while well known, doesn't seem to be universally accepted as a best practice at all for server side apps. (even Bill Shanon uses strings for passwords in GF, I noticed ;))

+1
 

That said, I'm not against them, but I don't see much value in them either.
 
  • The TokenCredential and CallerOnlyCredential are not referenced anywhere in the API or the RI. Given that, we should consider whether they're really needed.

True, there were several ideas around TokenCredential including using them as base for the JWT support that we hoped to be able to get in, but we never got to them (Rudy made a proof of concept though, but it wasn't included in the spec proper).
 
    • I have mixed feelings about the value of a CallerOnlyCredential (and whether it should be called CallerCredential, or CallerNameCredential). If we do keep it, we may want to provide a constructor/accessors that take CallerPrincipal as an argument.
The concept around CallerOnlyCredential is incredible useful and addresses a very real requirement in real world applications, namely to be able to re-authenticate (refresh authentication) within a request, e.g. in response to callers changing their name or performing an action that gives them extra groups and thus roles. For several RunAS situations these are quite important as well, since a system not rarely has to run code for a specific caller, but does not have the credential of that user.

Indeed, we currently don't reference it, so I'm a bit torn here. It's definitely something that custom authentication mechanisms could use, it's just that the ones we currently ship don't use it.

So I agree, if we keep it, let's provide a validate method for them that use them. If somehow this fails (e.g. because we run out of time), then let's remove it for now.
 
    • It's not clear what value TokenCredential is providing. It doesn't represent any real token type, and it makes an assumption that any token can be usefully (and safely) represented as a string. Wouldn't it be better to let those implementing support for a particular type of token write their own Credential type?
Maybe, I'm a bit torn here too, but since we have to make decisions and can't think forever about everything I'll remove this one then.

 
  • I'm not sure the getCaller() method should be on the Credential interface, as it may not be possible to directly obtain a caller for all Credential types. It's probably not needed for most credentials, and I suspect the vast majority -- UsernamePasswordCredential and related types aside -- will end up just returning "" (or null -- the correct behavior isn't clear). The Credential interface should only have operations that can be generically performed for any Credential type, and should make no assumptions about the form, structure, or content of any Credential.
There are only 3 calls to getCaller() in all of the code (test code excluded), and they're all invocations on UsernamePasswordCredential. Since IdentityStores explicitly choose the types of credentials they handle, they'll be prepared to deal with specific types, like UsernamePasswordCredential. It makes more sense to me to remove getCaller() from Credential, and add getUsername to UsernamePasswordCredential, rather than force all Credential types to implement a method they don't need and possibly can't support.

True, I vaguely remember some deeper ideas behind this, but this one originates from the very first days of the JSR, possibly the oldest code of all. I think you're right here, so let me just remove this one too then.

Should anyone really object against this later it's easy enough to put it back before the FR is filed.

Kind regards,
Arjan Tijms

 
Thoughts?

Will



On 05/17/2017 12:28 AM, Will Hopkins wrote:
Hi Arjan,

On 05/16/2017 06:35 PM, Arjan Tijms wrote:
Hi Will,

On Tue, May 16, 2017 at 11:26 PM, Will Hopkins <will.hopkins@...> wrote:

OK, yhat seems like a good approach, I'll make sure it's spec'd that way. I don't think I'll spec the idea about requiring the container to use an app-supplied IdentityStore -- you've convinced me about the FORM and BASIC annotations, so that's how the app can leverage an IdentityStore for FORM and BASIC.

How do you feel about specifing that HAMs are only required to be enabled when the app is deployed with <auth-method>AUTHMECH</auth-method>, though, to ensure that containers can continue support legacy apps using the existing code paths, and to ensure deterministic behavior when, for example, two implementations of FORM or BASIC are still present in the container. Containers could still choose to provide just one impl, and to implement all functionality via HAM, and apps could ensure they always got HAM by specifying the AUTHMECH, but legacy behavior got be safely preserved if a container vendor chose.

By itself this would be okay, but there are a few caveats.

First is that the CDI bean representing the HAM is initially enabled via a CDI extension. Despite the best efforts of both the CDI and Servlet EGs, nobody succeeded in finding a solution for the fact that currently CDI extensions are not guaranteed to be able to read web.xml.

So without <auth-method>AUTHMECH</auth-method> we should be prepared for the beans still being around, but just not actually used for authentication (when it comes time to install the JASPIC bridge, web.xml can be read).

Second is that specifically simpler applications are trending to a no-XML/no web.xml solution. Requiring a web.xml only to activate JSR 375 is a bit of a step back then. This caveat could be reduced though by introducing an annotation to set the auth mechanism and/or extending the javax.servlet.ServletContext with an option to set the auth mechanism programmatically.

Hmm. I see the problem(s). Let me give that some thought.

So the light-bulb went off a little while ago. I had been thinking in terms of bank-level security, general-purpose architecture for token issuance, integration with container session management,etc. -- then I remembered that my bank won't let me stay logged in longer than 20 minutes, but lots of other sites have little "Remember Me" checkboxes, and I understood the use case. It's really not a distinct architectural function, it's more like a feature added onto authentication, so the current implementation makes sense. Sorry for being so dense about it.

No worries, I'm happy it's clear now. I've had the light-bulb moment myself on multiple occasions too ;)

 
Let's not add anything else new.  ;)

Okay, sure I agree with that.

 
I guess I see them all as functionally distinct now. The first analogy that comes to mind is the hierarchy of storage for a CPU -- the CPU operates on data in registers; data is loaded into registers from the L2 cache, when there's an L2 cache miss the data is loaded from core memory, and data that's not in core memory is read from disk.

Indeed, that's a totally fitting analogy.

 
So, on to the next topic -- there are a couple other interfaces I think need to be moved, removed, and/or tweaked:
  • There was discussion on the list a while back about EmbeddedIdentityStore, and whether it represented a security risk and/or an anti-pattern. Although there was not complete agreement within the EG, there was support for the idea that it's not a good practice, and that we shouldn't encourage it by including it in the API. It has value for some development use cases, but those use cases could be easily satisfied by including it in the RI, and ultimately it's pretty trivial to write an actual IdentityStore implementation that has hard-coded user data and/or reads from a file, keystore, or other embedded repository.
I agree with the concern, and intend to remove it from the spec (i.e., API javadoc).

Well, as mentioned I don't 100% share the concern, although I do see the issue. The fact is that every other application server out there already provides the equivalent to the embedded identity store, though mostly using some sort of file (property or annotation mostly). But since there indeed was not complete agreement about this in the EG, we probably should give this some more thought.

But since we unfortunately don't have the time for that, I (somewhat reluctantly) agree to move this to the RI then. It's late at my place now, but I'll make the attempt tomorrow then.

Great, thanks.

 
  • For ServletContext, I intend to:
    • Remove the newly added methods (getAllDeclaredCallerRoles() and hasAccessToXXX())
(p.s. I think this is about SecurityContext and not ServletContext, but that's probably just a typo)

Yes, SecurityContext. (I was tired when I wrote this -- there were several typos and I missed that HttpMessageContextWrapper actually did wrap something.)

I'm not sure what the need is for removing those, specifically the hasAccessTo methods. The latter ones are quite often requested by users. Even so much that over at OmniFaces we implemented those by parsing web.xml. This feature has been received with much enthusiasm by our users and is often used. We introduced it a while back so have experience with it for some time as has the general public and wider community.

It's quite important to have the hasAccessTo() methods for e.g. rendering menus in web applications (omit links users don't have access to), or to render links to protected pages in a different way (e.g. in red or with a lock symbol).

At a high level I don't disagree with the usefulness of the functionality, but I do have some concerns about the specifics:
  • The methods were added to the API only recently, at a point in the schedule where we needed to be constraining scope, not adding to it.
  • Depending on the container, the getAllDeclaredRoles() method could be difficult to implement, might perform poorly, or might provide inconsistent or unhelpful results. I'm thinking specifically of environments that do dynamic role mapping, such that generating the list would require evaluating every applicable role-mapping policy, and where the set of roles returned might be meaningless because the question "what roles?" was asked in an abstract or arbitrary context, rather than the context of an actual access control query, for a specific resource, in a specific context.
I also have mixed feelings about the usefulness of getting all of a user's roles, outside of a user- or role-management application. What the caller probably wants to know is, "what set of resources does this user have access to". An alternative way to get that answer is to query which of a set of resources the user can access. (Or query a set of roles -- a method isUserInRoles(Set<String> roles) might make sense.)
  • Some concerns with hasAccessToXXX():
    • The first concern has to do with the web-container-centric naming and signatures. As I understand it, SecurityContext is supposed to be more or less generic, and provide methods that are broadly applicable across all container types in which SecurityContext is available. The authenticate() method is already an exception to this, unfortunately, but if we continue down this path, we'll have a single SecurityContext for all containers, but it will be a fat, kitchen-sink API, with a proliferation of hasAccesstoEJBMethod(), hasAccessToRestResource(), hasAccessToJMSQueue(), etc., methods.
    • That brings me to the second concern -- there is no uniform representation of a resource that can be authorized. A string-based representation may make sense, but, if so, we need to define what the format of that string is, such that it can uniquely and unambiguously identify any Java EE resource, allows for flexible binding of authorization policies to resources, and efficient evaluation of policy at runtime, while remaining human readable and reasonably easy for developers to manipulate in code. Alternatively, we may be better served by an attribute-based model such that, for example, a URI and an HTTP method can be represented as separate (string) attributes as part of a "Resource" object. I'm not trying to argue for a specific representation, I'm just saying that we need one. Whatever that looks like, we should ultimately end up with, at most, one or two methods for authorization:
hasAccessToResource(Resource resource)
hasAccessToResources(Set<Resource> resources)
    • The question of *who* hasAccessToXXX() isn't really addressed. Implicitly, it's the Subject currently on the stack, and maybe that's all we need, but we should consider the question. What happens if there is no authenticated user in the context? What if we want to find out if User A has access to Resource X, where User A is not the currently authenticated user? (Asking that question should probably be allowed only for privileged users, but we should consider whether it's appropriate for the API.)
In short, while the functionality would be very useful, and I don't doubt that similar methods have worked well for, and been well-received by, your users, I think more thought needs to go into these APIs before they're standardized.

See:


Note that in OmniFaces we don't use JACC. I know there's a concern about JACC since it's not enabled by default in WLS (which as I think I mentioned is actually a spec violation), but for this feature JACC is not required. It's just trivial to implement using JACC, and I used that in the RI. But WLS and Liberty (the other server that doesn't have JACC enabled by default) can use proprietary code there, or even copy the code from OmniFaces if they want. 

Alternatively I could also provide the OmniFaces implementation in the RI for the hasAccessTo() methods.

 
    • As previously agreed, remove the "response only" signature for authenticate (authenticate(HttpServletResponse, AuthenticationParameters)).

Okay, I'll remove that one tomorrow too then.

Thanks!

 
    • Specify that ServletContext is supported only in EJB and Servlet containers, or possibly a *short* list of additional containers (likely the web-based containers) where it's known that the getCallerPrincipal() and isCallerInRole() methods can be implemented by layering on existing container APIs.
Okay, with the note that you probably mean SecurityContext here and not ServletContext.

Yes.

  • AuthenticationStatus -- weren't we going to align that with the JASPIC return codes, so that the correspondence would be completely clear even though we weren't re-using the actual type/constants?

Okay, I'm somewhat more in favour of the current names, but we indeed discussed that, so I'm going to make that change too then.

Thanks!

  • AuthenticationContext -- this is a bit of a kitchen sink. I guess we can live with that, but we at least need to provide javadoc for all of the getters/setters. As the author, you're best positioned to do so, I think.
Which class would that exactly be? There's no AuthenticationContext, but there is a HttpMessageContext. Did you meant that one?

Sorry, I meant javax/security/authentication/mechanism/http/AuthenticationParameters.java.

  • Would BaseMessageContext be a better name for MessageContextWrapper? The class doesn't actually wrap anything, it simply implements the MessageContext interface. (Both classes could use some javadoc.)

HttpMessageContextWrapper actually does wrapping, see e.g.

Yes, my mistake.

  • I'll probably add some fields to the LdapIdentityStoreDefinition annotation to allow developers to specify a particular search string (LDAP syntax) and map a particular LDAP attribute to the caller principal name.

Okay, thanks!

Thanks, Arjan. Appreciate all your help and your patience with my questions!

Regards,

Will

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803


--

Java Champion, JCP EC/EG Member, JUG Leader


Will Hopkins
 

Arjan,

On 05/17/2017 06:49 PM, Arjan Tijms wrote:
Hi,

On Wed, May 17, 2017 at 11:53 PM, Will Hopkins <will.hopkins@...> wrote:
A couple additional questions/concerns:
  • Should IdentityStores (and/or IdentityStoreHandler) be made available in containers other than the Servlet container? They're intended for use by HAMs, which are only present in the Servlet container, but they could in theory be useful for login to EJBs, etc.
I'm inclined to say Servlet-only, to keep things simple, and expand to other containers in a future JSR if there's demand for it.
There's nothing in the IdentityStore or the IdentityStoreHandler that makes it Servlet specific, or specific to any other container. The initial design was largely by Alex, with some revisions from many in the EG, and it being a protocol (message layer) independent artefact was the one thing everybody always agreed upon.Arj

At the moment the only artefact that uses it is indeed from the Servlet container, since we only happen to have specified the HttpAuthenticationMechanism.

The SOAP authentication mechanism (based on EJB) could just as well use it, as would the existing container proprietary EJB remote login systems, and even the JAX-RS non-Servlet variant.

However early on the decision was made (mostly as stated by Alex again) to emphasise Servlet usage since SOAP as you know is not that much uses anymore for new projects, and neither is remote EJB (plus there isn't any kind of message layer SPI standardised for remote EJB), and my guess is that 99.99% of the JAX-RS implementations out there just use Servlet, so there's no really big need there either.

Long story short, the IdentityStore and IdentityStoreHandler are just CDI artefacts and there's no specific spec text needed I think to either demand them being available in other containers than Servlet or to restrict them to Servlet. Just saying it's an enabled CDI bean should be all that's needed.

I agree that IdentityStore is very container-independent, and there's no reason it couldn't run pretty much anywhere, but I think the spec needs to say where it *has* to run, so that vendors know what they have to implement, and the TCK can test if it works everywhere it's supposed to work. I guess you're right that if an IdentityStore were deployed in any bean archive it would therefore be available, and the only way to stop that is for the container to be very intrusive and prevent the bean from being registered with CDI.

So I guess we should say it's mandatory to work in Servlet container, but not elsewhere? Even that, I guess doesn't mean much, since it's CDI and not Servlet that's making the bean available. Maybe I'll just describe the intent, and not specify any requirements, since CDI takes care of all the required behavior.

  • Some issues with credentials:
  • The Password class, and several related credentials (UsernamePasswordCredential, BasicAuthenticationCredential) do not handle passwords securely -- there are constructors and methods that accept or return passwords as strings. Passwords should always be handled as char[], since String values can't be cleared from memory; they can hang around indefinitely. This would apply also to a base64-decoded basic auth string -- it should be decoded to a char[] to avoid having the password portion exposed in memory.
Well, we have been thinking about that, but I'm personally not 100% convinced you're really protecting against anything by doing that for server side apps. If an attacker would ever get that level of access so that the attacker is able to snoop into server memory, I think all bets are already off.

We can argue about how much difference it makes, but it's clearly at least a little safer, and it contributes to a "defense in depth" strategy.

Even if we think it doesn't really matter, though, there are a lot of companies that take this kind of stuff seriously -- running security audits and code inspections, using static analysis tools to look for vulnerabilities in their code, etc. Oracle has policies about safe password handling, and using char[] is considered a best practice.

This *is* a security JSR, and if we want it to be adopted in those kinds of environments we should probably try to follow the best practices.

Plus, the HtppServletRequest.getParameter is a String already too, so what are you really protecting against?

True, but for basic auth at least it's not completely in the clear. I don't know what can be done to encode passwords better in login forms -- can form fields be declared or serialized as char[]/byte[] and retrieved from the request that way?

Regardless, the fact that you can't plug one hole doesn't mean there's no point in plugging any hole ... we can at least handle passwords more safely once they're in our code.

The advise seems to originate from the old idea of the AS as a kind of OS managing a diverse set of apps that are potentially hostile to each other and to the AS.

But IMHO server apps that are potentially hostile to each other should be isolated at an entirely different level, e.g. at least via OS processes, but if they are truly hostile via virtual machines on a hypervisor.

When we discussed this earlier pretty much everyone agreed that char[] doesn't buy you much for server side apps. The char[] rule for passwords, while well known, doesn't seem to be universally accepted as a best practice at all for server side apps. (even Bill Shanon uses strings for passwords in GF, I noticed ;))

Bill's review comments actually mention this exact issue with respect to the password field on the LDAP annotation. He's been absorbed into the Oracle Global Security Borg's Hive Mind. Resistance was futile. ;)

I do think the value is debatable, but it's not zero, and making corporate security architects (and tooling) happy seems like a good idea.

That said, I'm not against them, but I don't see much value in them either.

I think it makes sense to do, for the reasons mentioned above. It does require touching a fair amount of code, but it shouldn't be difficult really.

  • The TokenCredential and CallerOnlyCredential are not referenced anywhere in the API or the RI. Given that, we should consider whether they're really needed.

True, there were several ideas around TokenCredential including using them as base for the JWT support that we hoped to be able to get in, but we never got to them (Rudy made a proof of concept though, but it wasn't included in the spec proper).
 
    • I have mixed feelings about the value of a CallerOnlyCredential (and whether it should be called CallerCredential, or CallerNameCredential). If we do keep it, we may want to provide a constructor/accessors that take CallerPrincipal as an argument.
The concept around CallerOnlyCredential is incredible useful and addresses a very real requirement in real world applications, namely to be able to re-authenticate (refresh authentication) within a request, e.g. in response to callers changing their name or performing an action that gives them extra groups and thus roles. For several RunAS situations these are quite important as well, since a system not rarely has to run code for a specific caller, but does not have the credential of that user.

Indeed, we currently don't reference it, so I'm a bit torn here. It's definitely something that custom authentication mechanisms could use, it's just that the ones we currently ship don't use it.

So I agree, if we keep it, let's provide a validate method for them that use them. If somehow this fails (e.g. because we run out of time), then let's remove it for now.

I agree there are use cases for this, although I think that we're really talking about impersonation, not authentication. I don't object strongly to keeping it, but I don't think we have time to develop useful (and secure) validator for it, so I'd probably prefer to drop it, and add it back later with a solid use case.

    • It's not clear what value TokenCredential is providing. It doesn't represent any real token type, and it makes an assumption that any token can be usefully (and safely) represented as a string. Wouldn't it be better to let those implementing support for a particular type of token write their own Credential type?
Maybe, I'm a bit torn here too, but since we have to make decisions and can't think forever about everything I'll remove this one then.

OK.

  • I'm not sure the getCaller() method should be on the Credential interface, as it may not be possible to directly obtain a caller for all Credential types. It's probably not needed for most credentials, and I suspect the vast majority -- UsernamePasswordCredential and related types aside -- will end up just returning "" (or null -- the correct behavior isn't clear). The Credential interface should only have operations that can be generically performed for any Credential type, and should make no assumptions about the form, structure, or content of any Credential.
There are only 3 calls to getCaller() in all of the code (test code excluded), and they're all invocations on UsernamePasswordCredential. Since IdentityStores explicitly choose the types of credentials they handle, they'll be prepared to deal with specific types, like UsernamePasswordCredential. It makes more sense to me to remove getCaller() from Credential, and add getUsername to UsernamePasswordCredential, rather than force all Credential types to implement a method they don't need and possibly can't support.

True, I vaguely remember some deeper ideas behind this, but this one originates from the very first days of the JSR, possibly the oldest code of all. I think you're right here, so let me just remove this one too then.

Should anyone really object against this later it's easy enough to put it back before the FR is filed.

Sounds good.

Thanks!

Will


Kind regards,
Arjan Tijms

 
Thoughts?

Will



On 05/17/2017 12:28 AM, Will Hopkins wrote:
Hi Arjan,

On 05/16/2017 06:35 PM, Arjan Tijms wrote:
Hi Will,

On Tue, May 16, 2017 at 11:26 PM, Will Hopkins <will.hopkins@...> wrote:

OK, yhat seems like a good approach, I'll make sure it's spec'd that way. I don't think I'll spec the idea about requiring the container to use an app-supplied IdentityStore -- you've convinced me about the FORM and BASIC annotations, so that's how the app can leverage an IdentityStore for FORM and BASIC.

How do you feel about specifing that HAMs are only required to be enabled when the app is deployed with <auth-method>AUTHMECH</auth-method>, though, to ensure that containers can continue support legacy apps using the existing code paths, and to ensure deterministic behavior when, for example, two implementations of FORM or BASIC are still present in the container. Containers could still choose to provide just one impl, and to implement all functionality via HAM, and apps could ensure they always got HAM by specifying the AUTHMECH, but legacy behavior got be safely preserved if a container vendor chose.

By itself this would be okay, but there are a few caveats.

First is that the CDI bean representing the HAM is initially enabled via a CDI extension. Despite the best efforts of both the CDI and Servlet EGs, nobody succeeded in finding a solution for the fact that currently CDI extensions are not guaranteed to be able to read web.xml.

So without <auth-method>AUTHMECH</auth-method> we should be prepared for the beans still being around, but just not actually used for authentication (when it comes time to install the JASPIC bridge, web.xml can be read).

Second is that specifically simpler applications are trending to a no-XML/no web.xml solution. Requiring a web.xml only to activate JSR 375 is a bit of a step back then. This caveat could be reduced though by introducing an annotation to set the auth mechanism and/or extending the javax.servlet.ServletContext with an option to set the auth mechanism programmatically.

Hmm. I see the problem(s). Let me give that some thought.

So the light-bulb went off a little while ago. I had been thinking in terms of bank-level security, general-purpose architecture for token issuance, integration with container session management,etc. -- then I remembered that my bank won't let me stay logged in longer than 20 minutes, but lots of other sites have little "Remember Me" checkboxes, and I understood the use case. It's really not a distinct architectural function, it's more like a feature added onto authentication, so the current implementation makes sense. Sorry for being so dense about it.

No worries, I'm happy it's clear now. I've had the light-bulb moment myself on multiple occasions too ;)

 
Let's not add anything else new.  ;)

Okay, sure I agree with that.

 
I guess I see them all as functionally distinct now. The first analogy that comes to mind is the hierarchy of storage for a CPU -- the CPU operates on data in registers; data is loaded into registers from the L2 cache, when there's an L2 cache miss the data is loaded from core memory, and data that's not in core memory is read from disk.

Indeed, that's a totally fitting analogy.

 
So, on to the next topic -- there are a couple other interfaces I think need to be moved, removed, and/or tweaked:
  • There was discussion on the list a while back about EmbeddedIdentityStore, and whether it represented a security risk and/or an anti-pattern. Although there was not complete agreement within the EG, there was support for the idea that it's not a good practice, and that we shouldn't encourage it by including it in the API. It has value for some development use cases, but those use cases could be easily satisfied by including it in the RI, and ultimately it's pretty trivial to write an actual IdentityStore implementation that has hard-coded user data and/or reads from a file, keystore, or other embedded repository.
I agree with the concern, and intend to remove it from the spec (i.e., API javadoc).

Well, as mentioned I don't 100% share the concern, although I do see the issue. The fact is that every other application server out there already provides the equivalent to the embedded identity store, though mostly using some sort of file (property or annotation mostly). But since there indeed was not complete agreement about this in the EG, we probably should give this some more thought.

But since we unfortunately don't have the time for that, I (somewhat reluctantly) agree to move this to the RI then. It's late at my place now, but I'll make the attempt tomorrow then.

Great, thanks.

 
  • For ServletContext, I intend to:
    • Remove the newly added methods (getAllDeclaredCallerRoles() and hasAccessToXXX())
(p.s. I think this is about SecurityContext and not ServletContext, but that's probably just a typo)

Yes, SecurityContext. (I was tired when I wrote this -- there were several typos and I missed that HttpMessageContextWrapper actually did wrap something.)

I'm not sure what the need is for removing those, specifically the hasAccessTo methods. The latter ones are quite often requested by users. Even so much that over at OmniFaces we implemented those by parsing web.xml. This feature has been received with much enthusiasm by our users and is often used. We introduced it a while back so have experience with it for some time as has the general public and wider community.

It's quite important to have the hasAccessTo() methods for e.g. rendering menus in web applications (omit links users don't have access to), or to render links to protected pages in a different way (e.g. in red or with a lock symbol).

At a high level I don't disagree with the usefulness of the functionality, but I do have some concerns about the specifics:
  • The methods were added to the API only recently, at a point in the schedule where we needed to be constraining scope, not adding to it.
  • Depending on the container, the getAllDeclaredRoles() method could be difficult to implement, might perform poorly, or might provide inconsistent or unhelpful results. I'm thinking specifically of environments that do dynamic role mapping, such that generating the list would require evaluating every applicable role-mapping policy, and where the set of roles returned might be meaningless because the question "what roles?" was asked in an abstract or arbitrary context, rather than the context of an actual access control query, for a specific resource, in a specific context.
I also have mixed feelings about the usefulness of getting all of a user's roles, outside of a user- or role-management application. What the caller probably wants to know is, "what set of resources does this user have access to". An alternative way to get that answer is to query which of a set of resources the user can access. (Or query a set of roles -- a method isUserInRoles(Set<String> roles) might make sense.)
  • Some concerns with hasAccessToXXX():
    • The first concern has to do with the web-container-centric naming and signatures. As I understand it, SecurityContext is supposed to be more or less generic, and provide methods that are broadly applicable across all container types in which SecurityContext is available. The authenticate() method is already an exception to this, unfortunately, but if we continue down this path, we'll have a single SecurityContext for all containers, but it will be a fat, kitchen-sink API, with a proliferation of hasAccesstoEJBMethod(), hasAccessToRestResource(), hasAccessToJMSQueue(), etc., methods.
    • That brings me to the second concern -- there is no uniform representation of a resource that can be authorized. A string-based representation may make sense, but, if so, we need to define what the format of that string is, such that it can uniquely and unambiguously identify any Java EE resource, allows for flexible binding of authorization policies to resources, and efficient evaluation of policy at runtime, while remaining human readable and reasonably easy for developers to manipulate in code. Alternatively, we may be better served by an attribute-based model such that, for example, a URI and an HTTP method can be represented as separate (string) attributes as part of a "Resource" object. I'm not trying to argue for a specific representation, I'm just saying that we need one. Whatever that looks like, we should ultimately end up with, at most, one or two methods for authorization:
hasAccessToResource(Resource resource)
hasAccessToResources(Set<Resource> resources)
    • The question of *who* hasAccessToXXX() isn't really addressed. Implicitly, it's the Subject currently on the stack, and maybe that's all we need, but we should consider the question. What happens if there is no authenticated user in the context? What if we want to find out if User A has access to Resource X, where User A is not the currently authenticated user? (Asking that question should probably be allowed only for privileged users, but we should consider whether it's appropriate for the API.)
In short, while the functionality would be very useful, and I don't doubt that similar methods have worked well for, and been well-received by, your users, I think more thought needs to go into these APIs before they're standardized.

See:


Note that in OmniFaces we don't use JACC. I know there's a concern about JACC since it's not enabled by default in WLS (which as I think I mentioned is actually a spec violation), but for this feature JACC is not required. It's just trivial to implement using JACC, and I used that in the RI. But WLS and Liberty (the other server that doesn't have JACC enabled by default) can use proprietary code there, or even copy the code from OmniFaces if they want. 

Alternatively I could also provide the OmniFaces implementation in the RI for the hasAccessTo() methods.

 
    • As previously agreed, remove the "response only" signature for authenticate (authenticate(HttpServletResponse, AuthenticationParameters)).

Okay, I'll remove that one tomorrow too then.

Thanks!

 
    • Specify that ServletContext is supported only in EJB and Servlet containers, or possibly a *short* list of additional containers (likely the web-based containers) where it's known that the getCallerPrincipal() and isCallerInRole() methods can be implemented by layering on existing container APIs.
Okay, with the note that you probably mean SecurityContext here and not ServletContext.

Yes.

  • AuthenticationStatus -- weren't we going to align that with the JASPIC return codes, so that the correspondence would be completely clear even though we weren't re-using the actual type/constants?

Okay, I'm somewhat more in favour of the current names, but we indeed discussed that, so I'm going to make that change too then.

Thanks!

  • AuthenticationContext -- this is a bit of a kitchen sink. I guess we can live with that, but we at least need to provide javadoc for all of the getters/setters. As the author, you're best positioned to do so, I think.
Which class would that exactly be? There's no AuthenticationContext, but there is a HttpMessageContext. Did you meant that one?

Sorry, I meant javax/security/authentication/mechanism/http/AuthenticationParameters.java.

  • Would BaseMessageContext be a better name for MessageContextWrapper? The class doesn't actually wrap anything, it simply implements the MessageContext interface. (Both classes could use some javadoc.)

HttpMessageContextWrapper actually does wrapping, see e.g.

Yes, my mistake.

  • I'll probably add some fields to the LdapIdentityStoreDefinition annotation to allow developers to specify a particular search string (LDAP syntax) and map a particular LDAP attribute to the caller principal name.

Okay, thanks!

Thanks, Arjan. Appreciate all your help and your patience with my questions!

Regards,

Will

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803


-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803


Arjan Tijms
 



On Thu, May 18, 2017 at 6:12 AM, Will Hopkins <will.hopkins@...> wrote:
Even that, I guess doesn't mean much, since it's CDI and not Servlet that's making the bean available. Maybe I'll just describe the intent, and not specify any requirements, since CDI takes care of all the required behavior.

Yes, that would be best. It's available everywhere were CDI is available, and that's basically it. The intent is also that an IdentityStore uses resources such as JDBC data sources and (application) CDI or EJB beans, but even that is not strictly required.

 
We can argue about how much difference it makes, but it's clearly at least a little safer, and it contributes to a "defense in depth" strategy.

Even if we think it doesn't really matter, though, there are a lot of companies that take this kind of stuff seriously -- running security audits and code inspections, using static analysis tools to look for vulnerabilities in their code, etc. Oracle has policies about safe password handling, and using char[] is considered a best practice.

This *is* a security JSR, and if we want it to be adopted in those kinds of environments we should probably try to follow the best practices.

The question is, and I'm highly skeptical about it, whether char[] is indeed a best practice, specifically for server side apps. 

Sometimes things are taken as a best practice, but hardly anyone can explain why that is. Like a service always having to have an interface, or everything always having to throw checked exceptions. As a younger engineer I took the supposedly best practice of "everything throws checked exceptions" into question on the basis of there being no evidence it really did improve robustness of code. In some project we had to spend huge amount of times adding those checked exceptions to anything and everything since it was "a best practice".

Nowadays people kinda understand that making everything throw checked exceptions by default doesn't help one bit and only makes the code harder to read and more obscure to work with.


 


Plus, the HtppServletRequest.getParameter is a String already too, so what are you really protecting against?

True, but for basic auth at least it's not completely in the clear. I don't know what can be done to encode passwords better in login forms -- can form fields be declared or serialized as char[]/byte[] and retrieved from the request that way?

Nope, not really using the normal Servlet APIs. With most frameworks (Java EE or otherwise) they have been converted to various strings at least a dozen times over before it even reaches your application code.

If an extreme "defence in depth" strategy would be required I'd much rather go for encrypted memory and special decrypting CPUs, but those are rarely if ever required for server apps.

 
Bill's review comments actually mention this exact issue with respect to the password field on the LDAP annotation. He's been absorbed into the Oracle Global Security Borg's Hive Mind. Resistance was futile. ;)

Wow :) Then I can't resist any longer, can I? ;)

But do take a look at some of the existing APIs in Java (EE):



String password in common annotations:



String password in JMS:




String password in JDBC:



String password in the Tomcat security handling:



String password in Jetty security handling:



String password in Undertow (JBoss/Wildfly) security handling:



String password in Resin security handling:



String password in Liberty security handling:



Almost everywhere the password is a String at some point, and at quite some places in an unavoidable key API. Granted, in a number of locations shown above they do eventually get converted to a char[],  but in some cases then later on back to a string again.

 
I think it makes sense to do, for the reasons mentioned above. It does require touching a fair amount of code, but it shouldn't be difficult really.

I would not really be against it as I mentioned earlier, I just don't think it will add much if any actual security. But if you want to change this, then sure.


 
I agree there are use cases for this, although I think that we're really talking about impersonation, not authentication. I don't object strongly to keeping it, but I don't think we have time to develop useful (and secure) validator for it, so I'd probably prefer to drop it, and add it back later with a solid use case.

RunAs is indeed impersonation. It's technically very close to authentication. Well, not authentication itself but the result of it; setting the identity of a caller including the groups.

If you really think this should be removed at this time to be reconsidered for a later point indeed, I can remove it. Let me know how strongly you feel about this one.

The TokenCredential and getCaller method have been removed already.

Kind regards,
Arjan Tijms




 

Maybe, I'm a bit torn here too, but since we have to make decisions and can't think forever about everything I'll remove this one then.

OK.

  • I'm not sure the getCaller() method should be on the Credential interface, as it may not be possible to directly obtain a caller for all Credential types. It's probably not needed for most credentials, and I suspect the vast majority -- UsernamePasswordCredential and related types aside -- will end up just returning "" (or null -- the correct behavior isn't clear). The Credential interface should only have operations that can be generically performed for any Credential type, and should make no assumptions about the form, structure, or content of any Credential.
There are only 3 calls to getCaller() in all of the code (test code excluded), and they're all invocations on UsernamePasswordCredential. Since IdentityStores explicitly choose the types of credentials they handle, they'll be prepared to deal with specific types, like UsernamePasswordCredential. It makes more sense to me to remove getCaller() from Credential, and add getUsername to UsernamePasswordCredential, rather than force all Credential types to implement a method they don't need and possibly can't support.

True, I vaguely remember some deeper ideas behind this, but this one originates from the very first days of the JSR, possibly the oldest code of all. I think you're right here, so let me just remove this one too then.

Should anyone really object against this later it's easy enough to put it back before the FR is filed.

Sounds good.

Thanks!

Will



Kind regards,
Arjan Tijms

 
Thoughts?

Will



On 05/17/2017 12:28 AM, Will Hopkins wrote:
Hi Arjan,

On 05/16/2017 06:35 PM, Arjan Tijms wrote:
Hi Will,

On Tue, May 16, 2017 at 11:26 PM, Will Hopkins <will.hopkins@...> wrote:

OK, yhat seems like a good approach, I'll make sure it's spec'd that way. I don't think I'll spec the idea about requiring the container to use an app-supplied IdentityStore -- you've convinced me about the FORM and BASIC annotations, so that's how the app can leverage an IdentityStore for FORM and BASIC.

How do you feel about specifing that HAMs are only required to be enabled when the app is deployed with <auth-method>AUTHMECH</auth-method>, though, to ensure that containers can continue support legacy apps using the existing code paths, and to ensure deterministic behavior when, for example, two implementations of FORM or BASIC are still present in the container. Containers could still choose to provide just one impl, and to implement all functionality via HAM, and apps could ensure they always got HAM by specifying the AUTHMECH, but legacy behavior got be safely preserved if a container vendor chose.

By itself this would be okay, but there are a few caveats.

First is that the CDI bean representing the HAM is initially enabled via a CDI extension. Despite the best efforts of both the CDI and Servlet EGs, nobody succeeded in finding a solution for the fact that currently CDI extensions are not guaranteed to be able to read web.xml.

So without <auth-method>AUTHMECH</auth-method> we should be prepared for the beans still being around, but just not actually used for authentication (when it comes time to install the JASPIC bridge, web.xml can be read).

Second is that specifically simpler applications are trending to a no-XML/no web.xml solution. Requiring a web.xml only to activate JSR 375 is a bit of a step back then. This caveat could be reduced though by introducing an annotation to set the auth mechanism and/or extending the javax.servlet.ServletContext with an option to set the auth mechanism programmatically.

Hmm. I see the problem(s). Let me give that some thought.

So the light-bulb went off a little while ago. I had been thinking in terms of bank-level security, general-purpose architecture for token issuance, integration with container session management,etc. -- then I remembered that my bank won't let me stay logged in longer than 20 minutes, but lots of other sites have little "Remember Me" checkboxes, and I understood the use case. It's really not a distinct architectural function, it's more like a feature added onto authentication, so the current implementation makes sense. Sorry for being so dense about it.

No worries, I'm happy it's clear now. I've had the light-bulb moment myself on multiple occasions too ;)

 
Let's not add anything else new.  ;)

Okay, sure I agree with that.

 
I guess I see them all as functionally distinct now. The first analogy that comes to mind is the hierarchy of storage for a CPU -- the CPU operates on data in registers; data is loaded into registers from the L2 cache, when there's an L2 cache miss the data is loaded from core memory, and data that's not in core memory is read from disk.

Indeed, that's a totally fitting analogy.

 
So, on to the next topic -- there are a couple other interfaces I think need to be moved, removed, and/or tweaked:
  • There was discussion on the list a while back about EmbeddedIdentityStore, and whether it represented a security risk and/or an anti-pattern. Although there was not complete agreement within the EG, there was support for the idea that it's not a good practice, and that we shouldn't encourage it by including it in the API. It has value for some development use cases, but those use cases could be easily satisfied by including it in the RI, and ultimately it's pretty trivial to write an actual IdentityStore implementation that has hard-coded user data and/or reads from a file, keystore, or other embedded repository.
I agree with the concern, and intend to remove it from the spec (i.e., API javadoc).

Well, as mentioned I don't 100% share the concern, although I do see the issue. The fact is that every other application server out there already provides the equivalent to the embedded identity store, though mostly using some sort of file (property or annotation mostly). But since there indeed was not complete agreement about this in the EG, we probably should give this some more thought.

But since we unfortunately don't have the time for that, I (somewhat reluctantly) agree to move this to the RI then. It's late at my place now, but I'll make the attempt tomorrow then.

Great, thanks.

 
  • For ServletContext, I intend to:
    • Remove the newly added methods (getAllDeclaredCallerRoles() and hasAccessToXXX())
(p.s. I think this is about SecurityContext and not ServletContext, but that's probably just a typo)

Yes, SecurityContext. (I was tired when I wrote this -- there were several typos and I missed that HttpMessageContextWrapper actually did wrap something.)

I'm not sure what the need is for removing those, specifically the hasAccessTo methods. The latter ones are quite often requested by users. Even so much that over at OmniFaces we implemented those by parsing web.xml. This feature has been received with much enthusiasm by our users and is often used. We introduced it a while back so have experience with it for some time as has the general public and wider community.

It's quite important to have the hasAccessTo() methods for e.g. rendering menus in web applications (omit links users don't have access to), or to render links to protected pages in a different way (e.g. in red or with a lock symbol).

At a high level I don't disagree with the usefulness of the functionality, but I do have some concerns about the specifics:
  • The methods were added to the API only recently, at a point in the schedule where we needed to be constraining scope, not adding to it.
  • Depending on the container, the getAllDeclaredRoles() method could be difficult to implement, might perform poorly, or might provide inconsistent or unhelpful results. I'm thinking specifically of environments that do dynamic role mapping, such that generating the list would require evaluating every applicable role-mapping policy, and where the set of roles returned might be meaningless because the question "what roles?" was asked in an abstract or arbitrary context, rather than the context of an actual access control query, for a specific resource, in a specific context.
I also have mixed feelings about the usefulness of getting all of a user's roles, outside of a user- or role-management application. What the caller probably wants to know is, "what set of resources does this user have access to". An alternative way to get that answer is to query which of a set of resources the user can access. (Or query a set of roles -- a method isUserInRoles(Set<String> roles) might make sense.)
  • Some concerns with hasAccessToXXX():
    • The first concern has to do with the web-container-centric naming and signatures. As I understand it, SecurityContext is supposed to be more or less generic, and provide methods that are broadly applicable across all container types in which SecurityContext is available. The authenticate() method is already an exception to this, unfortunately, but if we continue down this path, we'll have a single SecurityContext for all containers, but it will be a fat, kitchen-sink API, with a proliferation of hasAccesstoEJBMethod(), hasAccessToRestResource(), hasAccessToJMSQueue(), etc., methods.
    • That brings me to the second concern -- there is no uniform representation of a resource that can be authorized. A string-based representation may make sense, but, if so, we need to define what the format of that string is, such that it can uniquely and unambiguously identify any Java EE resource, allows for flexible binding of authorization policies to resources, and efficient evaluation of policy at runtime, while remaining human readable and reasonably easy for developers to manipulate in code. Alternatively, we may be better served by an attribute-based model such that, for example, a URI and an HTTP method can be represented as separate (string) attributes as part of a "Resource" object. I'm not trying to argue for a specific representation, I'm just saying that we need one. Whatever that looks like, we should ultimately end up with, at most, one or two methods for authorization:
hasAccessToResource(Resource resource)
hasAccessToResources(Set<Resource> resources)
    • The question of *who* hasAccessToXXX() isn't really addressed. Implicitly, it's the Subject currently on the stack, and maybe that's all we need, but we should consider the question. What happens if there is no authenticated user in the context? What if we want to find out if User A has access to Resource X, where User A is not the currently authenticated user? (Asking that question should probably be allowed only for privileged users, but we should consider whether it's appropriate for the API.)
In short, while the functionality would be very useful, and I don't doubt that similar methods have worked well for, and been well-received by, your users, I think more thought needs to go into these APIs before they're standardized.

See:


Note that in OmniFaces we don't use JACC. I know there's a concern about JACC since it's not enabled by default in WLS (which as I think I mentioned is actually a spec violation), but for this feature JACC is not required. It's just trivial to implement using JACC, and I used that in the RI. But WLS and Liberty (the other server that doesn't have JACC enabled by default) can use proprietary code there, or even copy the code from OmniFaces if they want. 

Alternatively I could also provide the OmniFaces implementation in the RI for the hasAccessTo() methods.

 
    • As previously agreed, remove the "response only" signature for authenticate (authenticate(HttpServletResponse, AuthenticationParameters)).

Okay, I'll remove that one tomorrow too then.

Thanks!

 
    • Specify that ServletContext is supported only in EJB and Servlet containers, or possibly a *short* list of additional containers (likely the web-based containers) where it's known that the getCallerPrincipal() and isCallerInRole() methods can be implemented by layering on existing container APIs.
Okay, with the note that you probably mean SecurityContext here and not ServletContext.

Yes.

  • AuthenticationStatus -- weren't we going to align that with the JASPIC return codes, so that the correspondence would be completely clear even though we weren't re-using the actual type/constants?

Okay, I'm somewhat more in favour of the current names, but we indeed discussed that, so I'm going to make that change too then.

Thanks!

  • AuthenticationContext -- this is a bit of a kitchen sink. I guess we can live with that, but we at least need to provide javadoc for all of the getters/setters. As the author, you're best positioned to do so, I think.
Which class would that exactly be? There's no AuthenticationContext, but there is a HttpMessageContext. Did you meant that one?

Sorry, I meant javax/security/authentication/mechanism/http/AuthenticationParameters.java.

  • Would BaseMessageContext be a better name for MessageContextWrapper? The class doesn't actually wrap anything, it simply implements the MessageContext interface. (Both classes could use some javadoc.)

HttpMessageContextWrapper actually does wrapping, see e.g.

Yes, my mistake.

  • I'll probably add some fields to the LdapIdentityStoreDefinition annotation to allow developers to specify a particular search string (LDAP syntax) and map a particular LDAP attribute to the caller principal name.

Okay, thanks!

Thanks, Arjan. Appreciate all your help and your patience with my questions!

Regards,

Will

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803


-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803



Arjan Tijms
 

Hi,

On Wed, May 17, 2017 at 6:28 AM, Will Hopkins <will.hopkins@...> wrote:
 Requiring a web.xml only to activate JSR 375 is a bit of a step back then. This caveat could be reduced though by introducing an annotation to set the auth mechanism and/or extending the javax.servlet.ServletContext with an option to set the auth mechanism programmatically.

Hmm. I see the problem(s). Let me give that some thought.

ok
 
At a high level I don't disagree with the usefulness of the functionality, but I do have some concerns about the specifics:
  • The methods were added to the API only recently, at a point in the schedule where we needed to be constraining scope, not adding to it.
  • Depending on the container, the getAllDeclaredRoles() method could be difficult to implement, might perform poorly, or might provide inconsistent or unhelpful results. I'm thinking specifically of environments that do dynamic role mapping, such that generating the list would require evaluating every applicable role-mapping policy, and where the set of roles returned might be meaningless because the question "what roles?" was asked in an abstract or arbitrary context, rather than the context of an actual access control query, for a specific resource, in a specific context.
I agree that the list of roles should be considered in a context, but that's true for simple isCallerInRole queries as well. They too only hold for that moment in that context.

I do think that by defining them as *declared* roles, it should not perform particularly poorly. Maybe it could be spelled out more thoroughly in the spec, but it now essentially asks for the list of all globally declared application roles (via the Java EE deployment descriptors and annotation variants), which is almost always a small and finite list.

So suppose we have in web.xml:

<security-role>
    <role-name>employee</role-name>
    <role-name>manager</role-name>
    <role-name>financial</role-name>
</security-role>

Then the initial list would be [employee, manager, financial].

The runtime would then iterate over that list, essentially doing 

isCallerInRole("employee")
isCallerInRole("manager")
isCallerInRole("financial")

Given a caller in role employee and financial, this would then return [employee, financial].

In the spec text it currently says that too, but perhaps with an example included it's more clear. See this:

"The getAllDeclaredCallerRoles() method returns all static roles, meaning the roles that are declared upfront in the web application and are discovered during startup. For each role in this list the implementation MUST have ensured that the caller is currently indeed in that role, where "currently" refers to the context from where the call is made and the point in time that the call is made."

I'm well aware that the dynamic nature of role discovering and mapping can be troublesome, resource intensive and in some cases even impossible, but unless I'm missing something I think the above should be absolutely doable for all containers.
 
I also have mixed feelings about the usefulness of getting all of a user's roles, outside of a user- or role-management application.
Well, it was requested a couple of times, but with all the java.net migrations I can't easily find back where it came from. I do remember this being proposed quite early on. In a way it's also a compromise for Alex' initial identity store proposal which had more options to query the store.

  • Some concerns with hasAccessToXXX():
    • The first concern has to do with the web-container-centric naming and signatures. As I understand it, SecurityContext is supposed to be more or less generic, and provide methods that are broadly applicable across all container types in which SecurityContext is available. The authenticate() method is already an exception to this, unfortunately, but if we continue down this path, we'll have a single SecurityContext for all containers, but it will be a fat, kitchen-sink API, with a proliferation of hasAccesstoEJBMethod(), hasAccessToRestResource(), hasAccessToJMSQueue(), etc., methods.
I understand the concern, but the amount of things to query for access has stayed quite limited over the years and I don't foresee it really increasing much or at all.

"accessToRestResource" for example is covered by the web resource (I know of no effort to introduce rest resource specific constraints in either JAX-RS or Java EE). 

Contrary to hasAccessToWebResource, one can programmatically just try the hypothetical hasAccesstoEJBMethod() with a try/catch.

Usage about hasAccessToWebResource is certainly not limited for callers in the web container. Business logic in e.g. the EJB container not rarely needs to know this too, for instance for generating reports or emails containing links.

An alternative, which we may want to add anyway, now or at a later stage, is a "boolean hasAccessTo(java.security.Permission permission)" method, but I'm sure hasAccessToWebResource is a special case enough for it to warrant having its own method.


 
    • That brings me to the second concern -- there is no uniform representation of a resource that can be authorized. A string-based representation may make sense, but, if so, we need to define what the format of that string is, such that it can uniquely and unambiguously identify any Java EE resource, allows for flexible binding of authorization policies to resources, and efficient evaluation of policy at runtime, while remaining human readable and reasonably easy for developers to manipulate in code.

I agree with need for that representation, but the spec already addresses that:

"The hasAccessToWebResource(String resource) method determines if the caller has access to the provided "web resource" using the GET HTTP method, such as specified by section 13.8 of the Servlet specification, and the JACC specification, specifically the WebResourcePermission type."

and

       /**
* Checks whether the caller has access to the provided "web resource" using the GET HTTP method, 
* such as specified by section 13.8 of the Servlet specification, and the JACC specification, 
* specifically the {@link WebResourcePermission} type.
* <p>
* A caller has access if the web resource is either not protected (constrained), or when it is protected by a role
* and the caller is in that role.
* @param resource the name of the web resource to test access for. This is a <code>URLPatternSpec</code> that 
* identifies the application specific web resources to which the permission pertains. For a full specification of this
* pattern see {@link WebResourcePermission#WebResourcePermission(String, String)}.
         * 
* @return <code>true</code> if the caller has access to the web resource, <code>false</code> otherwise. 
*/
boolean hasAccessToWebResource(String resource);


The URLPatternSpec is very well defined. In it's simplest form it's a single exact matching URLPattern such as can be used in the @WebServlet annotation.

The spec gives an example of this:

"
@WebServlet("/protectedServlet")
@ServletSecurity(@HttpConstraint(rolesAllowed = "foo"))
public class ProtectedServlet extends HttpServlet { ... }

And the following call to hasAccessToWebResource:

securityContext.hasAccessToWebResource("/protectedServlet")

The above "hasAccessToWebResource" call would return true if and only if the caller is in role "foo".
"

The full spec text which we reference is;

     * The syntax of a URLPatternSpec
     * is as follows:
     *
     * <P><Pre>
     *
     *          URLPatternList ::= URLPattern | URLPatternList colon URLPattern
     *
     *          URLPatternSpec ::= null | URLPattern | URLPattern colon URLPatternList
     *
     * </Pre><P>
     *
     * A null URLPatternSpec is translated to the default URLPattern, "/", by
     * the permission constructor. The empty string is an exact URLPattern, and
     * may occur anywhere in a URLPatternSpec that an exact URLPattern may occur.
     * The first URLPattern in a URLPatternSpec may be any of the pattern 
     * types, exact, path-prefix, extension, or default as defined in the
     * <i>Java Servlet Specification)</i>. When a URLPatternSpec includes
     * a URLPatternList, the patterns of the URLPatternList identify the
     * resources to which the permission does NOT apply and depend on the
     * pattern type and value of the first pattern as follows:  <p>
     *
     * <ul>
     * <li> No pattern may exist in the URLPatternList that matches the
     *      first pattern.
     * <li> If the first pattern is a path-prefix
     *      pattern, only exact patterns matched by the first pattern
     *      and path-prefix patterns matched by, but different from,
     *      the first pattern may occur in the URLPatternList.
     * <li> If the first pattern is an extension
     *      pattern, only exact patterns that are matched by the first 
     *      pattern and path-prefix patterns may occur in the URLPatternList.
     * <li> If the first pattern is the default pattern, "/", any pattern
     *      except the default pattern may occur in the URLPatternList.
     * <li> If the first pattern is an exact pattern a URLPatternList must not
     *      be present in the URLPatternSpec.
     * </ul>
     * <P>


 
    • The question of *who* hasAccessToXXX() isn't really addressed. Implicitly, it's the Subject currently on the stack, and maybe that's all we need, but we should consider the question. What happens if there is no authenticated user in the context?
The same as when isCallerInRole is called and there's no authenticated caller; a false is returned. We could clarify this a little more if you want?

 
    • What if we want to find out if User A has access to Resource X, where User A is not the currently authenticated user? (Asking that question should probably be allowed only for privileged users, but we should consider whether it's appropriate for the API.)

Alex had an initial proposal that incorporated queries that were quite like that, as well as functionality to create/update/delete users from compatible identity stores in a portable way. I think a privileged system level artefact that could be used to query for any caller would certainly make sense. However, for this release the EG and then spec lead Alex decided not to go forward with that yet, but to consider it for a next release. 

The SecurityContext, just like e.g. the FacesContext in JSF, and the SessionContext in EJB is aimed at the current caller (current subject at the stack aka the established authenticated identity). 

 
Thanks, Arjan. Appreciate all your help and your patience with my questions!

Happy to help out and once again thanks for your thorough review.

Kind regards,
Arjan Tijms




 

Regards,

Will

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803



Will Hopkins
 



On 05/18/2017 05:22 AM, Arjan Tijms wrote:

On Thu, May 18, 2017 at 6:12 AM, Will Hopkins <will.hopkins@...> wrote:
Even that, I guess doesn't mean much, since it's CDI and not Servlet that's making the bean available. Maybe I'll just describe the intent, and not specify any requirements, since CDI takes care of all the required behavior.

Yes, that would be best. It's available everywhere were CDI is available, and that's basically it. The intent is also that an IdentityStore uses resources such as JDBC data sources and (application) CDI or EJB beans, but even that is not strictly required.
OK.

We can argue about how much difference it makes, but it's clearly at least a little safer, and it contributes to a "defense in depth" strategy.

Even if we think it doesn't really matter, though, there are a lot of companies that take this kind of stuff seriously -- running security audits and code inspections, using static analysis tools to look for vulnerabilities in their code, etc. Oracle has policies about safe password handling, and using char[] is considered a best practice.

This *is* a security JSR, and if we want it to be adopted in those kinds of environments we should probably try to follow the best practices.

The question is, and I'm highly skeptical about it, whether char[] is indeed a best practice, specifically for server side apps. 

Sometimes things are taken as a best practice, but hardly anyone can explain why that is. Like a service always having to have an interface, or everything always having to throw checked exceptions. As a younger engineer I took the supposedly best practice of "everything throws checked exceptions" into question on the basis of there being no evidence it really did improve robustness of code. In some project we had to spend huge amount of times adding those checked exceptions to anything and everything since it was "a best practice".

I think there's a very clear reason -- to prevent passwords from being visible in memory after they've been "cleared". A char[] can be zero'd out, a String can't. One can debate how much of a risk having passwords in memory really represents in any given server environment, but it's not zero.

So far, I've heard a lot of arguments to the effect that it's not necessary. Those can be debated pro and con, but I haven't heard any convincing argument to say that using char[] is a bad idea. Necessary or not, it can't hurt.

Plus, the HtppServletRequest.getParameter is a String already too, so what are you really protecting against?

True, but for basic auth at least it's not completely in the clear. I don't know what can be done to encode passwords better in login forms -- can form fields be declared or serialized as char[]/byte[] and retrieved from the request that way?

Nope, not really using the normal Servlet APIs. With most frameworks (Java EE or otherwise) they have been converted to various strings at least a dozen times over before it even reaches your application code.

If an extreme "defence in depth" strategy would be required I'd much rather go for encrypted memory and special decrypting CPUs, but those are rarely if ever required for server apps.

I don't mean that we're responsible for making sure everything is completely secure end-to-end. But we should make our little piece as robust as possible.

Bill's review comments actually mention this exact issue with respect to the password field on the LDAP annotation. He's been absorbed into the Oracle Global Security Borg's Hive Mind. Resistance was futile. ;)

Wow :) Then I can't resist any longer, can I? ;)

But do take a look at some of the existing APIs in Java (EE):
[...]
Almost everywhere the password is a String at some point, and at quite some places in an unavoidable key API. Granted, in a number of locations shown above they do eventually get converted to a char[],  but in some cases then later on back to a string again.

I realize there are a lot of places where passwords are handled as strings, for historical reasons or otherwise, but again, that doesn't seem like a convincing argument for unsafe password handling in a new API, particularly one defined by a security JSR.

I think it makes sense to do, for the reasons mentioned above. It does require touching a fair amount of code, but it shouldn't be difficult really.

I would not really be against it as I mentioned earlier, I just don't think it will add much if any actual security. But if you want to change this, then sure.

I think we should. As you point out, it may provide minimal additional security, but it seems like the right thing to do.
 
I agree there are use cases for this, although I think that we're really talking about impersonation, not authentication. I don't object strongly to keeping it, but I don't think we have time to develop useful (and secure) validator for it, so I'd probably prefer to drop it, and add it back later with a solid use case.

RunAs is indeed impersonation. It's technically very close to authentication. Well, not authentication itself but the result of it; setting the identity of a caller including the groups.

If you really think this should be removed at this time to be reconsidered for a later point indeed, I can remove it. Let me know how strongly you feel about this one.

I think it would be better to wait until we can think the impersonation case through fully, but I don't feel strongly about it. Up to you whether to leave it or take it out.


The TokenCredential and getCaller method have been removed already.

Thanks.

Will

-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803


Will Hopkins
 



On 05/18/2017 10:18 AM, Arjan Tijms wrote:
Hi,

On Wed, May 17, 2017 at 6:28 AM, Will Hopkins <will.hopkins@...> wrote:
 Requiring a web.xml only to activate JSR 375 is a bit of a step back then. This caveat could be reduced though by introducing an annotation to set the auth mechanism and/or extending the javax.servlet.ServletContext with an option to set the auth mechanism programmatically.

Hmm. I see the problem(s). Let me give that some thought.

ok
 
At a high level I don't disagree with the usefulness of the functionality, but I do have some concerns about the specifics:
  • The methods were added to the API only recently, at a point in the schedule where we needed to be constraining scope, not adding to it.
  • Depending on the container, the getAllDeclaredRoles() method could be difficult to implement, might perform poorly, or might provide inconsistent or unhelpful results. I'm thinking specifically of environments that do dynamic role mapping, such that generating the list would require evaluating every applicable role-mapping policy, and where the set of roles returned might be meaningless because the question "what roles?" was asked in an abstract or arbitrary context, rather than the context of an actual access control query, for a specific resource, in a specific context.
I agree that the list of roles should be considered in a context, but that's true for simple isCallerInRole queries as well. They too only hold for that moment in that context.

I do think that by defining them as *declared* roles, it should not perform particularly poorly. Maybe it could be spelled out more thoroughly in the spec, but it now essentially asks for the list of all globally declared application roles (via the Java EE deployment descriptors and annotation variants), which is almost always a small and finite list.
[...]
I'm well aware that the dynamic nature of role discovering and mapping can be troublesome, resource intensive and in some cases even impossible, but unless I'm missing something I think the above should be absolutely doable for all containers.

You're right, defining the set of roles as Declared Roles does make the task easier. I think it's a little bit of trouble because it does mean the container now has to remember all the declared roles when parsing the deployment descriptor (and possibly annotations), but aside from that the logic isn't difficult.

I really am reluctant to add "new" stuff at this point, though -- it's not just work to spec, it's work for testing, and for RI, and this seems more like a convenience method than a necessity. The author of the code presumably already knows the set of declared roles, since he or she will have written the web.xml as well as the code. If they really need to query the declared roles the caller has, they can write the little loop iterating over the roles themselves. Not as convenient, agreed, but not difficult either.

  • Some concerns with hasAccessToXXX():
    • The first concern has to do with the web-container-centric naming and signatures. As I understand it, SecurityContext is supposed to be more or less generic, and provide methods that are broadly applicable across all container types in which SecurityContext is available. The authenticate() method is already an exception to this, unfortunately, but if we continue down this path, we'll have a single SecurityContext for all containers, but it will be a fat, kitchen-sink API, with a proliferation of hasAccesstoEJBMethod(), hasAccessToRestResource(), hasAccessToJMSQueue(), etc., methods.
I understand the concern, but the amount of things to query for access has stayed quite limited over the years and I don't foresee it really increasing much or at all.

This is, to me, a fundamental contradiction w.r.t. the design goal of SecurityContext. If it's container-independent, then it should have only generic methods, with uniform syntax and semantics across containers. If not, then it should be specialized by container -- WebSecurityContext, EjbSecurityContext, etc. An API with lots of methods that are useful in some containers but not others seems like a step backwards, not forwards. I think we need to decide what the real nature of SecurityContext is, before we start adding a lot of container-specific methods.

"accessToRestResource" for example is covered by the web resource (I know of no effort to introduce rest resource specific constraints in either JAX-RS or Java EE). 

Contrary to hasAccessToWebResource, one can programmatically just try the hypothetical hasAccesstoEJBMethod() with a try/catch.

Usage about hasAccessToWebResource is certainly not limited for callers in the web container. Business logic in e.g. the EJB container not rarely needs to know this too, for instance for generating reports or emails containing links.

But why would we do this if we could just add a single method that covers all resource types?

An alternative, which we may want to add anyway, now or at a later stage, is a "boolean hasAccessTo(java.security.Permission permission)" method, but I'm sure hasAccessToWebResource is a special case enough for it to warrant having its own method.

I'd be inclined to stay away from exposing Java SE Permissions, for reasons we can discuss later.

    • That brings me to the second concern -- there is no uniform representation of a resource that can be authorized. A string-based representation may make sense, but, if so, we need to define what the format of that string is, such that it can uniquely and unambiguously identify any Java EE resource, allows for flexible binding of authorization policies to resources, and efficient evaluation of policy at runtime, while remaining human readable and reasonably easy for developers to manipulate in code.

I agree with need for that representation, but the spec already addresses that:
[...]
The URLPatternSpec is very well defined. In it's simplest form it's a single exact matching URLPattern such as can be used in the @WebServlet annotation.

Right, URL patterns are well defined. I was talking about a syntax, or a type, that could describe all EE resources, not just URL resources. So that we could have a single method that would test access to any EE resource. Also, I'm not sure it's completely true that, e.g., Rest resources are identical to Web resources. On some level they are, but, for example, in some Rest applications the URL path alone is insufficient to identify the resource; query parameters also need to be taken into account.

(As a side note, it's not completely clear to me what an URL pattern is -- the URL Pattern Spec you quoted is defined in terms of URL pattern, but I just searched through the Servlet 3.1 spec looking for a definition of URL pattern and couldn't find it. So I'm unclear as to whether you could specify query parameters as part of an URL pattern, but I suspect not -- I've never seen one in an actual web.xml, and since HTTP sends the path portion of an URL separately from query parameters, my guess is that only the path part is considered part of an URL pattern.

    • The question of *who* hasAccessToXXX() isn't really addressed. Implicitly, it's the Subject currently on the stack, and maybe that's all we need, but we should consider the question. What happens if there is no authenticated user in the context?
The same as when isCallerInRole is called and there's no authenticated caller; a false is returned. We could clarify this a little more if you want?

"doesCallerHaveAccessToXXX()" might be a little clearer, but my point wasn't so much that people couldn't figure it out, I was just wondering if we needed or wanted a richer syntax, so you that you could ask questions about specific users. I don't think we need to change that.

    • What if we want to find out if User A has access to Resource X, where User A is not the currently authenticated user? (Asking that question should probably be allowed only for privileged users, but we should consider whether it's appropriate for the API.)

Alex had an initial proposal that incorporated queries that were quite like that, as well as functionality to create/update/delete users from compatible identity stores in a portable way. I think a privileged system level artefact that could be used to query for any caller would certainly make sense. However, for this release the EG and then spec lead Alex decided not to go forward with that yet, but to consider it for a next release. 

The SecurityContext, just like e.g. the FacesContext in JSF, and the SessionContext in EJB is aimed at the current caller (current subject at the stack aka the established authenticated identity).

That's probably OK. I think the use cases for asking about other users are all privileged use cases, so not appropriate for an application-level API.

At the end of the day, though, I still feel we need to drop those three methods. It's not that they aren't useful, but I think there are open questions about the direction to go with SecurityContext, and how we want to provide for authorization. I think we'll have accomplished something very useful with the authentication-related APIs we have; let's focus on delivering that with high quality, and wait on authorization until we can give it the attention it deserves.

Thanks, Arjan. Appreciate all your help and your patience with my questions!

Happy to help out and once again thanks for your thorough review.

Kind regards,
Arjan Tijms

Thanks again,

Will
-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803


Werner Keil
 

I also joined this group now.

While there has been some discussion on the "Experts" list at Google Groups, that should not be lost (unless there is a way to migrate all of it here) I did delete another Google Group "JSR 375 Contributors" meant to replace the "users" lists. Since this is now a combined "users" and "experts" list and there was almost no discussion there.

Regards,

Werner


Arjan Tijms
 

Hi,


On Thu, May 18, 2017 at 11:31 PM, Will Hopkins <will.hopkins@...> wrote:
You're right, defining the set of roles as Declared Roles does make the task easier. I think it's a little bit of trouble because it does mean the container now has to remember all the declared roles when parsing the deployment descriptor (and possibly annotations), but aside from that the logic isn't difficult.

I really am reluctant to add "new" stuff at this point, though -- it's not just work to spec, it's work for testing, and for RI, and this seems more like a convenience method than a necessity.

It's not exactly new I think (yes I see the quotes ;)). The method was mentioned in the earliest of the JIRA issues right at the beginning of the JSR, well over 2 years ago and has been discussed at various occasions during that time. It was added to the API 2 months ago and implemented in the RI.

Two months should be plenty of time. After an EDR things can still massively change, and this constituted only a small change, so process wise I think we should absolutely be good to go.

And it's a convenience method, but an important aspect of JSR 375 is making security easier to use. Conveniences are a very big part of that.

 
The author of the code presumably already knows the set of declared roles, since he or she will have written the web.xml as well as the code.
If they really need to query the declared roles the caller has, they can write the little loop iterating over the roles themselves. Not as convenient, agreed, but not difficult either.

True and that's how it's done today, but that code and the list of declared roles easily tend to run out of sync. If a declaration changes, the code needs to be changed. The "source of truth" (which declared roles there are) is difficult to keep at one place. Yes, there are options, but they are relatively obscure so people don't use them.

We saw this in JSF too. It was for instance always possible to build custom components, but there were a lot of moving parts people had to take care of, so they weren't much used by "regular developers". A few conveniences left and right and people actually started to use them.


 
This is, to me, a fundamental contradiction w.r.t. the design goal of SecurityContext. If it's container-independent, then it should have only generic methods, with uniform syntax and semantics across containers. If not, then it should be specialized by container -- WebSecurityContext, EjbSecurityContext, etc.

Nah, that isn't fully the purpose. It's a one-stop access point for applications to interact with the most common aspects of the security system. That indeed means generic methods with universal syntax (like is caller in role), but also a number of common container specific methods. The idea is for it to be facade, with the most common methods at the top level so they can be easily discovered by users.

Also don't forget tat the method is useful for other containers just the like. For instance an EJB method can call out to a web resource in the same application (either Servlet based or Rest based).

 
But why would we do this if we could just add a single method that covers all resource types?

Basically discoverability and minimal mental overhead for users.

A single method means a more obscure method, as users would have to read about its specification, parse the explanation and extract from it how it applies to their use case.

JASPIC is the ultimate example perhaps here. The ServerAuthModule has a very small API overhead, with the methods being usable for every possible container. But it's exactly that that makes them difficult to use. 

 
Right, URL patterns are well defined. I was talking about a syntax, or a type, that could describe all EE resources, not just URL resources. So that we could have a single method that would test access to any EE resource.

I'm not really sure that would be a good idea. Sure, we can take the existing well defined format and prefix it with say "web:", "ejb-method:" and what have you, but I don't think that would help with discoverability and make it easier for users to use.

Additionally, I think we would be hard pressed to come up with even 5 realistic types of resources to query for, let alone 10. Even if we would have a hasAccessToWebResource, hasAccessToRestResource, ... up till 10 methods, then I don't think that would be a bad thing at all. Contrary to some other types, Facade-like types can tolerate a somewhat larger amount of methods.

 
Also, I'm not sure it's completely true that, e.g., Rest resources are identical to Web resources. On some level they are, but, for example, in some Rest applications the URL path alone is insufficient to identify the resource; query parameters also need to be taken into account.

I know, and I didn't said they were identical, just that currently in practice web resource constraints are often used for them. An @RolesAllowed per method would change that, but even though this has been discussed in the JAX-RS EG it's still not standardised (some JAX-RS implementations support it, others don't) and I know of no current effort to change this.
 

(As a side note, it's not completely clear to me what an URL pattern is -- the URL Pattern Spec you quoted is defined in terms of URL pattern, but I just searched through the Servlet 3.1 spec looking for a definition of URL pattern and couldn't find it. So I'm unclear as to whether you could specify query parameters as part of an URL pattern, but I suspect not -- I've never seen one in an actual web.xml, and since HTTP sends the path portion of an URL separately from query parameters, my guess is that only the path part is considered part of an URL pattern.

Indeed, query parameters are not part of a URL pattern.

The pattern is specified very well in the JACC spec, so that would be the clearest source to reference. It spells it out exactly. It corresponds to the mapping as specified in section 12.1 and 12.2 (Specification of Mappings) in the Servlet 3.1 Spec, and is referenced by the "javaee:url-pattern" element in the deployment descriptor and in among others the @WebServlet annotation.

 
"doesCallerHaveAccessToXXX()" might be a little clearer,

That might make sense indeed, or perhaps an "hasCallerAccessTo".

 
but my point wasn't so much that people couldn't figure it out, I was just wondering if we needed or wanted a richer syntax, so you that you could ask questions about specific users. I don't think we need to change that.

At this moment not, but at a later stage definitely. More specifically, questions about specific roles were proposed too earlier.

 I think we'll have accomplished something very useful with the authentication-related APIs we have; let's focus on delivering that with high quality, and wait on authorization until we can give it the attention it deserves.

Full on authorization is something we sadly weren't able to include in the spec, although I spend a massive amount of time on this and have created a working prototype for easy to use application level custom authorization rules.

That said, isCallerInRole is a simple authorization method already, so it's not like we're not covering authorization at all.

Kind regards,
Arjan Tijms




 


Thanks, Arjan. Appreciate all your help and your patience with my questions!

Happy to help out and once again thanks for your thorough review.

Kind regards,
Arjan Tijms

Thanks again,

Will
-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803



Werner Keil
 

I got some good input by colleagues in the current project I support on SSO and security requirements they see.


Would it be possible to diversify the topics a bit?

Details on IsCallerInRole or other aspects feel a bit lost and hard to find even with some great tools groups.io provides if they all reply to the welcome thread ;-)


Kind Regards,

Werner


Will Hopkins
 

Yes, by all means -- I'm going to briefly reply to Arjan's latest email on the same thread, but for any new conversation let's start a new topic.

Note that I will be heads down for the rest of the day making the final updates to the spec and generating the materials to deliver to the JCP today for PRD, so I likely won't won't have time to digest any length email, or respond.

Will

On 05/19/2017 06:53 AM, Werner Keil wrote:

I got some good input by colleagues in the current project I support on SSO and security requirements they see.


Would it be possible to diversify the topics a bit?

Details on IsCallerInRole or other aspects feel a bit lost and hard to find even with some great tools groups.io provides if they all reply to the welcome thread ;-)


Kind Regards,

Werner


-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803


Will Hopkins
 

Hi Arjan,

I feel badly about this, but I'm going to say we need to leave it out. It was added after I made clear statements that we couldn't add any more scope to the API, I'm not convinced the approach is the right one (although, equally, I'm not convinced it's wrong), and we're out of time to have the EG weigh in.

Thanks,

Will

On 05/19/2017 06:30 AM, Arjan Tijms wrote:
Hi,


On Thu, May 18, 2017 at 11:31 PM, Will Hopkins <will.hopkins@...> wrote:
You're right, defining the set of roles as Declared Roles does make the task easier. I think it's a little bit of trouble because it does mean the container now has to remember all the declared roles when parsing the deployment descriptor (and possibly annotations), but aside from that the logic isn't difficult.

I really am reluctant to add "new" stuff at this point, though -- it's not just work to spec, it's work for testing, and for RI, and this seems more like a convenience method than a necessity.

It's not exactly new I think (yes I see the quotes ;)). The method was mentioned in the earliest of the JIRA issues right at the beginning of the JSR, well over 2 years ago and has been discussed at various occasions during that time. It was added to the API 2 months ago and implemented in the RI.

Two months should be plenty of time. After an EDR things can still massively change, and this constituted only a small change, so process wise I think we should absolutely be good to go.

And it's a convenience method, but an important aspect of JSR 375 is making security easier to use. Conveniences are a very big part of that.

 
The author of the code presumably already knows the set of declared roles, since he or she will have written the web.xml as well as the code.
If they really need to query the declared roles the caller has, they can write the little loop iterating over the roles themselves. Not as convenient, agreed, but not difficult either.

True and that's how it's done today, but that code and the list of declared roles easily tend to run out of sync. If a declaration changes, the code needs to be changed. The "source of truth" (which declared roles there are) is difficult to keep at one place. Yes, there are options, but they are relatively obscure so people don't use them.

We saw this in JSF too. It was for instance always possible to build custom components, but there were a lot of moving parts people had to take care of, so they weren't much used by "regular developers". A few conveniences left and right and people actually started to use them.


 
This is, to me, a fundamental contradiction w.r.t. the design goal of SecurityContext. If it's container-independent, then it should have only generic methods, with uniform syntax and semantics across containers. If not, then it should be specialized by container -- WebSecurityContext, EjbSecurityContext, etc.

Nah, that isn't fully the purpose. It's a one-stop access point for applications to interact with the most common aspects of the security system. That indeed means generic methods with universal syntax (like is caller in role), but also a number of common container specific methods. The idea is for it to be facade, with the most common methods at the top level so they can be easily discovered by users.

Also don't forget tat the method is useful for other containers just the like. For instance an EJB method can call out to a web resource in the same application (either Servlet based or Rest based).

 
But why would we do this if we could just add a single method that covers all resource types?

Basically discoverability and minimal mental overhead for users.

A single method means a more obscure method, as users would have to read about its specification, parse the explanation and extract from it how it applies to their use case.

JASPIC is the ultimate example perhaps here. The ServerAuthModule has a very small API overhead, with the methods being usable for every possible container. But it's exactly that that makes them difficult to use. 

 
Right, URL patterns are well defined. I was talking about a syntax, or a type, that could describe all EE resources, not just URL resources. So that we could have a single method that would test access to any EE resource.

I'm not really sure that would be a good idea. Sure, we can take the existing well defined format and prefix it with say "web:", "ejb-method:" and what have you, but I don't think that would help with discoverability and make it easier for users to use.

Additionally, I think we would be hard pressed to come up with even 5 realistic types of resources to query for, let alone 10. Even if we would have a hasAccessToWebResource, hasAccessToRestResource, ... up till 10 methods, then I don't think that would be a bad thing at all. Contrary to some other types, Facade-like types can tolerate a somewhat larger amount of methods.

 
Also, I'm not sure it's completely true that, e.g., Rest resources are identical to Web resources. On some level they are, but, for example, in some Rest applications the URL path alone is insufficient to identify the resource; query parameters also need to be taken into account.

I know, and I didn't said they were identical, just that currently in practice web resource constraints are often used for them. An @RolesAllowed per method would change that, but even though this has been discussed in the JAX-RS EG it's still not standardised (some JAX-RS implementations support it, others don't) and I know of no current effort to change this.
 

(As a side note, it's not completely clear to me what an URL pattern is -- the URL Pattern Spec you quoted is defined in terms of URL pattern, but I just searched through the Servlet 3.1 spec looking for a definition of URL pattern and couldn't find it. So I'm unclear as to whether you could specify query parameters as part of an URL pattern, but I suspect not -- I've never seen one in an actual web.xml, and since HTTP sends the path portion of an URL separately from query parameters, my guess is that only the path part is considered part of an URL pattern.

Indeed, query parameters are not part of a URL pattern.

The pattern is specified very well in the JACC spec, so that would be the clearest source to reference. It spells it out exactly. It corresponds to the mapping as specified in section 12.1 and 12.2 (Specification of Mappings) in the Servlet 3.1 Spec, and is referenced by the "javaee:url-pattern" element in the deployment descriptor and in among others the @WebServlet annotation.

 
"doesCallerHaveAccessToXXX()" might be a little clearer,

That might make sense indeed, or perhaps an "hasCallerAccessTo".

 
but my point wasn't so much that people couldn't figure it out, I was just wondering if we needed or wanted a richer syntax, so you that you could ask questions about specific users. I don't think we need to change that.

At this moment not, but at a later stage definitely. More specifically, questions about specific roles were proposed too earlier.

 I think we'll have accomplished something very useful with the authentication-related APIs we have; let's focus on delivering that with high quality, and wait on authorization until we can give it the attention it deserves.

Full on authorization is something we sadly weren't able to include in the spec, although I spend a massive amount of time on this and have created a working prototype for easy to use application level custom authorization rules.

That said, isCallerInRole is a simple authorization method already, so it's not like we're not covering authorization at all.

Kind regards,
Arjan Tijms




 


Thanks, Arjan. Appreciate all your help and your patience with my questions!

Happy to help out and once again thanks for your thorough review.

Kind regards,
Arjan Tijms

Thanks again,

Will
-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803


-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803


Will Hopkins
 

Great, thanks Werner. It's my understanding that the old email archives have been preserved, but I don't know how to access them at this point.

On 05/18/2017 05:18 PM, Werner Keil wrote:

I also joined this group now.

While there has been some discussion on the "Experts" list at Google Groups, that should not be lost (unless there is a way to migrate all of it here) I did delete another Google Group "JSR 375 Contributors" meant to replace the "users" lists. Since this is now a combined "users" and "experts" list and there was almost no discussion there.

Regards,

Werner


-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803


Arjan Tijms
 

Hi Will,

I think this is a very big miss. The Security API is already much smaller than we planned for, and besides criticisms that EE security is too difficult another criticism is that it takes way too long for simple things to get in. It's been 4 years since Java EE 7, which is a very long time.

The scope that you mentioned should have been more about totally new things; things that weren't discussed or not even had JIRA issues created for them. That was not the case for this one. As mentioned, it was presented to the EG, discussed, and in scope since the beginning of the JSR. So I really don't think it's about adding scope at all.

Taking this out would be just a waste. There's IMHO not reason to fix the feature set of a JSR even before the EDR is filed, and between the EDR and the PRD it's allowed to make many changes, so I don't really understand why you would want to limit the scope at all at that point.

Given more time, I'm sure this feature would not change at all and I haven't heard anyone besides you asking it to be radically different.

The only thing I think of that's "wrong" with it is that "JACC" is mentioned. I know that JACC triggers certain reactions in people (I started my JACC series a while ago with exactly that), and I've noticed that in the WLS manual there's still the passage about not using JACC that originates from the BEA days.

But as mentioned, this feature does not depend upon JACC. The format is fully specified by Servlet too, it's just that JACC has a very clear reference to exactly the same pattern (which is no coincidence as that worked together on it).

If JACC is really such a red flag, then we could as last resort take out even the textual references to JACC and only refer to the Servlet spec. Would that help?

Finally, even after the PRD, it's still allowed to make small changes. PRD is not "final". So we can always leave it in, and then if you or Oracle still wants it out because of JACC associations or otherwise it can still be removed before we go final.

Hope you want to reconsider this.

Kind regards,

Arjan Tijms


Will Hopkins
 

Hi Arjan,

The issue isn't JACC at all, the issue is that it's not fully baked, at least IMO, and we're out of time. It may have been discussed in the past, or mentioned in JIRAs, but it was not previously in the actual API or spec.

I think it's harder to add something after PRD than to remove it, but adding something won't be any harder than making major changes to something that's in the PRD. I'm open to considering what that API would look like after the PRD is published, and potentially adding it back in.

Will

On 05/19/2017 10:58 AM, Arjan Tijms wrote:

Hi Will,

I think this is a very big miss. The Security API is already much smaller than we planned for, and besides criticisms that EE security is too difficult another criticism is that it takes way too long for simple things to get in. It's been 4 years since Java EE 7, which is a very long time.

The scope that you mentioned should have been more about totally new things; things that weren't discussed or not even had JIRA issues created for them. That was not the case for this one. As mentioned, it was presented to the EG, discussed, and in scope since the beginning of the JSR. So I really don't think it's about adding scope at all.

Taking this out would be just a waste. There's IMHO not reason to fix the feature set of a JSR even before the EDR is filed, and between the EDR and the PRD it's allowed to make many changes, so I don't really understand why you would want to limit the scope at all at that point.

Given more time, I'm sure this feature would not change at all and I haven't heard anyone besides you asking it to be radically different.

The only thing I think of that's "wrong" with it is that "JACC" is mentioned. I know that JACC triggers certain reactions in people (I started my JACC series a while ago with exactly that), and I've noticed that in the WLS manual there's still the passage about not using JACC that originates from the BEA days.

But as mentioned, this feature does not depend upon JACC. The format is fully specified by Servlet too, it's just that JACC has a very clear reference to exactly the same pattern (which is no coincidence as that worked together on it).

If JACC is really such a red flag, then we could as last resort take out even the textual references to JACC and only refer to the Servlet spec. Would that help?

Finally, even after the PRD, it's still allowed to make small changes. PRD is not "final". So we can always leave it in, and then if you or Oracle still wants it out because of JACC associations or otherwise it can still be removed before we go final.

Hope you want to reconsider this.

Kind regards,

Arjan Tijms


-- 
Will Hopkins | WebLogic Security Architect | +1.781.442.0310
Oracle Application Development
35 Network Drive, Burlington, MA 01803