Re: A possible JEP to replace SecurityManager after JEP 411

2022-04-26 Thread David Lloyd
On Tue, Apr 26, 2022 at 8:17 AM Mario Torre  wrote:
> I think there's a difference between "perceived" security and "actual" one.
>
> The SM in today's post Spectre, Meltdown and the likes world is
> "perceived" security, which may lead to a relaxation on the security
> of other layers because at least "we have additional security
> checkpoints in the JDK".

I agree. Perceived security is a problem that has plagued
SecurityManager since its inception, due in no small part to the noble
(if misguided) apparent initial intention that it was to be an
_actual_ sandbox. Fast forward through a quarter of a century of
bitter education, and still, nothing in the documentation really
refutes this or sets up a realistic expectation for what level of
protection is _actually_ provided. Even the deprecation notice doesn't
give any indication that SecurityManager is anything less than a
perfect sandbox, referring instead to JEP 411.

A necessary part of the proposal is a documentation overhaul that
defines the exact contract of the API as well as a real-world
discussion about what level of protection is (and is not) actually
provided. If the user comes away from reading the documentation with
the idea that turning on this switch means that they don't have to
worry about security anymore, then that is a big problem. And from
personal experience, I believe this is the case today, at least to a
degree.

-- 
- DML • he/him



Re: A possible JEP to replace SecurityManager after JEP 411

2022-04-26 Thread David Lloyd
On Tue, Apr 26, 2022 at 4:47 AM Alan Bateman  wrote:
>
> On 25/04/2022 13:53, David Lloyd wrote:
> > Nothing in the proposal causes splitting or delegation of
> > responsibilities. It is _only_ about preserving security checkpoints
> > in the JDK, which *add* a layer of checks to what the container might
> > already provide. Nothing is being subtracted, and thus I find it hard
> > to accept that preserving these checks somehow reduces security
> > overall.
>
> "preserving security checkpoints in the JDK" suggests just leaving the
> calls do AccessController.doPrivileged and
> SecurityManager.checkPermission in place.

The proposal is clear that the JDK 'doPrivileged' calls would not be
necessary, and it is not necessary to exactly use `checkPermission`
depending on the approach taken. This might or might not counter your
argument, but as I said before, even if the proposal is rejected, it
should be rejected based on a fair and accurate evaluation of what is
proposed.

> That amounts to putting a tax
> on every feature, every JEP, and all ongoing maintenance of the
> platform. If there is refactoring or a change that forgets to insert a
> checkPermission somewhere then we have a security issue and everything
> that goes along with that.

Sure, if you continue to think of SecurityManager as a sandbox, then
every case where it is not a perfect sandbox can be a major security
issue. In real terms though, was SecurityManager ever _actually_
effective as a sandbox? It is certainly the case that few (if any) of
us who do systems and security programming in application servers and
containers at Red Hat have ever considered it so. Thus, the proposal
as given tries to reflect this reality by treating the SecurityManager
as exactly two things: firstly, a central point for any application or
library to be able to statically perform an authorization check
without needing to rely on any library other than the JDK itself; and
secondly, a mechanism by which certain operations performed by the JDK
can be authorized by the user.

The lack of an authorization check would therefore be less impactful
when considered from this perspective. Maybe a security issue, yes,
but not a sandbox escape because there _is_ no sandbox. It's not a
part of the contract anymore. In fact, part of the contract could even
be an _explicit admonition_ that missing checks in the JDK would be
considered bugs but not security issues. Even that would be better
than removing the whole thing; if doing X has an unacceptable cost,
let's stop doing X instead of getting rid of Y.

> I think Martin is right that hooking authorization libraries into low
> level libraries isn't the right way to do this. Aside from the
> complexity methods I would add that threads pools or any hand-off
> between threads will typically break when the context is not carried.

Propagation of security context is not a problem that needs a solution
in the JDK and is not a part of the proposal. It's generally already a
solved problem within containers, where this feature is intended to be
used, including cases where threads are created or tasks are
dispatched to thread pools, fork/join, etc. Thus the proposal
establishes that this is 100% the responsibility of the security
manager implementation.

> One other point about authorization libraries wanting to hook into low
> level code is that it's a minefield of potential issues with recursive
> initialization, stack overflows and some really hard to diagnose issues.
> JDK-8155659 [1] is one report that comes to mind.

It is up to the implementation to deal with recursive calls into the
security manager today (and we already do so to our own satisfaction),
and I don't see any reason why that wouldn't continue to be the case.
In my opinion, this would not be an issue that the JDK has to be
concerned about (other than maybe as a documentation issue).

-- 
- DML • he/him



Re: A possible JEP to replace SecurityManager after JEP 411

2022-04-25 Thread David Lloyd
On Fri, Apr 22, 2022 at 10:04 PM Martin Balao  wrote:
> In my view, authorization decisions at higher layer generally
> have better context, are more clear and less riskier. At a lower layer
> there is more complexity and chances of both subtle combinations or
> unseen paths that may lead to check bypasses. I lean towards not
> splitting authorization responsibility through different layers, which
> might create confusion or even a false sense of security in some cases.
> To illustrate with a trivial example, if a subject is not supposed to
> access to some information, it's the application that has enough context
> to decide and block right away. Letting the attempt go though the call
> stack down to the network or the file system might be the recipe for
> missing a channel.

The point of the proposal is to allow checks at system-level
operations _in addition to_ application-level operations. It sounds
like you are saying that adding the extra authorization check is
somehow less secure? I'm not sure I agree with that reasoning.

> I won't enter the untrusted code case -which has been extensively
> discussed already- but want to briefly mention something about the
> "trusted code performing risky operations" case. My first point is that
> vulnerabilities at the JVM level (i.e.: memory safety compromises) are
> serious enough to potentially bypass a SecurityManager. My second point
> is that the SecurityManager is really unable to deal with tainted data
> flowing from low to high integrity domains, and sanitation must be
> performed by the application or the library anyways because there are
> infinite ways in which it can be harmful. Even when the data is obtained
> at a low integrity domain, there will be a flow towards a high integrity
> domain to perform a legitimate action (i.e.: SQL query, OS command
> execution, etc). The OS and the DB engine, in this example, have the
> knowledge, granularity and power to be the next level of enforcement
> after data sanitation. Again, I wouldn't split this responsibility or
> pass it to the JDK for the same reasons than before.

Nothing in the proposal causes splitting or delegation of
responsibilities. It is _only_ about preserving security checkpoints
in the JDK, which *add* a layer of checks to what the container might
already provide. Nothing is being subtracted, and thus I find it hard
to accept that preserving these checks somehow reduces security
overall. In absolute terms, using a security manager (even with all of
its problems) is *more* secure than not using it on the same code base
and deployment. I'm not sure how this can be rationally refuted. The
proposal is about retaining this incremental increase in security,
while both adjusting the user's expectations via documentation *and*
significantly reducing the burden of CVEs and maintenance on the JDK
itself (and putting it on to the third party authorization framework),
which in my estimation is the *real* stakes here.

Nothing in the proposal is intended to solve the issue of tainted
data; with or without this proposal, this is an unsolved problem.
-- 
- DML • he/him



Re: A possible JEP to replace SecurityManager after JEP 411

2022-04-08 Thread David Lloyd
On Fri, Apr 8, 2022 at 10:14 AM Sean Mullan  wrote:
>
> Ok, thanks for some clarification on the proposal.
>
> How many applications currently depend on the SM for this type of usage?
> What other alternate models have you considered?

There are some number of customers and users within our user base who
rely on SM for certain security certifications or requirements. The
alternative would be to attempt to reframe these certifications or
requirements on a case by case basis to exclude SM, which might or
might not be less work than preserving it.

> In general, I think authorization is best done at a higher layer within
> the application and not via low-level SM callouts. Authorize the subject
> first and if not acceptable, prevent the operation or API from being
> called in the first place. Once the operation is in motion, you have
> already taken a greater risk that something might go wrong.

The low level authorization checks would be in addition to the high
level checks that we already perform. But I understand your position.

>  > I hope this clarifies things. Like I said, "no" is an acceptable
>  > answer for us but I would be remiss if I didn't ensure that the "no"
>  > was based on an accurate understanding of what we are proposing, so
>  > hopefully this helps.
>
> It does help, but not enough to change my previous stance.

OK, thanks for the feedback.

-- 
- DML • he/him



Re: A possible JEP to replace SecurityManager after JEP 411

2022-04-08 Thread David Lloyd
Small correction:

On Fri, Apr 8, 2022 at 8:03 AM David Lloyd  wrote:

> Instead the API would exist to give containers and applications an
> extra layer of authorization which does not exist today.

Of course I meant "which would not exist after JEP 411".

-- 
- DML • he/him



Re: A possible JEP to replace SecurityManager after JEP 411

2022-04-08 Thread David Lloyd
Hi Sean, thanks for replying. I'll reply inline.

On Thu, Apr 7, 2022 at 2:20 PM Sean Mullan  wrote:
>
> With this proposal, as I understand it, the JDK would still be
> responsible for maintaining and preserving essentially all of the
> existing calls to the Security Manager (SM).

I think we'd actually want to evaluate each case to decide whether to
retain the permission check. But I didn't want to preemptively
eliminate any check, especially considering that the `Permission`
classes were not included in JEP 411.

