Re: RFR: 8212136: Remove BaseSSLSocketImpl finalizer method [v2]

2022-04-08 Thread Bradford Wetmore
On Fri, 8 Apr 2022 06:52:57 GMT, Xue-Lei Andrew Fan  wrote:

> The Socket implementation will take care of the file description/native 
> memory release, I think.

I've walked through the network code and understand it now.  The Socket 
creation code calls into sun.nio.ch.NioSocketImpl.create():451, which then 
allocates the FileDescriptor fd and assigns it to the Socket, then registers a 
closer for that FileDescriptor which will be triggered by the Socket GC.  When 
the Socket is reclaimed, the FileDescriptor is released, but not by referencing 
the Socket itself.  

> It is expected to send the close_notify at the TLS layer. But the current 
> code using finalizer, which is not reliable. The underlying socket may have 
> been closed when the SSLSocket finalizing action is triggered. Generally, 
> application should call close() method explicitly, otherwise the finalizer is 
> not expect to work reliable. I was wondering it may be safe to remove the 
> finalizer.

Yeah, I'm just not sure yet.  

> I agree that adding a best effort cleanup may be better. I was wondering to 
> check if it is possible to clean the socket in the socket creation factories 
> so that the object reference issues could be resolved. But as socket is a 
> kind of resource, application layer may make the clean up as well.

> I'm still looking for a solution to clean up resource by using a method of 
> 'this'. Please advice if anyone has experiences.

@AlanBateman, @dfuch, any great ideas here?

-

PR: https://git.openjdk.java.net/jdk/pull/8065


Re: RFR: 8284105: Update security libraries to use sealed classes [v2]

2022-04-08 Thread Weijun Wang
On Fri, 8 Apr 2022 20:07:32 GMT, Sean Mullan  wrote:

>> Please review these changes to update the security libraries to use sealed 
>> classes. See JEP 409 for more details on sealed classes.
>> 
>> No CSR is required as all the changes are to internal classes. A few classes 
>> that did not have subclasses were simply marked final instead of sealed.
>
> Sean Mullan has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Make some classes package-private instead of sealed.

Only one comment. Others look fine.

src/java.security.jgss/share/classes/sun/security/krb5/KrbTgsRep.java line 43:

> 41:  * Kerberos client.
> 42:  */
> 43: final class KrbTgsRep extends KrbKdcRep {

Make `KrbAsRep` final also to be symmetric.

-

Marked as reviewed by weijun (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8165


Re: RFR: 8284105: Update security libraries to use sealed classes [v2]

2022-04-08 Thread Sean Mullan
> Please review these changes to update the security libraries to use sealed 
> classes. See JEP 409 for more details on sealed classes.
> 
> No CSR is required as all the changes are to internal classes. A few classes 
> that did not have subclasses were simply marked final instead of sealed.

Sean Mullan has updated the pull request incrementally with one additional 
commit since the last revision:

  Make some classes package-private instead of sealed.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8165/files
  - new: https://git.openjdk.java.net/jdk/pull/8165/files/52aa0ad6..c27b1ba0

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8165=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8165=00-01

  Stats: 93 lines in 8 files changed: 42 ins; 38 del; 13 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8165.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8165/head:pull/8165

PR: https://git.openjdk.java.net/jdk/pull/8165


Re: RFR: 8284105: Update security libraries to use sealed classes [v2]

2022-04-08 Thread Sean Mullan
On Fri, 8 Apr 2022 16:11:27 GMT, Weijun Wang  wrote:

>> Sean Mullan has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Make some classes package-private instead of sealed.
>
> src/java.base/share/classes/sun/security/rsa/RSASignature.java line 50:
> 
>> 48:  * @author  Andreas Sterbenz
>> 49:  */
>> 50: public abstract class RSASignature extends SignatureSpi {
> 
> We can probably move the `RSASignature.encodeSignature` method to `RSAUtil` 
> and this class can be package private.

Ok. I'll also move `RSASignature.decodeSignature` to `RSAUtil` to maintain 
symmetry even though it isn't called outside the package.

> src/java.base/share/classes/sun/security/util/math/intpoly/IntegerPolynomial.java
>  line 64:
> 
>> 62:  */
>> 63: 
>> 64: public abstract class IntegerPolynomial implements IntegerFieldModuloP {
> 
> Although we only have several implementations, I think this class is meant to 
> be freely extendable for whatever new modulus.

We can always add new ones to the permits clause later.

-

PR: https://git.openjdk.java.net/jdk/pull/8165


Re: RFR: 8284105: Update security libraries to use sealed classes

2022-04-08 Thread Sean Mullan
On Fri, 8 Apr 2022 16:17:19 GMT, Weijun Wang  wrote:

> It looks `KrbTgsRep.java`, `Krb5ProxyCredential.java`, `Builder.java`, 
> `Vertex.java`, `Validator.java`, and `RSAKeyPairGenerator.java` can all be 
> made package private.

Good point, although I would prefer to leave `Validator` as public for now 
until we are more sure of the compatibility risk as there have been external 
dependencies on it.

It may be useful to apply the `sealed` modifier to package-private classes, but 
for this RFE that is not the goal, so I will remove the `sealed` modifier from 
these classes (if applicable) when I make them package-private.

-

PR: https://git.openjdk.java.net/jdk/pull/8165


Re: RFR: 8284490: Remove finalizer method in java.security.jgss [v3]

2022-04-08 Thread Brent Christian
On Fri, 8 Apr 2022 13:50:25 GMT, Weijun Wang  wrote:

>> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Update copyright year
>
> @bchristi-git showed me this test. You might try it on other objects:
> 
> import org.ietf.jgss.GSSManager;
> import org.ietf.jgss.GSSName;
> 
> import java.util.WeakHashMap;
> 
> public class A1 {
> private static WeakHashMap whm = new WeakHashMap();
> public static void main(String[] args) throws Exception {
> System.setProperty("sun.security.nativegss.debug", "true");
> System.setProperty("sun.security.jgss.native", "true");
> GSSName e = GSSManager.getInstance().createName("u1", 
> GSSName.NT_USER_NAME);
> whm.put(e, null);
> e = null;
> for (int i = 0; i < 10; i++) {
> System.gc();
> Thread.sleep(100);
> }
> if (whm.size() > 0) {
> throw new RuntimeException("GSSName still reachable");
> }
> }
> }
> 
> The test ensures the objects are GCed and there's no memory leak. You need to 
> inspect the debug output to make sure native resources are released. The 
> `NativeGSSContext` code still needs to be fixed.

Thanks, @wangweij . Wherever practical, it would be good to include tests to 
confirm correct conversions from finalizer to Cleaner -- bugs can be subtle, 
and hard to spot.

-

PR: https://git.openjdk.java.net/jdk/pull/8136


Re: RFR: 8284105: Update security libraries to use sealed classes

2022-04-08 Thread Xue-Lei Andrew Fan
On Fri, 8 Apr 2022 13:40:37 GMT, Sean Mullan  wrote:

> Please review these changes to update the security libraries to use sealed 
> classes. See JEP 409 for more details on sealed classes.
> 
> No CSR is required as all the changes are to internal classes. A few classes 
> that did not have subclasses were simply marked final instead of sealed.

It looks safe to me as if compiling and test passed.

-

Marked as reviewed by xuelei (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/8165


Re: RFR: 8284105: Update security libraries to use sealed classes

2022-04-08 Thread Weijun Wang
On Fri, 8 Apr 2022 13:40:37 GMT, Sean Mullan  wrote:

> Please review these changes to update the security libraries to use sealed 
> classes. See JEP 409 for more details on sealed classes.
> 
> No CSR is required as all the changes are to internal classes. A few classes 
> that did not have subclasses were simply marked final instead of sealed.

It looks `KrbTgsRep.java`, `Krb5ProxyCredential.java`, `Builder.java`, 
`Vertex.java`, `Validator.java`, and `RSAKeyPairGenerator.java` can all be made 
package private.

src/java.base/share/classes/sun/security/rsa/RSASignature.java line 50:

> 48:  * @author  Andreas Sterbenz
> 49:  */
> 50: public abstract class RSASignature extends SignatureSpi {

We can probably move the `RSASignature.encodeSignature` method to `RSAUtil` and 
this class can be package private.

src/java.base/share/classes/sun/security/util/math/intpoly/IntegerPolynomial.java
 line 64:

> 62:  */
> 63: 
> 64: public abstract class IntegerPolynomial implements IntegerFieldModuloP {

Although we only have several implementations, I think this class is meant to 
be freely extendable for whatever new modulus.

-

PR: https://git.openjdk.java.net/jdk/pull/8165


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: RFR: 8284105: Update security libraries to use sealed classes

2022-04-08 Thread Joe Darcy
On Fri, 8 Apr 2022 13:40:37 GMT, Sean Mullan  wrote:

> Please review these changes to update the security libraries to use sealed 
> classes. See JEP 409 for more details on sealed classes.
> 
> No CSR is required as all the changes are to internal classes. A few classes 
> that did not have subclasses were simply marked final instead of sealed.

Marked as reviewed by darcy (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/8165


Integrated: 8283698: Refactor Locale constructors used in src/test

2022-04-08 Thread Naoto Sato
On Wed, 6 Apr 2022 17:45:13 GMT, Naoto Sato  wrote:

> This is a follow-on task after deprecating the Locale constructors 
> (https://bugs.openjdk.java.net/browse/JDK-8282819). Most of the changes are 
> simple replacements to Locale constructors with `Locale.of()` or Locale 
> constants, such as `Locale.US`.

This pull request has now been integrated.

Changeset: d6b4693c
Author:Naoto Sato 
URL:   
https://git.openjdk.java.net/jdk/commit/d6b4693c0527385f8999089b3f8b2120548efecb
Stats: 750 lines in 182 files changed: 3 ins; 3 del; 744 mod

8283698: Refactor Locale constructors used in src/test

Reviewed-by: iris, joehw

-

PR: https://git.openjdk.java.net/jdk/pull/8130


Re: A possible JEP to replace SecurityManager after JEP 411

2022-04-08 Thread Sean Mullan

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?


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.


> 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.

--Sean

On 4/8/22 9: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.
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.


Re: RFR: 8284490: Remove finalizer method in java.security.jgss [v3]

2022-04-08 Thread Weijun Wang
On Thu, 7 Apr 2022 19:21:31 GMT, Xue-Lei Andrew Fan  wrote:

>> Please review the update to remove finalizer method in the 
>> java.security.jgss module. It is one of the efforts to clean up the use of 
>> finalizer method in JDK.
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update copyright year

@bchristi-git showed me this test. You might try it on other objects:

import org.ietf.jgss.GSSManager;
import org.ietf.jgss.GSSName;

import java.util.WeakHashMap;

public class A1 {
private static WeakHashMap whm = new WeakHashMap();
public static void main(String[] args) throws Exception {
System.setProperty("sun.security.nativegss.debug", "true");
System.setProperty("sun.security.jgss.native", "true");
GSSName e = GSSManager.getInstance().createName("u1", 
GSSName.NT_USER_NAME);
whm.put(e, null);
e = null;
for (int i = 0; i < 10; i++) {
System.gc();
Thread.sleep(100);
}
if (whm.size() > 0) {
throw new RuntimeException("GSSElementName still reachable");
}
}
}

-

PR: https://git.openjdk.java.net/jdk/pull/8136


RFR: 8284105: Update security libraries to use sealed classes

2022-04-08 Thread Sean Mullan
Please review these changes to update the security libraries to use sealed 
classes. See JEP 409 for more details on sealed classes.

No CSR is required as all the changes are to internal classes. A few classes 
that did not have subclasses were simply marked final instead of sealed.

-

Commit messages:
 - Update security libraries to use sealed classes.

Changes: https://git.openjdk.java.net/jdk/pull/8165/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8165=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8284105
  Stats: 50 lines in 20 files changed: 8 ins; 0 del; 42 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8165.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8165/head:pull/8165

PR: https://git.openjdk.java.net/jdk/pull/8165


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-08 Thread Peter Firmstone

Thanks for trying David. :)

Regards,

Peter.

On 8/04/2022 7:15 pm, Andrew Dinn wrote:
I'm 100% in agreement with Sean. This proposal leaves the OpenJDK team 
with just as much -- or possibly more -- code to maintain, test and 
design around while making the behaviour under the retained API less 
determinate, less reliable as a security enforcement mechanism and, in 
consequence, even less likely to be used than it is already.


regards,


Andrew Dinn
---
Red Hat Distinguished Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill

On 07/04/2022 20:19, Sean Mullan wrote:

Hi David,

Thanks for the feedback and spending some time on this proposal. Some 
specific comments below.


On 4/5/22 9:52 AM, David Lloyd wrote:

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.


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). 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). Any missing checks would need to be treated 
as security issues. 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.


The proposal also includes retaining calls to doPrivileged (but later 
potentially replacing them with some other mechanism TBD). 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.


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.


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?


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.


Although it sounds beneficial to be able to delegate the SM 
implementation to a 3rd-party, in reality, I think very few people would

take the time to implement it securely, and instead would mostly
leverage its power to do things that aren't at all security related. 
Sure, removing the default SM and Policy implementation reduces the 
complexity a little, but there would still be a fairly significant 
maintenance overhead and an additional drawback that it would make it 
more difficult for applications and libraries to depend on any type 
of consistent behavior.


--Sean



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 

Re: A possible JEP to replace SecurityManager after JEP 411

2022-04-08 Thread Andrew Dinn
I'm 100% in agreement with Sean. This proposal leaves the OpenJDK team 
with just as much -- or possibly more -- code to maintain, test and 
design around while making the behaviour under the retained API less 
determinate, less reliable as a security enforcement mechanism and, in 
consequence, even less likely to be used than it is already.


regards,


Andrew Dinn
---
Red Hat Distinguished Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill

On 07/04/2022 20:19, Sean Mullan wrote:

Hi David,

Thanks for the feedback and spending some time on this proposal. Some 
specific comments below.


On 4/5/22 9:52 AM, David Lloyd wrote:

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.


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). 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). Any 
missing checks would need to be treated as security issues. 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.


The proposal also includes retaining calls to doPrivileged (but later 
potentially replacing them with some other mechanism TBD). 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.


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.


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?


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.


Although it sounds beneficial to be able to delegate the SM 
implementation to a 3rd-party, in reality, I think very few people would

take the time to implement it securely, and instead would mostly
leverage its power to do things that aren't at all security related. 
Sure, removing the default SM and Policy implementation reduces the 
complexity a little, but there would still be a fairly significant 
maintenance overhead and an additional drawback that it would make it 
more difficult for applications and libraries to depend on any type of 
consistent behavior.


--Sean



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 

Re: RFR: 8212136: Remove BaseSSLSocketImpl finalizer method [v2]

2022-04-08 Thread Xue-Lei Andrew Fan
On Thu, 7 Apr 2022 20:17:28 GMT, Xue-Lei Andrew Fan  wrote:

>> Please review the update to remove finalizer method in the SunJSSE provider 
>> implementation.  It is one of the efforts to clean up the use of finalizer 
>> method in JDK.
>
> Xue-Lei Andrew Fan has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - typo blank linke correction
>  - revise the update

> Thanks for the explanation: this is my first exposure to the 
> `java.lang.ref.Cleaner` API, so am getting up to speed. Sorry if these are 
> dumb comments/questions.
> 
> I see now what was being talked about in your other PR: #8136 and to not use 
> a reference to `this` which would keep it from being GC'd. I also see how 
> you're keeping a cleaner object that has outside ("static") references to the 
> actual things that need to be released, but don't we need to do the similar 
> cleaning for the underlying Socket somehow? What do Sockets do to make sure 
> the underlying file descriptors/native memory are properly released?
> 

The Socket implementation will take care of the file description/native memory 
release, I think.

> That said, we still need to send the close_notify at the TLS layer, right? 
> Simply removing the finalize() method is just going to silently shutdown the 
> connection, and then the Socket is going to do whatever it does for 
> finalization/Cleaning.

It is expected to send the close_notify at the TLS layer.  But the current code 
using finalizer, which is not reliable.  The underlying socket may have been 
closed when the SSLSocket finalizing action is triggered.  Generally, 
application should call close() method explicitly, otherwise the finalizer is 
not expect to work reliable.   I was wondering it may be safe to remove the 
finalizer.

I agree that adding a best effort cleanup may be better.  I was wondering to 
check if it is possible to clean the socket in the socket creation factories so 
that the object reference issues could be resolved.  But as socket is a kind of 
resource, application layer may make the clean up as well.

Socket s = ...
cleaner.register(this, s::close); 


I'm still looking for a solution to clean up resource by using a method of 
'this'.  Please advice if anyone has experiences.

-

PR: https://git.openjdk.java.net/jdk/pull/8065