> All new code and APIs would
> still need to be evaluated and determined if permission checks were
> needed as well as making appropriate specification changes to note the
> behavior when an SM is enabled (throwing a SecurityException, etc).

Correct.

> Any missing checks would need to be treated as security issues.

This I disagree with: we are not proposing retention of this API for
sandboxing purposes. In fact the only reason to keep the name
`SecurityManager` is for compatibility purposes.

Instead the API would exist to give containers and applications an
extra layer of authorization which does not exist today.
Hypothetically speaking, if even one authorization check is retained,
then that is more than would exist if the API were removed. There
would be no expectation that usage of this API conveys any kind of end
to end security, and this would be explicitly conveyed in the API
documentation.

> And we would
> still need to test the code and APIs to ensure that it worked properly
> and complied with the API specification. This would likely mean
> implementing and maintaining an internal SM implementation in OpenJDK.

Only within the test suite, and this would be expected to be trivial
implementations only: always allow vs always throw.

> The proposal also includes retaining calls to doPrivileged (but later
> potentially replacing them with some other mechanism TBD).

No, this is incorrect; the proposal includes retaining the *methods*,
not the calls; this is explicitly left off of the proposal.

> The JDK source code includes over 1000 calls to doPrivileged. Each of these 
> need
> to be carefully reviewed to ensure that they do not contain security
> issues and any new code needs to be evaluated to see if new calls to
> doPrivileged are necessary.

Under this proposal, it would be perfectly acceptable to eliminate
these calls, and put the onus on the implementation to decide the
circumstances of the invocation. The JDK would be under no obligation
to provide any level of actual security - it would only be obligated
to use the installed SecurityManager, if any, for authorization checks
at some subset of the locations at which an authorization check
currently takes place.

> Retaining doPrivileged (or something similar) means that there can be
> domains of code with different permissions running within the VM, which
> retains much of the complexity of the current SM model.

The only reason to retain these methods is the massive amount of
existing code which is doing something along the lines of:

if (System.getSecurityManager() != null) {
doPrivileged(something);
} else {
something.run();
}

It has nothing to do with what is in the JDK, which is why this item
was marked "optional" in the initial proposal.

> In this proposal, how privileges are established or propagated is
> implementation-specific. But how could applications or libraries depend
> on the APIs and still have some confidence that the code is behaving
> consistently and securely?

It is already an error to assume that code is behaving consistently
and securely on the sole basis of the presence of a SecurityManager,
and this would not change; the major difference is that the API would
be able to explicitly document this fact. The value that the user
gains from using a SecurityManager versus not using one would be that
JDK operations would be authorization checked.

> Today, the cost of buying into the SM model is high for libraries and
> applications. Not many third party libraries support the SM and have
> modified their code to perform permission checks and call doPrivileged
> in the right places. If there were pluggable SMs each behaving
> differently, there would likely be less incentive.

The point of retaining these APIs isn't necessarily to increase usage;
it is to provide some continuity for things which already expect some
level of authorization from the JDK. We think that it is the case that
this would add some value for our customers and users which offsets
the value lost by the removal of SM. The question is, is the value
enough to make our efforts to make this happen worthwhile? This is not
only a question of the cost of developing the JEP and implementation
effort, but also a question of how difficult it will be to gain enough
acceptance of the idea in the upstream community that the change could
potentially be integrated.

> Although it sounds beneficial to be able to delegate the SM
> implementation 

Re: A possible JEP to replace SecurityManager after JEP 411

2022-04-06 Thread David Lloyd
Thanks for responding Peter, I've commented inline below.

On Tue, Apr 5, 2022 at 8:19 PM Peter Firmstone
 wrote:
>
> Thanks David,
>
> I'd certainly support such a proposal and encourage OpenJDK to consider
> exploring it.
>
> Perhaps also consider; no privileges should be granted unless a
> privileged call is made, this simplifies the the stack walk, such that
> it's only required when a privileged call is made.

I think this is a reasonable requirement for an implementation. If we
move forward with this however, the proposal would explicitly not
specify how privileges are established or propagated; that would be
implementation-defined.

> With a policy tool that generates policy files, it allows the developer
> to turn off all features that are not required, which improves security.

Policy is specifically excluded from the proposal and would be
implementation-defined. This should grant significantly more
flexibility to meet the authorization needs of the end user.

For example, one implementation could be designed to perform
authorization checks based on the call stack, like AccessController
does today. Another implementation might authorize based on an
authenticated user identity and not be concerned with the call stack
at all. Other implementations might do both.

> Something that bothered me about SocketPermission was that it didn't
> allow granting permission to subnets, or ranges of IP addresses.
>
> It always bothered me that data parsing isn't controlled with permissions.
>
> For data parsing the remote authenticated subject represents the source
> of the data, if the data source cannot be authenticated, then data
> cannot be parsed.   Of  course when parsing is done it needs to be
> validated, but authentication goes a long way to filtering out potential
> attack vectors.

By removing the bulk of the specification from the JDK, the
implementation is more free to define how permissions are granted and
checked (for example, by authorizing by CIDR ranges rather than by
single addresses). Both proposal options would allow for the
possibility of adding more authorization checks in the future in a
reasonably backwards compatible way, though I think in practice it is
unlikely (except maybe in the context of new features that might
benefit from such checks).

The reason that this is a "proposal to propose" rather than an actual
proposal is because we ourselves do not have a clear answer as to
whether the costs even of this simplified compromise solution are low
enough, compared to the expected value return, to justify the work
necessary to produce a formal proposal and prototype. If the upstream
consensus is "no, we're not likely to approve this", we're going to
consider having to plead our case when we decide whether we want to
devote that effort as an additional cost.

And we really do have to balance cost and benefit *to us* as well as
to the OpenJDK team. When suggesting new features it's always tempting
to try and "boil the ocean", especially when inspiration strikes and
one is tempted to incorporate dozens of new and interesting ideas. But
in this particular case, the real benefit comes from figuring out what
we can *remove* versus what must be kept: the more we can remove, the
simpler the change would be, and the more likely it will be that we
can justify the labor involved in making the change - and, I think,
the more likely that the proposal would be palatable to the upstream
OpenJDK maintainers.

As a final note, I don't think that any context-based authorization
based system would really suffice for handling tainted data,
unfortunately. For some languages, tainted data is handled at a
language level. We would not set out to solve that problem with this
proposal, though I think we would all agree that this is a real
problem and I think it's safe to speculate that Red Hat would likely
be in favor of any effort to tackle it.

-- 
- DML • he/him



A possible JEP to replace SecurityManager after JEP 411

2022-04-05 Thread David Lloyd
Here at Red Hat there have been serious discussions about the impacts
of security manager removal on our users, and whether there is an
actual value impact, and if so, whether it can be mitigated or
reversed somehow. We are interested in exploring whether we can come
up with a way in which vendors and projects that wish to continue
using SecurityManager (or something like it) would be able to do so,
while still removing the majority of the ongoing maintenance burden
from the OpenJDK project.

Before we make a decision on whether or not we think there is
sufficient justification for working up a formal JEP, we have decided
that the best first step would be to socialize the idea in a more
general form so that we can know whether the upstream OpenJDK team
would even be amenable *at all* to the solution (or something like
it), particularly in light of the observation that previous threads
about retaining SecurityManager in any form have been looked upon in a
fairly negative light.

The primary idea behind this proposal is that, while all of the points
in JEP 411 relating to the lack of what most experts might refer to as
"actual security" are certainly true, the SecurityManager mechanism
itself does nevertheless have some inherent value. The challenge,
then, is to strike a balance between the value provided by retaining
some semblance of the mechanism versus the costs inherent in retaining
it; we would want as much of the former as possible, for as little of
the latter as possible.

So, here's the idea. It is assumed (for the sake of common
understanding) that as things stand, all of the classes and members
marked as "deprecated for removal" as a part of JEP 411 are intended
to be completely removed without replacement at the end of the term of
deprecation.  The proposals here are based on this assumption.

The center of this proposal is that, at the end of the term of
deprecation, all of the deprecated classes, members, and behavior are
still removed (including, and especially, AccessController and Policy
and related classes) /except/ as mentioned here:

 * Rather than completely removing SecurityManager,
 * The SecurityManager class becomes abstract and non-deprecated,
with all of its methods being removed, except as follows
 * SecurityManager.getSecurityContext() becomes abstract (this is
the one that returns Object, *not* the stack walking one)
 * SecurityManager.checkPermission() (both of them) become abstract
 * Rather than removing the SecurityManager-related methods from System,
 * System.getSecurityManager() is retained and de-deprecated
 * [Optional] System.setSecurityManager() is retained and
de-deprecated (we would want to explore whether it is feasible to
replace this (and the system property lookup mechanism) using
ServiceLoader, if bootstrap allows it)
 * [Optional] Rather than /immediately/ removing all of AccessController,
 * Retain its deprecation-for-removal status
 * Retain only doPrivileged(PrivilegedAction) and
doPrivileged(PrivilegedExceptionAction) as simple pass-throughs (no
JVM semantics other than being present on the call stack like any
method) since they are pervasively used, to allow frameworks time to
transition to (for example) a third-party alternative.

The burden of permission verification would lie completely with the
security manager implementation.  The JDK would not have a
'SecurityManager' implementation of any kind, outside of the internal
test suite.

The other part of this proposal can come in one of two possible flavors.

### Option 1: Authorization interfaces

Each point in the JDK where there presently is a permission check is
classified into an authorization category of related operations. An
interface is introduced for each category which contains the methods
encapsulating the relevant check, in a package that is deemed most
appropriate for that particular grouping.  For example, there might be
a 'SocketAuthorization' interface in the 'java.net' package, with
methods like 'checkConnect(SocketAddress from, SocketAddress to)' and
'checkAccept(SocketAddress addr)'.

At the point where a permission check previously would take place, a
check like this is performed instead:

if (System.getSecurityManager() instanceof SocketAuthorization sa) {
sa.checkAccept(addr);
}

Any public or protected method with such a check should include
@throws Javadoc explaining that a SecurityException may be thrown.

The Permission subclasses previously used specifically by these
operation sites *may* in this case be deprecated for removal
immediately or at some point in the future, if desired.

It is the sole responsibility of the SecurityManager implementer to
implement the various necessary interfaces, and any third-party
authorization interfaces that would also be relevant.

### Option 2: Retain permission system

Under this option, the existing authorization checks are mostly
retained, however, since the SecurityManager class only has a gene

Re: JEP 411: Deprecation with removal would break most existing Java libraries

2021-06-16 Thread David Lloyd
On Mon, Jun 14, 2021 at 6:47 PM Peter Firmstone
 wrote:
> SecurityManager depends on Permission, currently there are Permission
> checks throughout the JVM, however Permission implementation classes
> will be removed, although the Permission class itself won't be.

This is incorrect AFAICT.  The relevant JEP text is:

> Permission and subclasses — Other significant classes, such as 
> ProtectionDomain, depend on Permission. Many of the subclasses of Permission, 
> however, are specific to use cases which will likely no longer be relevant 
> after the Security Manager is removed. The maintainers of these subclasses 
> can deprecate and remove them separately, after evaluating the compatibility 
> risk.

> Permission references can be replaced with Guard references (which
> Permissions are instances of).

I guess you've got something fairly complex in mind, could you give
some practical examples of how this would work?

> The Permission implementations of Guard::check call SecurityManager, so
> checks will continue working as expected, but it allows us to intercept
> them and do something different.

What do you envision these checks looking like?  Where would the JDK
find these Guard instances?

> By replacing Permission references with Guard, it allows us to implement
> our own checks in these locations, and OpenJDK doesn't need to maintain
> Permission instances, and or, we don't need to make use of unmaintained
> Permission implementations.

As I said I don't think there is an intention to remove the permission
classes just yet, and I don't think that it is a fair statement to say
that the permission implementations would be unmaintained.  Most of
those classes have not needed to be touched in many years; there's
just not a lot of complexity there for the vast majority of them.

> There are already issues with Permission implementations, take for
> example SocketPermission, it consults DNS and it isn't possible to enter
> a range of IP addresses (such as the local subnet, and a list of public
> IP addresses), for now, every single IP address must be entered and this
> isn't practical.   The proposed API would allow us to re-implement
> SocketPermission functionality, as well as other Permission implementations.

Sure, this would be nice to clean up.  I just don't understand the
proposed mechanism.

> > On Mon, Jun 14, 2021 at 12:57 AM Alan Bateman  
> > wrote:
> >> AccessController::doPriv just runs the action.
> > TBH this should have always been the case.  Implementation-wise, if
> > one were constructing an access control context based on stack
> > walking, one would stop at points where `AccessController` is on the
> > stack (which is easily determinable) to do special work on assembling
> > the access control context based on the method called at that frame.
>
> Yes, one can do that, but these classes will also eventually be removed.

Sure. This was mainly a practical observation about the current
implementation.  And any replacement third-party stack-based
authorization system could (and should) use a similar mechanism
regardless of whether these exact methods stay in the JDK for 5 or 50
more years.

-- 
- DML • he/him



Re: JEP 411: Deprecation with removal would break most existing Java libraries

2021-06-14 Thread David Lloyd
On Mon, Jun 14, 2021 at 2:38 AM Peter Firmstone
 wrote:
>  1. Develop authorization layer security provider services in OpenJDK,
> back port it to Java 8 and Java 11 (these provide most of the
> utilised functionality of SecurityManager, allowing developers to
> only implement those which they need, without enabling
> SecurityManager and editing policy files).
>  2. Remove SecurityManager, Policy and Policy provider in OpenJDK 19.

The SecurityManager class itself already is *exactly* an authorization
provider.  I don't think it makes sense to consider removing the
security manager class to replace it with something that has basically
exactly the same API (specifically, a single method for each general
authorization check that can be called without constructing any new
objects, if and only if the authorization provider is installed).  See
my other proposal where, post-"removal", SecurityManager (the class)
is retained but made abstract (and sans a few methods).  All of the
existing code which performs authorization checks would be retained
and the problem solved in essentially the way you're describing, just
using existing APIs.

The security manager implementation itself can implement any kind of
authorization behavior whatsoever, based mainly on the Permission
types (which work just fine for this purpose, and anyway are already
retained by the current JEP).  Policy and its supporting classes are
completely unnecessary for implementing a security policy.  In fact,
this is the case today already.

On Mon, Jun 14, 2021 at 12:57 AM Alan Bateman  wrote:
> AccessController::doPriv just runs the action.

TBH this should have always been the case.  Implementation-wise, if
one were constructing an access control context based on stack
walking, one would stop at points where `AccessController` is on the
stack (which is easily determinable) to do special work on assembling
the access control context based on the method called at that frame.

-- 
- DML • he/him



Re: JEP 411: Missing use-case: user functions in an RDBMS

2021-05-28 Thread David Lloyd
On Thu, May 27, 2021 at 8:36 PM Chapman Flack  wrote:

> Hello, I see I am another person relatively late to stumble on this
> "well publicized" JEP. (I am not sure how to recommend the publicity
> could have been better handled, but apparently the avenues that were
> used aren't ones that reached me.)
>
> I maintain, on a volunteer basis, the extension for Java server-side
> functions in the PostgreSQL RDBMS [1].
>

I posted this alternative proposal over on jdk-dev, I suppose I should
have CC'd this list as well. The idea is that maybe the SecurityManager
could stay with enough scaffolding for a third party  (say, your extension)
to be able to use their own stack-based access checker (made practical
thanks to the new-ish StackWalker API).

Anyway here it is:

------ Forwarded message -
From: David Lloyd 
Date: Sat, May 22, 2021 at 9:11 AM
Subject: Re: JEP proposed to target JDK 17: 411: Deprecate the Security
Manager for Removal
To: 
Cc: jdk-dev 



On Fri, May 21, 2021 at 6:04 PM  wrote:

> The following JEP is proposed to target JDK 17:
>
>   411: Deprecate the Security Manager for Removal
>https://openjdk.java.net/jeps/411


I'm not a committer or reviewer, so perhaps my feedback is unwelcome - but
one can't help but note the amount of heated discussion on this topic, and
the determination of whether or not the problematic points have been
addressed satisfactorily is probably pretty subjective.  I have a bit of
experience in this area, having either designed or having had a major part
of the design of the authentication and authorization APIs presently in use
in WildFly [1], as well as the WildFly security manager [2] (and a
significant number of other security-related APIs), so I thought I'd give
some feedback and also offer a possible compromise.

The security manager defines a variety of behaviors.  Some of these have
clearly been supplanted by other APIs (like getClassContext() vs
StackWalker) or security mechanisms (checkPackageAccess() and friends vs
the new encapsulation protections).  There are also however other APIs on
the security manager which clearly have no replacement.  These include
socket and file access checking APIs, and of course the general permission
check methods.  In addition, the JEP proposes deprecation of the access
controller and policy classes which are the mechanism of stack-based access
checking, and finally unavoidably bumps against JAAS, which itself is a
very difficult and problematic API which also uses the same stack-based
access mechanism (and which we here at Red Hat have for the most part long
since abandoned).

A large part of this deprecated machinery relates to the stack-based access
controller (not to mention, I believe, a nontrivial number of CVEs).  The
idea, of course, was that one can authenticate untrusted or semi-trusted
code.  It is definitely clear through years of experience on all sides
though that one cannot truly rely on this mechanism to protect against
malicious code; it is as easy as an infinite loop to cause massive
undesired CPU usage (and, in this modern $/cpu world, cost) for example.

However, one other undeniably useful function of the security manager is to
authorize basic native operations *not* in the context of what code is
executing, but what person or principal is executing it.  In other words,
the use case of *trusted* code running on behalf of one of many potentially
untrusted users - probably the widest application of server-side Java in
existence today.  It cannot be argued that removing all of the above
security checks does not weaken security of such users, when they could
have a narrower authorization applied to them to limit the possibility of
impact of server exploitation.

On the other hand, the cost of the SecurityManager mechanism as it stands
is undeniably too high; there is absolutely no point in arguing
otherwise, in my view.  Leaving aside the substantial CVE load, the access
controller and policy APIs are very difficult to use correctly by
containers and frameworks, for one thing, and are cumbersome for users as
well.  Many users and frameworks get doPrivileged() wrong, and combining
JAAS subjects into the same mechanism historically doesn't even work
consistently between otherwise-compliant JDK implementations.

What I would propose then is a compromise aimed at maximizing the amount of
value retained and minimizing the amount of cost incurred, by *only*
retaining permission checks that specifically pertain to or are useful for
user authorization, while *also* deprecating (for removal) the existing
problem-prone stack-based access checking mechanism, policy, and security
manager implementation.

Thus I would suggest not deprecating all of SecurityManager, rather just
the following:

* java.lang.SecurityManager#getClassContext - it is replaced by StackWalker
* java.lang.SecurityManager#getSecurityContext
* 

Re: New candidate JEP: 411: Deprecate the Security Manager for Removal

2021-04-29 Thread David Lloyd
If it helps, we've solved this particular problem in a couple of places by
using an MR-JAR which selects an implementation using `StackWalker` when
Java 9+ is used.  I will say however that it appears to be slightly less
performant, which is unfortunate (but hopefully fixable at some point in
the future).

On Thu, Apr 29, 2021 at 1:48 AM Peter Firmstone 
wrote:

> We also use the SecurityManager for caller sensitive method calls.
>
> I re-implemented a secure implementation of Java Serialization, using a
> public API and fewer features (eg no circular links), in this
> implementation, each class in an object's hierarchy has its own
> namespace, the calling class is discovered using a SecurityManager
> subclass.
>
> --
> Regards,
>
> Peter Firmstone
> 0498 286 363
> Zeus Project Services Pty Ltd.
>
> On 29/04/2021 3:37 pm, Peter Firmstone wrote:
> > Which version of Java is this planned for?   Will the last version
> > supporting the security manager be a long term support version, eg
> > back ports of security patches and TLS technologies?
> >
> > We have our own security manager implementation and policy provider
> > implementations.  Both of these are high performance and non-blocking
> > and we are able to dynamically grant and revoke some permissions.
> > While I acknowledge the Java policy implementation has a significant
> > performance impact, due to blocking permission checks, ours is less
> > than 1%.  Our software doesn't share PermissionCollection instances
> > among threads, or even have a Permissions cache,
> > PermissionCollection's are generated for each permission check and
> > discarded for garbage collection, the Permission object themselves are
> > cached (after initialization and safe publication), as are the results
> > of repeated permission checks.  We also have our own Permission
> > implementations.
> >
> > We have tools that generate policy files with least privilege,
> > although we will manually alter them with wildcards, for network
> > connections for instance.
> >
> > In our software, dynamic permissions are granted after authentication
> > of TLS connections.
> >
> > It is too early for me to tell if there are suitable replacement
> > technologies available.  I can understand the motivation for reducing
> > Java's software development burden, but I think this version of Java
> > might be the last for us, it would certainly be good if a long term
> > support version was available, perhaps indefinitely lol.
> >
> > Regards,
> >
> > Peter Firmstone
> > Zeus Project Services Pty Ltd.
> >
> >
> > On 16/04/2021 4:05 am, mark.reinh...@oracle.com wrote:
> >> https://openjdk.java.net/jeps/411
> >>
> >>Summary: Deprecate the Security Manager for removal in a future
> >>release. The Security Manager dates from Java 1.0. It has not been
> >> the
> >>primary means of securing client-side Java code for many years,
> >> and it
> >>has rarely been used to secure server-side code. To move Java
> >> forward,
> >>we intend to deprecate the Security Manager for removal in concert
> >> with
> >>the legacy Applet API (JEP 398).
> >>
> >> - Mark
> >
>
>

-- 
- DML • he/him


Re: RFR: 8253368: TLS connection always receives close_notify exception

2020-11-13 Thread David Lloyd
Thinking about this some more, I suppose I tend to interpret the
shutdownInput() API as indicating that the caller doesn't really care
about any further data being received on the socket.  I expect that a
truncation attack would only be significant if the socket EOF was
received before close_notify while the caller is trying to read data
from the socket.  So in that light the change is probably OK (since it
only exists when explicitly shutting down the input) from the
perspective that I was thinking of.  Whether it has any protocol-level
implications I don't know and won't comment on.

That said, maybe shutdownInput(boolean) should be collapsed into
shutdownInput() since the parameter is no longer used?

On Fri, Nov 13, 2020 at 3:01 PM Seán Coffey  wrote:
>
> Open to discussion on that. It's also highlighted in the TLSv1.3 RFC.
>
> The TLS spec doesn't require a fatal alert to be issued when closing
> inbound without receiving the peer's close_notify. I've seen a handful
> of applications making JDK upgrades which are seeing regression as a
> result of this new JDK check (like the one described in this bug). If
> we're to keep the check in the JDK, should we restrict it to the v1.3
> protocol or should we implement it based on a system property perhaps ?
>
> regards,
> Sean.
>
> On 13/11/2020 17:11, David Lloyd wrote:
> > How would a truncation attack be avoided in this case?
> >
> > On Fri, Nov 13, 2020 at 8:23 AM Sean Coffey  
> > wrote:
> >> removing the "closing inbound before receiving peer's close_notify" 
> >> exception that can be seen with TLS stack if calling close on inbound. 
> >> After reading the relevant parts of the TLS v1.2/v1.3 RFCs, I believe the 
> >> local end point doesn't have to wait for close_notify alert from remote 
> >> end.
> >>
> >> -
> >>
> >> Commit messages:
> >>   - 8253368: TLS connection always receives close_notify exception
> >>
> >> Changes: https://git.openjdk.java.net/jdk/pull/1205/files
> >>   Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1205&range=00
> >>Issue: https://bugs.openjdk.java.net/browse/JDK-8253368
> >>Stats: 25 lines in 2 files changed: 12 ins; 10 del; 3 mod
> >>Patch: https://git.openjdk.java.net/jdk/pull/1205.diff
> >>Fetch: git fetch https://git.openjdk.java.net/jdk 
> >> pull/1205/head:pull/1205
> >>
> >> PR: https://git.openjdk.java.net/jdk/pull/1205
> >>
> >
>


-- 
- DML • he/him



Re: RFR: 8253368: TLS connection always receives close_notify exception

2020-11-13 Thread David Lloyd
How would a truncation attack be avoided in this case?

On Fri, Nov 13, 2020 at 8:23 AM Sean Coffey  wrote:
>
> removing the "closing inbound before receiving peer's close_notify" exception 
> that can be seen with TLS stack if calling close on inbound. After reading 
> the relevant parts of the TLS v1.2/v1.3 RFCs, I believe the local end point 
> doesn't have to wait for close_notify alert from remote end.
>
> -
>
> Commit messages:
>  - 8253368: TLS connection always receives close_notify exception
>
> Changes: https://git.openjdk.java.net/jdk/pull/1205/files
>  Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=1205&range=00
>   Issue: https://bugs.openjdk.java.net/browse/JDK-8253368
>   Stats: 25 lines in 2 files changed: 12 ins; 10 del; 3 mod
>   Patch: https://git.openjdk.java.net/jdk/pull/1205.diff
>   Fetch: git fetch https://git.openjdk.java.net/jdk pull/1205/head:pull/1205
>
> PR: https://git.openjdk.java.net/jdk/pull/1205
>


-- 
- DML • he/him



Re: Reading from a closed socket: different behavior between Linux and other operating systems

2020-01-03 Thread David Lloyd
This is, AFAICT, expected based on the differences between the socket
layers of the various operating systems involved and their handling of
closed sockets.  If you write a similar test program in C using OS specific
APIs, I believe you will see similar results.  I don't think this is a
problem with the JDK, nor is it likely to be something that can be fixed in
the JDK (since the error reported by the OS is, as far as I know, unlikely
to be universally sufficient to extrapolate the exact cause of failure).

On Thu, Jan 2, 2020 at 9:14 AM Sean Mullan  wrote:

> Cross-posting to security-dev as SSL is involved.
>
> --Sean
>
> On 12/29/19 4:01 PM, Dawid Weiss wrote:
> > Hello,
> >
> > I am a committer to the Apache Lucene project. We have been looking
> > into a problem in which SSL connections were handled differently in
> > tests on different operating systems and narrowed it down to
> > essentially the following scenario (full repro code at [1]):
> >
> > Server side:
> >
> >try (ServerSocketChannel serverChannel = ServerSocketChannel.open()) {
> >  SocketChannel clientChannel = serverChannel.accept();
> >  clientChannel.close();
> >}
> >
> > Client side:
> >
> >Socket socket = new Socket();
> >socket.connect(target);
> >// ... server closes the socket here.
> >// Queue some data for writing to the closed socket. This succeeds.
> >socket.getOutputStream().write("will succeed?!".getBytes("UTF-8"));
> >// Try to read something from the closed socket.
> >socket.getInputStream().read(new byte[100]);
> >
> > The last line of the client results in different behavior between
> > operating systems.
> >
> > 1) Linux, JDK 11, 13, 14: succeeds with -1 (EOF).
> > 2) Windows, JDK 11: SocketException ("recv failed") is thrown
> > 3) Windows, JDK 13, 14: SocketException (localized message) is thrown
> > 4) FreeBSD: SocketException (connection reset) is thrown
> > 5) Mac OS X: SocketException (connection reset) is thrown
> >
> > I admit my original thinking on the Lucene issue (see full  discussion
> > at [2]) was that it was Windows that was off here (due to
> > WSAECONNRESET not being handled at all in SocketInputStream.c [3].
> > Since then (JDK11) the underlying socket implementation has changed
> > due to JEP 353 [4] (which Alan Bateman kindly pointed out to me).
> >
> > But the difference in runtime behavior between Linux and other
> > operating systems still exists on both the old and the new
> > implementation. I don't know whether it's something that should be
> > qualified as platform-specific but it causes additional problems when
> > it triggers somewhere deep inside the SSL handling layer -- then the
> > application-level code receives a different exception depending on
> > where it's run (an SSLException with a suppressed SocketException or a
> > SocketException directly).
> >
> > I don't have any ideas about what a "good" fix for this is but I'm
> > curious what others think.
> >
> > Dawid
> >
> > [1]
> https://issues.apache.org/jira/secure/attachment/12989538/RecvRepro.java
> > [2] https://issues.apache.org/jira/browse/SOLR-13778
> > [3]
> https://github.com/openjdk/jdk14/blob/f58a8cbed2ba984ceeb9a1ea59f917e3f9530f1e/src/java.base/windows/native/libnet/SocketInputStream.c#L120-L154
> > [4] https://openjdk.java.net/jeps/353
> >
>
>

-- 
- DML


Re: RFR 6722928: Support SSPI as a native GSS-API provider

2019-03-23 Thread David Lloyd
On Fri, Mar 22, 2019 at 10:29 AM Nico Williams
 wrote:
>
> On Thu, Mar 21, 2019 at 10:17:36PM +0100, Michael Osipov wrote:
> > * header comment: Why do actually exclude NTLM from SPNEGO? Let SSPI work as
> > it is intended to work. Means less code you have to maintain
>
> There's a few reasons:
>
>  - NTLM doesn't have an OID, at least as I remember
>
>  - the JDK's JGSS stuff is very Kerberos-specific, especially w/ regards
>to the ServicePermission stuff
>
> IMO JAAS (and with it, *Permission) should be removed with prejudice now
> that applet support has been removed.  Perhaps stubs should be left
> behind for compatibility reasons, and all the doAs*() methods should
> just act as though permission is granted.

I assume that you mean SecurityManager and AccessController as well
(which are not a part of JAAS AFAIK)?

-- 
- DML


Re: Java SSLSocketChannel/SSLSelector?

2019-03-08 Thread David Lloyd
On Fri, Mar 8, 2019 at 4:17 PM Andi Mullaraj  wrote:
>
> Inline my comments but putting this upfront so that it doesn't go unnoticed 
> for the occasional reader:
>
> I was directed to this forum for discussing the merits and technicals details 
> of a possible SSLSelector/SSLSocketChannels. The level of engagement seems to 
> be very minimal though (I appreciate the effort of the two members pitching 
> their feedback). The best I have heard was "[..] make SSL with Selectors 
> easier, because right now it is very painful.  But this approach is unlikely 
> to succeed, at least in the JDK.", but without saying why.

The answer is that selectors are for implementing event loops.  SSL is
a thing that applies to sockets.  Combining them doesn't make sense
from any rational engineering perspective.  Having a special Selector
type for SSL channels is weird, period.  It's ugly, it smells.  It
doesn't solve the problem in a clean way.  It's certainly not the
*best* solution.  And what other things will we then need special
Selector types for?  If the answer is "nothing", then why do we need a
special Selector subclass for SSL to begin with, instead of just using
Selector?

> Every Java developer I pitch this idea to, says the same thing "Are you sure 
> that SSLSelector/SSLSocketChannels are not in JDK?"

All this says is that the Java developers you have talked to have (a)
never implemented non-blocking SSL (as evidenced by the fact that they
do not have any familiarity with the APIs) and (b) therefore do not
understand the complexities therein, or to put it more bluntly, they
have no idea what they're talking about.  I mean saying that Joe
Developer thinks some complex thing they've never looked at or
bothered to understand ought to be simple isn't really a great
argument for a lot of otherwise very busy people to drop everything to
review something that they know, based on their own expertise, isn't
really a workable solution to the problem.

> > but they chose to separate the SSL from the nio such that it was more 
> > re-usable and opted to not spend the money/time on creating an SSLSelector 
> > knowing people could program it on top.  (ie. the history).
> That decision must have been made 15 years ago.

This, for the record, has nothing to do with anything.  Some things
designed long ago are good; some are not good.  Age doesn't make ideas
go rotten.  These decisions were made for good reasons; the result may
not be perfect but it's not significantly worse now than in the 1.4
days.  The OS-level APIs that underpin Selector have not changed that
much either.

> > Most stuff should be done outside the jdk anyways.  ie. look at logback and 
> > how good it is and how the jdk logging which tried to copy log4j(and 
> > failed) kinda sucks.
> The reason I tackled this problem 2 years ago is that I could find no 
> third-party implementation of SSLSocketChannels with Selectors. I checked how 
> other applications dealt with that and felt the pain they must have felt to 
> get huge dependencies in, just to get a functionality as primordial as this.

The correct abstraction you're looking for is not a Selector, it's an
"event loop".  It is possible to have an API based on Channels that is
ready-callback driven instead of Selector-driven, and such an API can
support SSL cleanly.  This is the basis of my XNIO project.  It is
only slightly higher level than Selector; the Channels are otherwise
largely similar.

Selector is just too low-level to abstract the concept neatly (see:
countless discussions about the difficulty of implementing the
lowest-level event loop in every network daemon software written in
the past 30 years).

> This must be related to the pain threshold. I believe (and David also 
> mentioned) that working with SSLEngine is very painful. I mean, in order to 
> achieve a robust application with no edge cases especially when security 
> comes into play. This is not about making it perfect, it is about offering 
> something decent that, IMO, should have been there in first place.

What you're looking for is a better non-blocking approach to SSL, and
I too would love to see it.  If it ever comes to the JDK though, it
would have to come in the form of a JDK-level event loop abstraction,
with callback-driven channels that extend NIO.  I don't see that
happening unless it would be part of a long term plan to deprecate
NIO: the JDK folks hate needless duplication.  This seems unlikely.
But hey, maybe it's time for that idea to get its day.

-- 
- DML


Re: Java SSLSocketChannel/SSLSelector?

2019-02-18 Thread David Lloyd
On Sat, Feb 16, 2019 at 4:19 PM Andi Mullaraj  wrote:
> 2. What if someone decides to take the exact same approach to solve some 
> other higher-OSI-layer protocol decoding? Now you have to choose which kind 
> of protocol you want your selector to support.
>
> They create their own selector. I have hard time imagining one selector 
> implementation being able to support various protocols at once.

That is the case today.  I can use one selector event loop today to
support TCP (including SSL) as well as UDP, even SCTP.  The point of
selectors is to combine many things into one event loop.

> 3. Note that with a plain selector and plain sockets, you *need* a thread to 
> support the event loop anyway, I mean it has to run somewhere.
>
> I don't have enough knowledge here, but I am thinking it will depend on the 
> OS ... if the OS provides similar hooks, then I'd say no other threads are 
> needed -- the thread selecting would be entering a wait() until some notifier 
> wakes it up ... not vital to the SSL part though.

It is definitely not OS-dependent; something in the end has to call
selector.select(), and that thing is indeed a thread.

I understand the urge to make SSL with Selectors easier, because right
now it is very painful.  But this approach is unlikely to succeed, at
least in the JDK.



--
- DML


Re: Java SSLSocketChannel/SSLSelector?

2019-02-15 Thread David Lloyd
Then your selector can only be used for SSL things.  What if someone
decides to take the exact same approach to solve some other
higher-OSI-layer protocol decoding?  Now you have to choose which kind
of protocol you want your selector to support.  Note that with a plain
selector and plain sockets, you *need* a thread to support the event
loop anyway, I mean it has to run somewhere.  And with SSL, you also
need an extra thread or thread pool to handle SSL authentication
tasks, no matter whether you make a subclass of Selector.

On Fri, Feb 15, 2019 at 2:35 AM Andi Mullaraj  wrote:
>
> Hi Dean,
>
> Thanks for sharing your experience. I took a look at your links and the model 
> and implementation seems quite distant from the classic 
> Selector/SelectableChannel though. You seem to allocate a thread per each 
> Selector (which I agree is too much of a cost to pay), then fire actively 
> [listener.selectorFired()] from that thread towards a SelectorListener ...
>
> We can come back to implementation details later, but first I would like to 
> ask: any reason we cannot have an SSLSelector which extends the 
> java.nio.channels.Selector, an SSLSocketChannel extending a 
> java.nio.channels.SocketChannel, and so on? I believe we can, and in this doc 
> https://github.com/rouplex/rouplex-niossl/blob/master/README.md I have 
> provided the set of base classes that do just that. Since this is a dev group 
> I am sharing a code snippet from the doc:
>
> As an example, an existing application which is using SocketChannel with a 
> Selector for their communications, would be turned into a secure one, by 
> simply replacing:
>
> Selector selector = Selector.open();
> SocketChannel socketChannel = SocketChannel.open();
> socketChannel.register(selector, SelectionKey.OP_READ);
>
> while(true) {
> selector.select();
> for (SocketChannel sc : selector.selectedKeys() {
> sc.read(...)
> }
> }
>
> with:
>
> Selector selector = SSLSelector.open(); // notice the SSL
> SocketChannel socketChannel = SSLSocketChannel.open();  // notice the SSL
> socketChannel.register(selector, SelectionKey.OP_READ);
>
> while(true) {
> selector.select();
> for (SocketChannel sc : selector.selectedKeys() {
> sc.read(...)
> }
> }
>
> How much cooler and easier than that can it get redressing your existing 
> service using (performant) plain communications with (performant) secure 
> ones? Or create a new (and secure) one using the exact knowledge you already 
> have and never having to think SSLEngine again?
>
> As I have already mentioned I have worked on such implementation but would 
> like to first discuss with the group the merits and possible 
> problems/shortcomings of adopting the inheritance model from the plain 
> counterparts.
>
> Thanks in advance,
> --Andi
>
>
> On Wed, Feb 13, 2019 at 8:22 AM Dean Hiller  wrote:
>>
>> I would highly suggest implementing your own for a much better understanding.
>>
>> I did implement something like what you want and in so doing realized I like 
>> their decision.  ie. See the heirarchy here
>> https://github.com/deanhiller/webpieces/tree/master/core/core-channelmanager2/src/main/java/org/webpieces/nio/api/channels
>>
>> The TCPChannel could be SSL or not SSL as there are two implementations.
>>
>> If you do want an implementation that does what you want though, this 
>> library does exactly that
>> https://github.com/deanhiller/webpieces/tree/master/core/core-channelmanager2
>>
>> which is used in the webpieces webserver
>> https://github.com/deanhiller/webpieces
>>
>> That nio library is standalone even though it is in the webpieces repo.  I 
>> mean, every component in webpieces is another stand-alone piece.
>>
>> The downside is the library owns a thread which typical java libraries 
>> avoid.  ie. it has to have a thread to poll the selector and read from all 
>> the sockets to be fed to the thread pools, etc.  I think that is the main 
>> reason they decided not to have this type of library.  They prefer not to be 
>> running threads(which I agree, the jdk shouldn't).
>>
>> later,
>> Dean
>>
>>
>>
>>
>> On Tue, Feb 12, 2019 at 7:54 PM Sean Mullan  wrote:
>>>
>>> Hi Andi,
>>>
>>> TLS/JSSE topics are best discussed on the security-dev alias, so I am
>>> copying that list for more discussion and -bcc-ing core-libs-dev.
>>>
>>> --Sean
>>>
>>> On 2/11/19 3:29 PM, Andi Mullaraj wrote:
>>> > Hi java-core community,
>>> >
>>> > I have been directed to this channel to discuss matters related to Java
>>> > performant ssl communications.  Maybe this is a topic that has been
>>> > discussed in the past, in which case I would appreciate if someone pointed
>>> > me to that particular topic.
>>> >
>>> > Back in 2002 I personally applauded the java.nio.channels (jdk1.4) 
>>> > package,
>>> > realizing that there was no way to port this to the secure communications,
>>> > due to lack of a comparable implementation for SSL.  But I thought it was
>>> > going to just be matter 

Re: Fwd: Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

2018-10-04 Thread David Lloyd
On Wed, Oct 3, 2018 at 7:53 PM Sergey Bylokhov
 wrote:
> Hi, Sean.
> One question related to SecurityManager and performance, is it possible
> to provide a special version of AccessController.doPrivileged which will
> be noop if SecurityManager is not present?

TBH that method (at least, the no-arg variant) should *always* have
been a no-op.  All it really accomplishes, in practice, is to place a
mark on the stack where the stack crawl to build the access control
context should stop (plus one frame), and this effect is something
that is already a natural consequence of a method being called in the
JVM.

The doPrivileged variant that accepts an ACC parameter should likewise
always have been no-op other than stashing the nested ACC into some
sort of thread-local (or better, a field on Thread) which can be
referred to by the aforementioned stack crawl.

The pure-java AccessController I prototyped late last year relies on
these ideas, among other things.
-- 
- DML


Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

2018-09-17 Thread David Lloyd
On Sun, Sep 16, 2018 at 2:38 PM Will Sargent  wrote:
>
> > The security manager is legacy these days and I think we need to figure out 
> > a plan how to deprecate and eventually bury it.
>
> I don't know of any research or papers that explicitly say that 
> SecurityManager is "legacy".  I did some research into this a while ago, and 
> while SecurityManager has some major flaws, I don't know of any other way to 
> sandbox a Java application.

Another interesting point is that, when it comes to the actual work of
sandboxing, all of the difficult work is done by AccessController and
AccessControlContext.  However it's difficult to say that these should
be legacy without addressing the problem of JAAS, which also uses
these constructs to pass and check the current Subject.  This aspect
of JAAS seems to be known to be inherently defective by several
(perhaps most) application server vendors, as those which use JAAS
also tend to have a custom, thread-local-based solution for handling
Subjects which is used in place of the ACC-attachment mechanism.

With that in mind it's quite difficult to imagine a path that starts
with deprecating SecurityManager and ends with its removal, unless
JAAS is addressed.  And if JAAS is successfully decoupled from
AccessController, then maybe SecurityManager becomes a somewhat more
viable technology again: based on some research I did a while back, it
should be possible to reimplement it compatibly (and purely in Java)
with less overhead and with more convenient constructs (once JAAS is
out of the way).

That said, the main security-oriented use of SecurityManager isn't
around sandboxing fully untrusted code (since it cannot sandbox memory
allocations or CPU usage), it's more about ensuring that your trusted
code isn't inadvertently exploited, and (as Alan said) also
occasionally about fencing in badly-behaved third-party code that (for
whatever reason) cannot be modified.  Though we've had AOP and similar
technologies for quite a long time, as well as plenty of other
frameworks that allow build-time and run-time bytecode rewriting, and
it's not hard to argue that this is a better solution to this class of
problem.

Anyway I think that a better first move would have been to instead
deal with the JAAS issue, one way or another.  If this cannot be done,
then the access controller is here to stay, and the security manager
issue is moot.
-- 
- DML


Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

2018-09-14 Thread David Lloyd
On Fri, Sep 14, 2018 at 8:22 AM Alan Bateman  wrote:
>
> On 14/09/2018 13:46, David Lloyd wrote:
> > :
> >> There are essentially two main parts to this change:
> >>
> >> 1. Deprecation of System.s[etS]ecurityManager()
> > We (JBoss) use this method to configure some things at run time (like
> > customizing the Policy) before installing our security manager, so
> > this would be not so great for us.
> The security manager is legacy these days and I think we need to figure
> out a plan how to deprecate and eventually bury it. I have no idea how
> many releases or years that might take but the proposal here is a
> profitable first step. It's easy to imagine follow on steps where the
> default changes to not support the security manager without some opt-in.
> Yes, this will be disruptive for a number of usages but there is lots of
> time to look for solutions to the issues that people are using the
> security manager for today - for example we've seen several cases where
> people set a security manager because they lack hooks to prevent plugins
> from doing things (plugins or tests calling System.exit comes up
> periodically for example).

I can say that this explanation would be more palatable by far if the
security manager as a whole could be deprecated all at once.  I for
one would certainly be happy to remove support for it in the software
that I maintain (it's a considerable amount of code and other
gymnastics to be sure).  However, I'm not sure that our users and
customers will be so easily convinced.  My understanding is that there
are plenty of corporate and perhaps government security standards that
dictate security manager usage.

If the security manager itself is not yet to be deprecated, then I
have a much harder time accepting this argument.  It's not really a
stepping stone in any practical sense, at least not from our
perspective, unless that step is "break JBoss first, and then break
everyone else later"; to be fair though I'm approximately 200% more
cynical in the morning. :)

-- 
- DML


Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

2018-09-14 Thread David Lloyd
On Thu, Sep 13, 2018 at 3:03 PM Sean Mullan  wrote:
>
> This is a SecurityManager related change which warrants some additional
> details for its motivation.
>
> The current System.setSecurityManager() API allows a SecurityManager to
> be set at run-time. However, because of this mutability, it incurs a
> performance overhead even for applications that never call it and do not
> enable a SecurityManager dynamically, which is probably the majority of
> applications.

What kind of performance overhead?

> For example, there are lots of "SecurityManager sm =
> System.getSecurityManager(); if (sm != null) ..." checks in the JDK. If
> it was known that a SecurityManager could never be set at run-time,
> these checks could be optimized using constant-folding.

I think they could be optimized using constant-folding either way, if
something like SwitchPoint were used instead of a plain field; am I
incorrect in my understanding?  The reason I ask is... (see below)

> There are essentially two main parts to this change:
>
> 1. Deprecation of System.s[etS]ecurityManager()

We (JBoss) use this method to configure some things at run time (like
customizing the Policy) before installing our security manager, so
this would be not so great for us.

-- 
- DML


Re: SSLEngine weird behavior in 11+21?

2018-07-12 Thread David Lloyd
On Thu, Jul 12, 2018 at 2:30 PM Xuelei Fan  wrote:
> On 7/12/2018 10:47 AM, Simone Bordet wrote:
> > On Thu, Jul 12, 2018 at 4:34 PM Xuelei Fan  wrote:
[...]
> >> In TLS 1.3, half-close policy is used.  The server may not response with
> >> the close_notify for client close_notify request.  See the "Closure
> >> Alerts" section for more details:
> >>  https://tools.ietf.org/html/draft-ietf-tls-tls13-28#section-6.1
> >
> > In that section I read:
> > "Each party MUST send a "close_notify" alert before closing its write
> > side of the connection, unless it has already sent some error alert."
> >
> > Given that, I expect that for close_notify, at step #2, the client
> > goes into NEED_UNWRAP, as the server MUST send a close_notify too.
[...]
> Per the TLS 1.3 specification, the local can request to close its
> writing side, but it cannot to request to close the peer writing side.
> It means it is not defined about how to close the read side.  It could
> lead to issues in practice.  I had a question in the IETF TLS email
> list.  But unfortunately, I did not get a ideal solution:
>  https://www.ietf.org/mail-archive/web/tls/current/msg25581.html
>
> In order to countermeasure the issues, JDK will try to close the
> underlying socket if either side sends the close_notify message.

Please forgive my interjection - does this not explicitly violate the
half-close policy?  The socket should not be closed until close_notify
is both received (which means that read-shutdown is allowed) and sent
(after which write-shutdown may be performed), should it not?

> > At this point, my gut feeling is that having a single codebase
> > handling TLS 1.2 and TLS 1.3 would be difficult.

Is this feeling due to the half-closed situation?  Because it's my
firm belief that an TLS 1.2 implementation which treats half-closed in
the manner specified by TLS 1.3 cannot be shown (from the perspective
of an outside observer) to be in violation of the specification, and
furthermore, _not_ doing so may represent a theoretical existent
security risk.

Put another way, imagine that my endpoint receives data and then sends
data.  Now imagine that the receive side immediately receives
close_notify after its data is processed but before the send
(response) begins.  This obviously will terminate the connection
before the data is output.

Now imagine that the close_notify is completely contained in a single
TCP packet, and that packet is (for whatever reason) delayed on the
network until my send is complete.  The OpenJDK TLS stack closes the
socket at the right time (from my perspective), after both receive and
send completed correctly. This delay has caused a material behavior
change of my application, which is not a desirable effect.  Two
different behaviors, with the only input difference being one of
timing.

Imagine again now that the TLS 1.2 stack would not immediately send a
close_notify, but instead would follow the half-close model as
specified in TLS 1.3.  As far as the *peer* is aware, my endpoint is
conformant, because the peer cannot possibly differentiate between the
TLS stack deferring shutdown to the application versus the
close_notify message being delayed on the network.  And, a bad actor
can no longer affect the behavior of the application by delaying
packets.

These reasons were, without a doubt, considerations when the rules
were changed for TLS 1.3.  Also another important consideration was
probably the observation that OpenSSL *already* appears to behave in
this manner.  OpenJDK's behavior has always IMO been clearly incorrect
in this regard, despite the wording of the specification, and seeing
this belief borne out by the changes in the TLS 1.3 spec is somewhat
vindicating in this regard.  But more importantly, I think this
represents a chance to fix this long-standing bad behavior of the JDK.

-- 
- DML


Re: JEP 332: Transport Layer Security (TLS) 1.3

2018-03-30 Thread David Lloyd
Is it possible that this could make Java 11, or is that a long shot?

On Fri, Mar 30, 2018 at 11:36 AM,   wrote:
> New JEP Candidate: http://openjdk.java.net/jeps/332
>
> - Mark



-- 
- DML


Re: Code Review Request: TLS 1.3 full handshake (JDK-8196584)

2018-02-22 Thread David Lloyd
I have an update. :)

I've spoken with everyone I could find who deals with user
configurations which may include these cipher suites, and they all
agree that users are no longer using TLS KRB5 and that it is no longer
recommended either.  So, from our perspective, it would be OK to drop
these after all.

On Wed, Feb 21, 2018 at 9:32 AM, Xuelei Fan  wrote:
> Thanks for the feed back.  It's good enough to get KRB5 cipher suites back.
>
> Regards,
> Xuelei
>
>
> On 2/21/2018 5:06 AM, David Lloyd wrote:
>>
>> On Tue, Feb 20, 2018 at 1:57 PM, Xuelei Fan  wrote:
>>>
>>> In this implementation, I removed:
>>> 1. the KRB5 cipher suite implementation.
>>> Please let me know if you are still using KRB5 cipher suite.  I may not
>>> add
>>> them back if no objections.
>>
>>
>> I am given to understand that we have multiple users using the KRB5
>> cipher suite.
>>
>



-- 
- DML


Re: Code Review Request: TLS 1.3 full handshake (JDK-8196584)

2018-02-21 Thread David Lloyd
On Tue, Feb 20, 2018 at 1:57 PM, Xuelei Fan  wrote:
> In this implementation, I removed:
> 1. the KRB5 cipher suite implementation.
> Please let me know if you are still using KRB5 cipher suite.  I may not add
> them back if no objections.

I am given to understand that we have multiple users using the KRB5
cipher suite.

-- 
- DML


Exploring an alternative AccessController implementation

2017-12-11 Thread David Lloyd
This past weekend I took some time to try out an idea I've been
kicking around for a couple of weeks: a pure-Java (no native)
AccessController implementation [1].

•Rationale•

I have a number of reasons for attempting this enhancement.

 • I think it would be beneficial (security-wise) if access control
was purely Java, because the set of users who can understand, enhance,
and troubleshoot the implementation would be expanded from those with
a relatively specific intersection of knowledge (hotspot, C++, Java,
security) to a significantly larger set of people.
 • I suspect that there are multiple performance gains to be had by
eliminating the jump to JNI and dropping the array-centric current
design.
 • It is interesting!

•Goals•

The things I primarily wish to accomplish were:

 • Test the hypotheses that a pure Java AccessController is even possible
 • Eliminate, to the maximum extent possible, the cost of plain doPrivileged()
 • Reduce the cost of getContext() and other doPrivileged() methods

Secondary goals:

 • Discover the limitations of Java-side access checking
 • Explore some optimization ideas
 • Have fun!

•Principal concepts•

Of main interest is that AccessControlContext is now an immutable
linked list with each instance containing a ProtectionDomain and a
"next" pointer.  Any given AccessControlContext permits the
intersection of its ProtectionDomain and whatever the next
AccessControlContext permits.

All AccessControlContext instances are ultimately linked to a
singleton ROOT_CONTEXT (or in certain less-likely circumstances, a
clone thereof).  Any two given AccessControlContext instances may
share an arbitrary number of tail nodes.  Any given ProtectionDomain
will appear in an ACC list only once (by equals/hashCode contract).
The means that, assuming well-written ClassLoaders, the depth of the
ACC stack will usually be shallow compared to the depth of the call
stack, as the ACC stack will usually maximally contain a number of
entries equal to the total number of JARs or modules present in a
given application.

Because ACC is a linked list, using a DomainCombiner will likely
impose a significant penalty (because it operates on
ProtectionDomain[], thus it entials the unwinding of the linked list
to an array and back again), though there's nothing in the design here
that would prohibit them from actually working.  There might be some
optimization(s) possible that I didn't think of on my first pass
through the code.  Nevertheless, I marked DomainCombiner and its use
sites as @Deprecated in this implementation, because it violates the
principle of only narrowing access (as well as, in my opinion, any
other sane security principle, given that a DC can arbitrarily muck up
an ACC before any access check).  Coming up with an alternative for
Subject association (and similar use cases) is something that could be
discussed separately.

Methods were added to ACC to get new ACC instances that consist of the
intersection of the existing ACC and additional protection domains
(see AccessControlContext#with(*)).  Because these instance methods
always yield a less-privileged ACC, I determined that these methods do
not need additional permission checks.  Calling with() with a
protection domain that is null or is already included in the ACC will
return the same ACC instance.

Making ACC act like a stack has a similar algorithmic complexity to
the old array-based code in many cases, though it's possible that in a
few cases, N×M iteration was able to be optimized away.  There is a
potential disadvantage in that linked lists can have worse cache
locality compared to arrays.

However, using a linked list/stack also allows for a potentially
interesting optimization: the caching of ACC instances into the
StackFrame of calling classes.  I made a placeholder API method (for
illustrative purposes) in JavaLangAccess which is meant to allow
retrieval and update of an AccessControlContext field on a StackFrame.
This would allow (for example) consecutive or nearby calls to
AccessController.getContext() to share instances.  Of course this
would require a potentially complex modification to the JVM itself to
reserve space for an extra reference field on every stack frame, and
it is not certain that there would be a net benefit, at least not
unless it could be tested.

If a cached AccessControllerContext instance is found on a stack
frame, then it would also allow checkPermission() to be optimized as
well: if a frame is encountered with a cached ACC, then traversal can
end and the checkPermission() method on ACC can be used, which is sure
to be optimized as the ACC stack will contain no duplicate items.

•Implementation•

So far, my first rough implementation (which can be found at [1])
consists of 822 additions and 1709 deletions, so that's what I could
call a move in the right direction (especially for security-related
code).  It seems to "work", in that regular applications seem to
function.  It is not thoroughly tested y

Re: Eliminating the security overhead when not running with a security manager

2017-11-21 Thread David Lloyd
On Tue, Nov 21, 2017 at 5:41 AM, Alan Bateman  wrote:
> On 21/11/2017 00:48, David Lloyd wrote:
>>
>> One thing that springs to mind.  Some allowance would have to be made
>> for domain combiners and JAAS Subject propagation: this mechanism also
>> uses access control contexts, to its own great detriment.
>
> Are you thinking about usages where there is no security manager but
> AccessController.checkPermission is still used to check permissions?

Not specifically; I'm thinking more of the general problem of Subject
association. Calling Subject#doAs*() has a heavy cost, as does
Subject#getSubject().  I believe that most container vendors who use
JAAS for authentication therefore provide an alternative association
API using thread-locals.

> In terms of performance the main interest here is the "no security manager"
> case. If you have prototypes that moving the stack walking and help the
> security manager case then I expect the folks here will be interested.

I think that the main benefit of this suggested approach is in fact
that it should reduce the cost of the "no security manager" case.
Maybe if I get some time over the upcoming holidays I'll give it a
spin and see how it goes.


-- 
- DML


Re: Eliminating the security overhead when not running with a security manager

2017-11-20 Thread David Lloyd
One thing that springs to mind.  Some allowance would have to be made
for domain combiners and JAAS Subject propagation: this mechanism also
uses access control contexts, to its own great detriment.  I would say
that the JAAS domain combiner strategy should be dropped in favor of a
simple thread local holding the Subject, regardless of what else is
decided.

I do personally believe that using a security manager as some kind of
proof against untrusted Java code is a mistake.  However I think one
important use case of security managers is where the Java code being
run actually _is_ trusted, but you want to avoid "weird machine"
effects where server code could be exploited (by way of malformed
requests or whatever) to perform actions that are unexpected but also
technically allowed (for example, if your file management abstraction
is only ever intended to modify "/app/user-storage/-" then you don't
want it to be able to access "/app-server/secret-passwords", and if
that happens, you maybe want to start yelling a lot).  In this case
it's an extra layer of protection.  I don't see this use case going
away or being invalidated in any way; an application server request
_is_ code, in a way, that must be interpreted and "executed" after a
fashion in order to do something useful for the user.

In terms of approach: back when the discussion around StackWalker
design was still going on, it seemed to me that the performance costs
of the security manager could be mostly shifted to the security
manager itself (thus solving the problem in a different way), if it
would walk the stack to do access checking directly, like this:

• An "acc" field on Thread is used to hold the inherited access control context
• AccessController.doPrivileged(task) becomes empty other than to
clear the acc field and delegate to the called block (and restore the
acc field after)
• AccessController.doPrivileged(task, acc) becomes empty other than to
set the acc field to the given ACC argument and delegate to the called
block (and restore the acc field after)
• AccessController.checkPermission() iterates the call stack until it
hits one-frame-lower-than-doPrivileged or the bottom of the stack,
checking access along the way for each protection domain (with some
simple duplicate elimination as it walks), and once it hits the end,
it calls checkPermission on the inherited acc field in Thread (if
any).
• Creating a new thread still captures the caller acc but then copies
it into the new thread's inherited acc field
• Construction/compilation of the ACC could possibly be moved to
userspace, which will reduce any memory overhead of having it baked in
to the native JVM code when it's not actually used
• The doPrivileged(action, acc, perms) case could be handled with
another Thread field which could be checked before or after the
primary check (in this case all the doPrivileged methods would have to
stash and set this field as well, and restore it after)
• A "shared secret" is added to let AccessController access the two
Thread cache field(s)

This way the cost of doPrivileged is greatly minimized (two Thread
field reads and writes before the block, and two Thread field writes
after the block, assuming the aforementioned second field for array
permissions) without affecting security manager functionality should
it happen to be enabled in software.  Also it's easy to improve the
security manager debug output, such that you could get a report on
_all_ classes or modules or codeSources or whatever which would do not
grant a given permission (instead of just the first one), which is
greatly useful in reducing iterations in the design and debug stages.

When the security manager is actually enabled, only testing would
reveal the comparative performance of a stack walker based approach
versus the current native ACC optimization code (maybe C2 can work
some magic here).  There's probably some kind of disgusting weirdness
around method handles and reflection things on the call stack that
would have to be examined as well.  Finally, it will (as always) be a
challenge to craft a Stream-consuming Function that doesn't allocate
scads of objects when actually iterating the thread stack.  Maybe we
could allow an Iterator-based StackWalker method.  Maybe I'll keep
dreaming.

Anyway I never got a chance to prototype this, but it might be a fun
option worth exploring.  I found the idea of moving this stuff all to
user space to be very appealing (due in no small part to the idea that
it could potentially be examined and analyzed by a much larger
audience, being Java code). It also hints at the possibility of a
fully "user space" replacement of the security manager concept (much
of the remaining cost lives in the structure of AccessControlContext,
which is based on an array of ProtectionDomain objects; this is
definitely non-ideal and could possibly be hidden behind a smarter
abstraction).

I really firmly believe that domain combiners should be ultimately
eliminated regardl

Re: Code Review Request: JDK-6491070 (Support for RFC 5929-Channel Bindings)

2017-09-01 Thread David Lloyd
I can say that from the perspective of SASL that we don't need any
special indication about handshake, and it would be much easier to
just read the material off of the engine, socket, or session as
appropriate.

On Thu, Aug 31, 2017 at 8:32 PM, Xuelei Fan  wrote:
> The failure-and-retry mechanism could a nightmare for some applications.
> Please think more how could we avoid it.  If need more APIs, what the update
> may looks like and how complicated it could be?
>
> If required, Bernd's proposal can be extended a little bit to support
> operations other than listening.
>
> APIs maintain is very complicated, a good design may need more time
> currently, but could save much more in the future.
>
> Thanks,
> Xuelei
>
> On 8/31/2017 11:41 AM, Martin Balao wrote:
>>
>> Hi,
>>
>> The material is already cached in the handshaker if secure renegotiation
>> is enabled. However, I agree with you that we are going to cache the value
>> even when secure renegotiation is not enabled, thus, wasting roughly 12
>> bytes (as bytes for an empty array are already consumed). In fact, the exact
>> case -adding a few conditionals to webrev.02- is the one in which secure
>> renegotiation is disabled and extended master secret is enabled. GnuTls and
>> OpenSSL -including its derivatives like Boring SSL and Python (through
>> OpenSSL)- always cache this information.
>>
>> As for the empty / null cases, the API consumer was expected to ask for
>> the binding information after the TLS connection is established. It's on the
>> API consumer knowledge that asking for the information before (i.e.: just
>> after creating a disconnected socket) or while the handshake is taking
>> place, makes no sense and no valid value will be obtained (either we define
>> this as null or empty). For those providers that do not support this
>> feature, the information wouldn't have been available after the handshake.
>> However, I agree with you that before the handshake is completed there is no
>> means of knowing if the provider does support this API. My first webrev
>> (webrev.01) was throwing an UnsupportedOperationException to make this case
>> explicit but I had doubts regarding the real value it provides for the API
>> consumer. The proposed API was similar to Python, SSLBoring and GnuTLs.
>> However, handshake listener callbacks -as Bernd suggests- and the idea of
>> just exposing the handshake material (as a lower level API) sounds good to
>> me. I can give it a try. I propose then to bring the handshake information
>> as part of a HandshakeCompletedEvent instance, even though the callback
>> registrant may not consume it. Would that work for you?
>>
>> In regard to the handshake material update -which I assumed unlikely-, the
>> point in which a renegotiation may take place (from the server side) is when
>> reading data, not when writing. That cannot be controlled by the application
>> because it's JSSE internal and not exposed. Thus, an application may read
>> from the socket, get the handshake material and write a message using the
>> binding value -which we can make sure that is the valid one at that point-.
>> However, as soon as the application reads again from the socket, a
>> renegotiation -if requested by the client- may be processed and the binding
>> value gets updated. The higher level protocol may fail -because the binding
>> value was already sent but not processed on the other side- and a re-try
>> needed. This looks independent of whether we use the originally proposed API
>> or the handshake listener callback interface (or even a sync callback),
>> because the underlying problem is that the application cannot really control
>> the renegotiation flow in the lower layer (as RFC 5929 suggest). The options
>> I see are adding more complexity to the API and let the application control
>> the renegotiation flow or live with that and expect the application to
>> retry.
>>
>> Thanks,
>> Martin.-
>>
>> On Tue, Aug 29, 2017 at 4:34 PM, Xuelei Fan > > wrote:
>>
>> On 8/26/2017 2:56 PM, Bernd Eckenfels wrote:
>> > How about only passing it to an extended handshake listener. The
>> > material does not have to be cached (the app can do it if needed)
>> and
>> > renegotiation works the same way. This can also be helpful for
>> things
>> > like logging the master secret (for wireshark decryption). It can
>> even
>> > handle auditing of session resumptions
>> Martin, what do you think about Bernd's proposal above and similar
>> callback mechanism?
>>
>> More comment inlines.
>>
>> On 8/29/2017 11:50 AM, Martin Balao wrote:
>>
>> Hi Xuelei,
>>
>>   >There are a few protocols that can benefits from exporting
>> SSL/TLS handshake materials, including RFC 5929, RFC 5056, token
>> binding and TLS 1.3 itself.  Can we define a general API so as
>> to exposing the handshake materials, so as to mitigate the
>> inflating of JSSE APIs?  I