Re: [External] : Re: JEP411: Missing use-case: Monitoring / restricting libraries

[Peter Firmstone]

Studies, experts, meetings and academics, without practical application 
of the principle of least privilege, these are meaningless.  I hope 
someone remembered to bring tea and scones.


My point is nothing gets done without workers.  It's the Pareto 
Principle or 80:20 rule.   Li Gong may have been an academic at that 
time, but he also got down to business and got the work done too, today 
he's a CEO.  How many of your experts are CEO's?   Very smart people 
study in academia, but they spread their wings and apply what they've 
learned, and in doing so learn new lessons and truly understand, then 
they are no longer academics, some retreat to the safety of the academic 
world.   The best lecturers were retiree's from the real world of 
application, well lets just leave it at that.


No matter how expert you are at A, doesn't qualify you to comment on B, 
when you've had limited or no experience in B.


I have demonstrated practical application and arguments based on 
examples and implementations.


Yes, NPV analysis indicates the best option is to not upgrade. This is 
the least harm, lowest cost option, in your words, I can't have both A 
and B, so I've chosen A.


Abandoning the principles of least privilege will significantly increase 
the consequences of a security breach.


The Australian native bee, has a unique defense system:

1. Guard bee's are positioned at the hive entrance to authenticate the
   bees entering the hive and will defend against intruders, battling
   until death.
2. If an intruder makes it past the guard bees, the bees inside the
   hive use resin to glue the intruders to the hives internal
   structures, intruders are rendered immobile and cannot move. Then
   the bees continue about their daily tasks ignoring the intruders
   until they die, then a bee on cleanup duty removes them and discards
   their carcasses outside the hive.

This is the principle of least privilege, to render the attacker 
harmless after breaching the perimeter, also it helps maintain perimeter 
defenses by reducing the attackers ability to pull them down, in doing 
so, it adds to the effectiveness of perimeter defenses.


I wonder how long the bee's would survive if they stopped immobilizing 
intruders and let them eat their larvae?


No it makes Bee's less secure, not more.

I get it that many developers don't care about security, at least until 
they wish they had.   Bee's get it, I wonder why many developers don't?


I get it that OpenJDK didn't know that SecurityManager infrastructure 
actually worked and thought that no one was using it, maybe that's why 
they did nothing to improve it, because they never believed in it in the 
first place, it was an annoying feature that got in the way of getting 
something else done.


https://github.com/openjdk/jdk/blame/master/src/java.base/share/classes/java/lang/SecurityManager.java

https://github.com/openjdk/jdk/blame/master/src/hotspot/share/prims/jvm.cpp

https://github.com/openjdk/jdk/blame/master/src/java.base/share/classes/java/security/AccessController.java

https://github.com/openjdk/jdk/blame/master/src/java.base/share/native/libjava/AccessController.c

I accept it's being removed, I just want you to understand just what it 
is you're removing, so you understand that this isn't going to be easy 
for you either, the consequences await you at a later date, I can see 
them, but you cannot because you cannot foresee what's unknown to you.


I hope that next time, more thorough research is done before making such 
hasty decisions.  Even experts make mistakes.  More time needs to be 
spent on community consultation, but then maybe not if its not something 
you care about.


With respect I think it's time to retire this conversation, I'm sure we 
both have other things we need to do.  Judging by the conversation, 
people haven't had time to review the practical examples submitted, or 
aren't interested, I'll remain available should there be interest.


Regards,

Peter.

On 22/05/2021 11:12 am, Ron Pressler wrote:

Let me be very clear: the proposers of this JEP, some of whom have worked on 
the Security Manager for the
last twenty years, strongly believe that not only will its removal not harm 
Java’s security, but considerably
improve it, as do the maintainers of other platforms who have decided to either 
not try to offer security
through the deep stack-dependent sandbox model or did but have since also 
removed it. Their view is backed
by security experts, both those working on OpenJDK and outside it.

While I seriously hope you don’t actually believe a decision was made before 
publication, it is also obviously
true that this wouldn’t have been proposed in the first place without *a lot* 
of thought, study and discussion.
The proposal was made only after it was clear this was a very, *very* strong 
case, and that the chances of finding
fault with it are low. It wasn’t put up after someone just had an idea. It is, 
therefore, not entirely
surprising th

Re: [External] : Re: JEP411: Missing use-case: Monitoring / restricting libraries

[Ron Pressler]

Let me be very clear: the proposers of this JEP, some of whom have worked on 
the Security Manager for the
last twenty years, strongly believe that not only will its removal not harm 
Java’s security, but considerably
improve it, as do the maintainers of other platforms who have decided to either 
not try to offer security
through the deep stack-dependent sandbox model or did but have since also 
removed it. Their view is backed
by security experts, both those working on OpenJDK and outside it.

While I seriously hope you don’t actually believe a decision was made before 
publication, it is also obviously
true that this wouldn’t have been proposed in the first place without *a lot* 
of thought, study and discussion.
The proposal was made only after it was clear this was a very, *very* strong 
case, and that the chances of finding
fault with it are low. It wasn’t put up after someone just had an idea. It is, 
therefore, not entirely 
surprising that no one has been able to give any relevant arguments against it. 
Clearly, you have had far less 
time to think about it, but I can’t seem to steer your arguments in a direction 
that’s relevant to what is actually 
the issue.

I understand you’re invested in the Security Manager and that its removal would 
impose real costs on you. I am even
willing to believe that you actually believe that, despite what studies show, 
despite what experts say, despite
what the developers of Java say, despite what the developers of *all* other 
mainstream software platforms with 
an emphasis on security, both new and old —- from .NET to WebAssembly —- have 
concluded after decades of experience, 
that the Security Manager is not only the best way to secure Java but 
apparently the only one. Maybe you’re right 
and those others are wrong, but please accept that we all want to improve 
Java’s security, we just disagree with you 
on the best way to do it.

Yes, additional security measures, whatever they are, would provide additional 
security. But if the choice is 
between measure A and measure B -— you *can’t* have both —- you pick the one 
that is *more* effective per cost. 
None of your arguments so much as glance in that direction, and they don’t 
acknowledge the fact that Java security 
would be hardly affected by the Security Manager’s removal even without better 
protection elsewhere for the simple 
reason that it is hardly ever installed, including on the most 
security-critical applications, whose defences 
apparently aren’t so feeble even today. In any event, if the question is, do we 
want a perimeter fence and security 
cameras *XOR* locks on all room doors, the argument that they provide security 
through different mechanisms so we 
should have both completely misunderstands the question. I am also confused by 
your point about multi-user
applications. Of course different users have different access, but surely you 
are aware that very few applications
do that using the Security Manager, which isn’t needed —- and is rarely used -— 
for that purpose.

Short of making relevant arguments, I would urge you again to focus on 
suggestions to reduce the harm this proposal 
would cause you.

— Ron



> On 22 May 2021, at 00:17, Peter Firmstone  wrote:
> 
> I had hoped by end of this discussion, that there would at least be an 
> understanding of what OpenJDK is so hastily choosing to destroy.
> 
> Once it is gone, it will be irretrievable, it will never be possible to lock 
> down the JVM so securely again.
> 
> 
> On 21/05/2021 11:06 pm, Ron Pressler wrote:
>> 
>>> On 21 May 2021, at 12:52, Peter Firmstone  
>>> wrote:
>>> 
>>> It's quite clear this will be pushed through anyway,
>>> 
>> No, not *anyway*, but given the fact that the community consists of millions 
>> of users, this
>> proposal has been well-publicised,
> 
> 
> I discovered the proposal on the 11th of the May on a mailing list I was 
> subscribed to and I almost missed it.   Yes, it will be pushed through 
> regardless, clearly the decision was made before publication.   Everyone saw 
> applets coming, if the developers were serious about supporting applets, they 
> would have designed a stripped down subset of Java, a JVM specifically suited 
> that task which, didn't include things like XML or serialization.
> 
> Just think, Applets were killed because of their atrocious security.   How 
> ironic.
> 
> 
>>> The granularity is not arbitrary, you said by class, which is incorrect.
>>> 
>>> Granularity is by a combination of one or more of the following:
>>> 
>>> • ProtectionDomain
>>> • CodeSource
>>> • Code signers
>>> • ClassLoader
>>> • Principals.
>> What I said is correct. Assigning a ProtectionDomain to a class is possible, 
>> though not to a method
>> (certainly not in code you can’t modify). In fact, ProtectionDomain is 
>> defined as “a set of classes,”
>> i.e. class granularity. In particular, that is the granularity that 
>> instrumentation with doPrivileged
>> aims to address,

Re: RFR: 8248268: Support KWP in addition to KW [v7]

[Xue-Lei Andrew Fan]

On Fri, 14 May 2021 00:33:12 GMT, Valerie Peng  wrote:

>> This change updates SunJCE provider as below:
>> - updated existing AESWrap support with AES/KW/NoPadding cipher 
>> transformation. 
>> - added support for AES/KWP/NoPadding and AES/KW/PKCS5Padding.
>> 
>> Existing AESWrap impl, i.e. AESWrapCipher class, is re-factored and renamed 
>> to KeyWrapCipher class. The W and W_inverse functions are moved to KWUtil 
>> class. The KW and KWP support are in the new AESKeyWrap and AESKeyWrapPadded 
>> classes which extend FeedbackCipher and used in KeyWrapCipher class. To 
>> minimize data copying, AESKeyWrap and AESKeyWrapPadded will do the crypto 
>> operation over the same input buffer which is allocated and managed by 
>> KeyWrapCipher class. 
>> 
>> Also note that existing AESWrap impl does not take IV. However, the 
>> corresponding PKCS#11 mechanisms do, so I added support for accepting IVs to 
>> both KW and KWP.
>> 
>> Thanks,
>> Valerie
>
> Valerie Peng has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains seven commits:
> 
>  - Merge master into JDK-8248268
>  - Minor update to address review comments.
>  - Changed AESParameters to allow 4-byte, 8-byte IVs and removed
>KWParameters and KWPParameters.
>  - Refactor code to reduce code duplication
>Address review comments
>Add more test vectors
>  - Changed AlgorithmParameters impls to register under AES/KW/NoPadding and
>AES/KWP/NoPadding
>  - Restored Iv algorithm parameters impl.
>  - 8248268: Support KWP in addition to KW
>
>Updated existing AESWrap support with AES/KW/NoPadding cipher
>transformation. Added support for AES/KWP/NoPadding and
>AES/KW/PKCS5Padding support to SunJCE provider.

src/java.base/share/classes/com/sun/crypto/provider/ConstructKeys.java line 176:

> 174: return ConstructKeys.constructPublicKey(encoding, 
> keyAlgorithm);
> 175: default:
> 176: throw new RuntimeException("Unsupported key type");

Good improvement to thrown an exception rather than return "null"!

Is NoSuchAlgorithmException fitted better the specification?  See also the 
comment in  KeyWrapCipher.engineUnwrap().

src/java.base/share/classes/com/sun/crypto/provider/ConstructKeys.java line 190:

> 188: return constructKey(encoding, keyAlgorithm, keyType);
> 189: } else {
> 190: byte[] encoding2 = Arrays.copyOfRange(encoding, ofs, ofs 
> + len);

The array will be copied again in the X509EncodedKeySpec and 
PKCS8EncodedKeySpec.  It is out of this scope, but it may be a performance 
improvement if adding X509EncodedKeySpec and PKCS8EncodedKeySpec constructors 
that accept array offset, so that the array copy here could be avoid.

src/java.base/share/classes/com/sun/crypto/provider/ConstructKeys.java line 192:

> 190: byte[] encoding2 = Arrays.copyOfRange(encoding, ofs, ofs 
> + len);
> 191: try {
> 192: return constructKey(encoding2, keyAlgorithm, 
> keyType);

The two constructKey methods are depended on each other.  It may improve the 
readability if only having one-way dependence, by moving the switch-block logic 
from the former constructKey() to the later one.


 static final Key constructKey(byte[] encoding, String keyAlgorithm,
   int keyType) throws InvalidKeyException, NoSuchAlgorithmException {
return constructKey(encoding, 0. encoding.length, keyAlgorithm, keyType);
}


With this re-org, the array copy could be avoid if the key type is unknown.

src/java.base/share/classes/com/sun/crypto/provider/KeyWrapCipher.java line 710:

> 708: outLen = helperDecrypt(dataBuf, dataIdx);
> 709: return ConstructKeys.constructKey(dataBuf, 0, outLen,
> 710: wrappedKeyAlgorithm, wrappedKeyType);

Per the specification of engineUnwrap,  maybe, the ConstructKeys.constructKey() 
implementation could throw NoSuchAlgorithmException if wrappedKeyType is 
unknown.  See the common in the the ConstructKeys.constructKey() implementation.

-

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

Re: [External] : Re: JEP411: Missing use-case: Monitoring / restricting libraries

[Peter Firmstone]

I had hoped by end of this discussion, that there would at least be an 
understanding of what OpenJDK is so hastily choosing to destroy.


Once it is gone, it will be irretrievable, it will never be possible to 
lock down the JVM so securely again.



On 21/05/2021 11:06 pm, Ron Pressler wrote:



On 21 May 2021, at 12:52, Peter Firmstone  wrote:

It's quite clear this will be pushed through anyway,


No, not *anyway*, but given the fact that the community consists of millions of 
users, this
proposal has been well-publicised,



I discovered the proposal on the 11th of the May on a mailing list I was 
subscribed to and I almost missed it.   Yes, it will be pushed through 
regardless, clearly the decision was made before publication.   Everyone 
saw applets coming, if the developers were serious about supporting 
applets, they would have designed a stripped down subset of Java, a JVM 
specifically suited that task which, didn't include things like XML or 
serialization.


Just think, Applets were killed because of their atrocious security.   
How ironic.




The granularity is not arbitrary, you said by class, which is incorrect.

Granularity is by a combination of one or more of the following:

• ProtectionDomain
• CodeSource
• Code signers
• ClassLoader
• Principals.

What I said is correct. Assigning a ProtectionDomain to a class is possible, 
though not to a method
(certainly not in code you can’t modify). In fact, ProtectionDomain is defined 
as “a set of classes,”
i.e. class granularity. In particular, that is the granularity that 
instrumentation with doPrivileged
aims to address, and that is one of the Security Manager’s most defining 
features.


It may be possible to assign a ProtectionDomain, to a single class, but 
that doesn't make your assertions correct, you should be quoting common 
use cases, I have never seen an example of assigning permissions to a 
single class, besides, it requires a dynamic policy to do that, and Java 
doesn't have one by default, so you can't use PolicyFile to assign it 
permissions.   Maybe you could use it to encapsulate ObjectInputStream 
with no permissions, then no one could grant it permissions, so that 
would be useful for Security.  It doesn't change class resolution or 
visibility. But OpenJDK didn't do that, why not?



What use case would there be to assign a ProtectionDomain to a method?

Just use a permission check in the method, or wrap a sensitive class 
with a decorator before publication:


http://svn.apache.org/viewvc/river/permission_delegates/src/main/java/org/apache/river/api/delegates/package.html?view=markup

But these are corner cases.

More useful cases are for isolation, such as JEE.




Restricting access by principal at the application level does not require the 
Security Manager, so that
part is irrelevant, and, in fact, not only Principal, but also Permission, and 
even CodeSource and
ProtectionDomain are *not* being proposed for terminal deprecation or even 
deprecation by this JEP.



I guess your use case is a desktop application running in a single process?

What about a multi user server application running in a single 
process?   Now we have to spawn multiple processes for each user, that's 
hardly efficient or performant is it?






I would like to understand this pain that is being caused to a far greater 
number of people?   So far information has been scarce and it seems more of an 
excuse, as it's very light on detail.  I would guess it's the pain of having to 
update policy files and making sure tests pass with security enabled.

The pain is that the high cost of maintaining the Security Manager comes at the 
expense of
other security measures that, we believe, provide far more security value to 
the Java ecosystem
as a whole.



Such as?



I think the results of locking down the JVM to principles of least privilege 
are totally worth it and a saleable commodity in the current global environment.

I absolutely accept the principle of least privilege. I do not accept that the 
marginal cost/benefit
of applying it at class granularity yields its best application.



I agree that there's little value for class granularity, but you are 
applying a corner case that although possible, is never applied in 
practice, and applying it with a broad brush, then using it as an 
argument against, please stop making this false assertion.   Just 
because you can do something, doesn't mean that you should.   Just 
because you can walk in front of a passing train, doesn't mean you 
should sir.


There is however a significant benefit for applying the principle of 
least privilege.


It can be assigned to Principal and Code signer granularity, that's 
actually quite coarse grained.  It's very flexible, unlike white listing 
Serializable classes.



Sure, theoretical things might, but there's no implementation in existence.  It 
has been quite affordable for me, so I wish to understand this pain, because

Re: RFR: 8255557: Decouple GCM from CipherCore [v5]

[Anthony Scarpino]

> Hi,
> 
> I need a review of this rather large change to GCM.  GCM will no longer use 
> CipherCore, and AESCrypt  to handle it's buffers and other objects.  It is 
> also a major code redesign limits the amount of data copies and make some 
> performance-based decisions.
> 
> Thanks
> 
> Tony

Anthony Scarpino has updated the pull request incrementally with one additional 
commit since the last revision:

  Review comments update

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4072/files
  - new: https://git.openjdk.java.net/jdk/pull/4072/files/e75926e8..76a984a0

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4072&range=04
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4072&range=03-04

  Stats: 221 lines in 5 files changed: 85 ins; 84 del; 52 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4072.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4072/head:pull/4072

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

Re: RFR: 8248268: Support KWP in addition to KW [v7]

[Xue-Lei Andrew Fan]

On Fri, 14 May 2021 00:33:12 GMT, Valerie Peng  wrote:

>> This change updates SunJCE provider as below:
>> - updated existing AESWrap support with AES/KW/NoPadding cipher 
>> transformation. 
>> - added support for AES/KWP/NoPadding and AES/KW/PKCS5Padding.
>> 
>> Existing AESWrap impl, i.e. AESWrapCipher class, is re-factored and renamed 
>> to KeyWrapCipher class. The W and W_inverse functions are moved to KWUtil 
>> class. The KW and KWP support are in the new AESKeyWrap and AESKeyWrapPadded 
>> classes which extend FeedbackCipher and used in KeyWrapCipher class. To 
>> minimize data copying, AESKeyWrap and AESKeyWrapPadded will do the crypto 
>> operation over the same input buffer which is allocated and managed by 
>> KeyWrapCipher class. 
>> 
>> Also note that existing AESWrap impl does not take IV. However, the 
>> corresponding PKCS#11 mechanisms do, so I added support for accepting IVs to 
>> both KW and KWP.
>> 
>> Thanks,
>> Valerie
>
> Valerie Peng has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains seven commits:
> 
>  - Merge master into JDK-8248268
>  - Minor update to address review comments.
>  - Changed AESParameters to allow 4-byte, 8-byte IVs and removed
>KWParameters and KWPParameters.
>  - Refactor code to reduce code duplication
>Address review comments
>Add more test vectors
>  - Changed AlgorithmParameters impls to register under AES/KW/NoPadding and
>AES/KWP/NoPadding
>  - Restored Iv algorithm parameters impl.
>  - 8248268: Support KWP in addition to KW
>
>Updated existing AESWrap support with AES/KW/NoPadding cipher
>transformation. Added support for AES/KWP/NoPadding and
>AES/KW/PKCS5Padding support to SunJCE provider.

src/java.base/share/classes/com/sun/crypto/provider/AESParameters.java line 50:

> 48: 
> 49: public AESParameters() {
> 50: core = new BlockCipherParamsCore(AESConstants.AES_BLOCK_SIZE, 4, 
> 8);

A cipher object may not take different IV sizes at the same time.  I was just 
wondering how it could be used in practice.  Maybe something like:


AlgorithmParameters algParams = AlgorithmParameters.getInstance("AES");
algParams.init(ivParameterSpec);

The IV parameter is given with the init() method.  Then, it may be not 
necessary to construct the BlockCipherParamsCore object will all potential IV 
sizes.  See the comments in BlockCipherParamsCore.

src/java.base/share/classes/com/sun/crypto/provider/BlockCipherParamsCore.java 
line 52:

> 50: private byte[] iv = null;
> 51: 
> 52: private int[] moreSizes = null;

The moreSizes is not used other than the init() method field.  It may be not 
easy to check the specific size if we cache all supported sized in the object.  
For example, if the required IV size if 8 bytes, it may be a problem about how 
to make sure the iv size is 8 bytes exactly for a specific algorithm.

Maybe, we could just have a ivSize filed.  The default value is block_size, 
which coupe be set with the init(ivParameterSpec) method.



private int ivSize;
...
   BlockCipherParamsCore(int blkSize) {
   block_size = blkSize;
   ivSize = blkSize;
}
...
   void init(AlgorithmParameterSpec paramSpec) {
ivSize = ...;  // reset IV size.
}

// use ivSize while encoding and decoding.

src/java.base/share/classes/com/sun/crypto/provider/BlockCipherParamsCore.java 
line 81:

> 79: expectedLen + " bytes long");
> 80: }
> 81: iv = tmpIv.clone();

The moreSizes is not used after initialization.  The iv/tmpIv could be a value 
other than the block_size.   The getEncoded() method would use the iv value for 
the encoding.  While in the decoding method init(byte[]) method, the IV sizes 
other block_size is not considered, and IOE will be thrown.  Could this be a 
problem?

-

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

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Weijun Wang]

On Fri, 21 May 2021 20:43:05 GMT, Phil Race  wrote:

> I haven't seen any response to this comment I made a couple of days ago and I 
> suspect it got missed
> 
> > Weijun Wang has updated the pull request incrementally with one additional 
> > commit since the last revision:
> > fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java
> 
> test/jdk/java/awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java line 59:
> 
> > 57: ProcessCommunicator
> > 58: .executeChildProcess(Consumer.class, new 
> > String[0]);
> > 59: if (!"Hello".equals(processResults.getStdOut())) {
> 
> Who or what prompted this change ?

I replied right in the thread but unfortunately GitHub does not display it at 
the end of page.

This is because the process is now launched with 
`-Djava.security.manager=allow` (because of another change in 
https://github.com/openjdk/jdk/pull/4071), and a new warning is displayed in 
stderr. Therefore I switched to stdout.

-

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

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Phil Race]

On Wed, 19 May 2021 13:47:53 GMT, Weijun Wang  wrote:

>> Please review this implementation of [JEP 
>> 411](https://openjdk.java.net/jeps/411).
>> 
>> The code change is divided into 3 commits. Please review them one by one.
>> 
>> 1. 
>> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>>  The essential change for this JEP, including the `@Deprecate` annotations 
>> and spec change. It also update the default value of the 
>> `java.security.manager` system property to "disallow", and necessary test 
>> change following this update.
>> 2. 
>> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>>  Manual changes to several files so that the next commit can be generated 
>> programatically.
>> 3. 
>> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>>  Automatic changes to other source files to avoid javac warnings on 
>> deprecation for removal
>> 
>> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
>> generated programmatically, see the comment below for more details. If you 
>> are only interested in a portion of the 3rd commit and would like to review 
>> it as a separate file, please comment here and I'll generate an individual 
>> webrev.
>> 
>> Due to the size of this PR, no attempt is made to update copyright years for 
>> any file to minimize unnecessary merge conflict.
>> 
>> Furthermore, since the default value of `java.security.manager` system 
>> property is now "disallow", most of the tests calling 
>> `System.setSecurityManager()` need to launched with 
>> `-Djava.security.manager=allow`. This is covered in a different PR at 
>> https://github.com/openjdk/jdk/pull/4071.
>> 
>> Update: the deprecation annotations and javadoc tags, build, compiler, 
>> core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are 
>> reviewed. Rest are 2d, awt, beans, sound, and swing.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java

I haven't seen any response to this comment I made a couple of days ago and I 
suspect it got missed
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
>
>   fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java

test/jdk/java/awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java line 59:

> 57: ProcessCommunicator
> 58: .executeChildProcess(Consumer.class, new 
> String[0]);
> 59: if (!"Hello".equals(processResults.getStdOut())) {

Who or what prompted this change ?

-

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

Re: RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K [v3]

[Weijun Wang]

> The code change refactors classes that have a `SuppressWarnings("removal")` 
> annotation that covers more than 50KB of code. The big annotation is often 
> quite faraway from the actual deprecated API usage it is suppressing, and 
> with the annotation covering too big a portion it's easy to call other 
> deprecated methods without being noticed.
> 
> The code change shows some common solutions to avoid such too wide 
> annotations:
> 
> 1. Extract calls into a method and add annotation on that method
> 2. Assign the return value of a deprecated method call to a new local 
> variable and add annotation on the declaration, and then assign that value to 
> the original l-value.
> 3. Put declaration and assignment into a single statement if possible.
> 4. Rewrite code to achieve #3 above.

Weijun Wang has updated the pull request incrementally with one additional 
commit since the last revision:

  update FtpClient.java

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4138/files
  - new: https://git.openjdk.java.net/jdk/pull/4138/files/d460efb8..60d97a4c

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4138&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4138&range=01-02

  Stats: 23 lines in 1 file changed: 0 ins; 12 del; 11 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4138.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4138/head:pull/4138

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

Re: RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K [v2]

[Weijun Wang]

On Fri, 21 May 2021 15:37:48 GMT, Daniel Fuchs  wrote:

>> Weijun Wang has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   typo on windows
>
> src/java.base/share/classes/sun/net/ftp/impl/FtpClient.java line 550:
> 
>> 548:  * @throws IOException if the connection was unsuccessful.
>> 549:  */
>> 550: @SuppressWarnings("removal")
> 
> Could the scope of the annotation be further reduced by applying it to the 
> two places where `doPrivileged` is called in this method?

I'll probably need to assign the doPriv result on L631 to a tmp variable and 
then assign it to `s`. Are you OK with this ugliness? Update: Ah, I see you 
already have similar suggestion in the next comment.

-

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

Re: RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K [v2]

[Weijun Wang]

On Fri, 21 May 2021 15:35:05 GMT, Daniel Fuchs  wrote:

>> Weijun Wang has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   typo on windows
>
> src/java.base/share/classes/sun/net/ftp/impl/FtpClient.java line 120:
> 
>> 118: vals[1] = 
>> Integer.getInteger("sun.net.client.defaultConnectTimeout", 
>> 300_000).intValue();
>> 119: return System.getProperty("file.encoding", 
>> "ISO8859_1");
>> 120: }
> 
> It is a bit strange that "file.encoding" seem to get a special treatment - 
> but I guess that's OK.

You might say we thus avoid wasting the return value, as much as possible.

-

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

Re: RFR: 8267184: Add -Djava.security.manager=allow to tests calling System.setSecurityManager [v3]

[Weijun Wang]

> Please review the test changes for [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> With JEP 411 and the default value of `-Djava.security.manager` becoming 
> `disallow`, tests calling `System.setSecurityManager()`  need 
> `-Djava.security.manager=allow` when launched. This PR covers such changes 
> for tier1 to tier3 (except for the JCK tests).
> 
> To make it easier to focus your review on the tests in your area, this PR is 
> divided into multiple commits for different areas (~~serviceability~~, 
> ~~hotspot-compiler~~, ~~i18n~~, ~~rmi~~, ~~javadoc~~, swing, 2d, 
> ~~security~~, ~~hotspot-runtime~~, ~~nio~~, ~~xml~~, ~~beans~~, 
> ~~core-libs~~, ~~net~~, ~~compiler~~, ~~hotspot-gc~~). Mostly the rule is the 
> same as how Skara adds labels, but there are several small tweaks:
> 
> 1. When a file is covered by multiple labels, only one is chosen. I make my 
> best to avoid putting too many tests into `core-libs`. If a file is covered 
> by `core-libs` and another label, I categorized it into the other label.
> 2. If a file is in `core-libs` but contains `/xml/` in its name, it's in the 
> `xml` commit.
> 3. If a file is in `core-libs` but contains `/rmi/` in its name, it's in the 
> `rmi` commit.
> 4. One file not covered by any label -- 
> `test/jdk/com/sun/java/accessibility/util/8051626/Bug8051626.java` -- is in 
> the `swing` commit.
> 
> Due to the size of this PR, no attempt is made to update copyright years for 
> all files to minimize unnecessary merge conflict.
> 
> Please note that this PR can be integrated before the source changes for JEP 
> 411, as the possible values of this system property was already defined long 
> time ago in JDK 9.
> 
> Most of the change in this PR is a simple adding of 
> `-Djava.security.manager=allow` to the `@run main/othervm` line. Sometimes it 
> was not `othervm` and we add one. Sometimes there's no `@run` at all and we 
> add the line.
> 
> There are several tests that launch another Java process that needs to call 
> the `System.setSecurityManager()` method, and the system property is added to 
> `ProcessBuilder`, `ProcessTools`, or the java command line (if the test is a 
> shell test).
> 
> 3 langtools tests are added into problem list due to 
> [JDK-8265611](https://bugs.openjdk.java.net/browse/JDK-8265611).
> 
> 2 SQL tests are moved because they need different options on the `@run` line 
> but they are inside a directory that has a `TEST.properties`:
> 
> rename test/jdk/java/sql/{testng/test/sql/othervm => 
> permissionTests}/DriverManagerPermissionsTests.java (93%)
> rename test/jdk/javax/sql/{testng/test/rowset/spi => 
> permissionTests}/SyncFactoryPermissionsTests.java (95%)
>  ```
> 
> The source change for JEP 411 is at https://github.com/openjdk/jdk/pull/4073.

Weijun Wang has updated the pull request incrementally with one additional 
commit since the last revision:

  feedback from Phil
  
  reverted:

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4071/files
  - new: https://git.openjdk.java.net/jdk/pull/4071/files/bfa955ad..9a3ec578

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4071&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4071&range=01-02

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4071.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4071/head:pull/4071

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

Re: RFR: 8267184: JEP 411: Add -Djava.security.manager=allow to tests calling System.setSecurityManager [v2]

[Weijun Wang]

On Fri, 21 May 2021 18:26:48 GMT, Phil Race  wrote:

>> Weijun Wang has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   adjust order of VM options
>
> test/jdk/java/awt/FontClass/CreateFont/fileaccess/FontFile.java line 3:
> 
>> 1: /*
>> 2:  * Copyright (c) 2008, 2021, Oracle and/or its affiliates. All rights 
>> reserved.
>> 3:  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
> 
> Probably the (c) update was meant to be on the .sh file that is actually 
> modified.

Oops, I'll back it out. Thanks.

-

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

Re: RFR: 8267184: JEP 411: Add -Djava.security.manager=allow to tests calling System.setSecurityManager [v2]

[Phil Race]

On Tue, 18 May 2021 21:44:43 GMT, Weijun Wang  wrote:

>> Please review the test changes for [JEP 
>> 411](https://openjdk.java.net/jeps/411).
>> 
>> With JEP 411 and the default value of `-Djava.security.manager` becoming 
>> `disallow`, tests calling `System.setSecurityManager()`  need 
>> `-Djava.security.manager=allow` when launched. This PR covers such changes 
>> for tier1 to tier3 (except for the JCK tests).
>> 
>> To make it easier to focus your review on the tests in your area, this PR is 
>> divided into multiple commits for different areas (~~serviceability~~, 
>> ~~hotspot-compiler~~, ~~i18n~~, ~~rmi~~, ~~javadoc~~, swing, 2d, 
>> ~~security~~, ~~hotspot-runtime~~, ~~nio~~, ~~xml~~, ~~beans~~, 
>> ~~core-libs~~, ~~net~~, ~~compiler~~, ~~hotspot-gc~~). Mostly the rule is 
>> the same as how Skara adds labels, but there are several small tweaks:
>> 
>> 1. When a file is covered by multiple labels, only one is chosen. I make my 
>> best to avoid putting too many tests into `core-libs`. If a file is covered 
>> by `core-libs` and another label, I categorized it into the other label.
>> 2. If a file is in `core-libs` but contains `/xml/` in its name, it's in the 
>> `xml` commit.
>> 3. If a file is in `core-libs` but contains `/rmi/` in its name, it's in the 
>> `rmi` commit.
>> 4. One file not covered by any label -- 
>> `test/jdk/com/sun/java/accessibility/util/8051626/Bug8051626.java` -- is in 
>> the `swing` commit.
>> 
>> Due to the size of this PR, no attempt is made to update copyright years for 
>> all files to minimize unnecessary merge conflict.
>> 
>> Please note that this PR can be integrated before the source changes for JEP 
>> 411, as the possible values of this system property was already defined long 
>> time ago in JDK 9.
>> 
>> Most of the change in this PR is a simple adding of 
>> `-Djava.security.manager=allow` to the `@run main/othervm` line. Sometimes 
>> it was not `othervm` and we add one. Sometimes there's no `@run` at all and 
>> we add the line.
>> 
>> There are several tests that launch another Java process that needs to call 
>> the `System.setSecurityManager()` method, and the system property is added 
>> to `ProcessBuilder`, `ProcessTools`, or the java command line (if the test 
>> is a shell test).
>> 
>> 3 langtools tests are added into problem list due to 
>> [JDK-8265611](https://bugs.openjdk.java.net/browse/JDK-8265611).
>> 
>> 2 SQL tests are moved because they need different options on the `@run` line 
>> but they are inside a directory that has a `TEST.properties`:
>> 
>> rename test/jdk/java/sql/{testng/test/sql/othervm => 
>> permissionTests}/DriverManagerPermissionsTests.java (93%)
>> rename test/jdk/javax/sql/{testng/test/rowset/spi => 
>> permissionTests}/SyncFactoryPermissionsTests.java (95%)
>>  ```
>> 
>> The source change for JEP 411 is at https://github.com/openjdk/jdk/pull/4073.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   adjust order of VM options

The client changes are fine except for the one misplaced (c)

test/jdk/java/awt/FontClass/CreateFont/fileaccess/FontFile.java line 3:

> 1: /*
> 2:  * Copyright (c) 2008, 2021, Oracle and/or its affiliates. All rights 
> reserved.
> 3:  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.

Probably the (c) update was meant to be on the .sh file that is actually 
modified.

-

Marked as reviewed by prr (Reviewer).

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

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Phil Race]

On Thu, 20 May 2021 07:06:00 GMT, Alan Bateman  wrote:

>> The JEP isn't PTT for 17 so there's plenty of time isn't there ?
>
> There are 827 files in this patch. Phil is right that adding SW at the class 
> level is introducing technical debt but if addressing that requires 
> refactoring and re-testing of AWT or font code then it would be better to 
> have someone from that area working on. Is there any reason why this can't be 
> going on now on awt-dev/2d-dev and integrated immediately after JEP 411? I 
> don't think we should put Max through the wringer here as there are too many 
> areas to cover.

Are you suggesting that the patch doesn't need testing as it is ? It should be 
the same either way.
It is very straight forward to run the automated tests across all platforms 
these days.
I get the impression that no one is guaranteeing to do this straight after 
integration.
It sounds like it is up for deferral if time runs out.

The amount of follow-on work that I am hearing about here, and may be for 
tests, does not  make it sound
like this JEP is nearly as done as first presented.

If there was some expectation that groups responsible for an area would get 
involved with this
issue which I am assured was already known about, then why was it not raised 
before and made
part of the plan ?

-

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

JEP 411: Disable warning message with flag?

[Mikael Sterner]

The current version of JEP 411 (Deprecate the Security Manager for Removal) has 
as its goal "Warn users if their Java applications rely on the Security 
Manager.". To that end it proposes to "Issue a warning message at startup if 
the Security Manager is enabled on the command line."

I would suggest adding a flag to disable the warning message, for use in cases 
where an application ships to end users with a Java runtime included. Because 
in those cases, the warning is meant for the developer of the application and 
not end users. End users would not be the ones providing/upgrading the Java 
runtime, and in many cases it would not be acceptable to have a warning 
displayed on startup that could confuse users.

If a flag to disable the command line warning is not added, the effect will be 
that the Security Manager is not possible to use in such applications already 
in Java 17 (counting on the proposed target), which seems rather harsh given 
the short notice.

If the flag is added, developers of applications that use the Security Manager 
will still notice the warning (until disabled) but they get more time to 
migrate to better solutions like process isolation. As a bonus, for 
hard-to-migrate cases you can stay on Java 17 with the Security Manager for as 
long as you're willing to pay, since many vendors seem to plan to offer long 
term support for it.

Yours,
Mikael Sterner

Re: RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K [v2]

[Daniel Fuchs]

On Fri, 21 May 2021 14:00:39 GMT, Weijun Wang  wrote:

>> The code change refactors classes that have a `SuppressWarnings("removal")` 
>> annotation that covers more than 50KB of code. The big annotation is often 
>> quite faraway from the actual deprecated API usage it is suppressing, and 
>> with the annotation covering too big a portion it's easy to call other 
>> deprecated methods without being noticed.
>> 
>> The code change shows some common solutions to avoid such too wide 
>> annotations:
>> 
>> 1. Extract calls into a method and add annotation on that method
>> 2. Assign the return value of a deprecated method call to a new local 
>> variable and add annotation on the declaration, and then assign that value 
>> to the original l-value.
>> 3. Put declaration and assignment into a single statement if possible.
>> 4. Rewrite code to achieve #3 above.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   typo on windows

src/java.base/share/classes/sun/net/ftp/impl/FtpClient.java line 120:

> 118: vals[1] = 
> Integer.getInteger("sun.net.client.defaultConnectTimeout", 
> 300_000).intValue();
> 119: return System.getProperty("file.encoding", 
> "ISO8859_1");
> 120: }

It is a bit strange that "file.encoding" seem to get a special treatment - but 
I guess that's OK.

src/java.base/share/classes/sun/net/ftp/impl/FtpClient.java line 550:

> 548:  * @throws IOException if the connection was unsuccessful.
> 549:  */
> 550: @SuppressWarnings("removal")

Could the scope of the annotation be further reduced by applying it to the two 
places where `doPrivileged` is called in this method?

src/java.base/share/classes/sun/net/ftp/impl/FtpClient.java line 921:

> 919: }
> 920: 
> 921: @SuppressWarnings("removal")

Could the scope be further reduced by applying it at line 926 in this method - 
at the cost of creating a temporary variable? The code could probably be 
further simplified by using a lambda...


PrivilegedAction pa = () -> new Socket(proxy);
@SuppressWarnings("removal")
var ps = AccessController.doPrivileged(pa);
s = ps;

-

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

Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal [v3]

[Daniel Fuchs]

On Wed, 19 May 2021 13:47:53 GMT, Weijun Wang  wrote:

>> Please review this implementation of [JEP 
>> 411](https://openjdk.java.net/jeps/411).
>> 
>> The code change is divided into 3 commits. Please review them one by one.
>> 
>> 1. 
>> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>>  The essential change for this JEP, including the `@Deprecate` annotations 
>> and spec change. It also update the default value of the 
>> `java.security.manager` system property to "disallow", and necessary test 
>> change following this update.
>> 2. 
>> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>>  Manual changes to several files so that the next commit can be generated 
>> programatically.
>> 3. 
>> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>>  Automatic changes to other source files to avoid javac warnings on 
>> deprecation for removal
>> 
>> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
>> generated programmatically, see the comment below for more details. If you 
>> are only interested in a portion of the 3rd commit and would like to review 
>> it as a separate file, please comment here and I'll generate an individual 
>> webrev.
>> 
>> Due to the size of this PR, no attempt is made to update copyright years for 
>> any file to minimize unnecessary merge conflict.
>> 
>> Furthermore, since the default value of `java.security.manager` system 
>> property is now "disallow", most of the tests calling 
>> `System.setSecurityManager()` need to launched with 
>> `-Djava.security.manager=allow`. This is covered in a different PR at 
>> https://github.com/openjdk/jdk/pull/4071.
>> 
>> Update: the deprecation annotations and javadoc tags, build, compiler, 
>> core-libs, hotspot, i18n, jmx, net, nio, security, and serviceability are 
>> reviewed. Rest are 2d, awt, beans, sound, and swing.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java

src/java.base/share/classes/java/lang/SecurityManager.java line 104:

> 102:  * method will throw an {@code UnsupportedOperationException}). If the
> 103:  * {@systemProperty java.security.manager} system property is set to the
> 104:  * special token "{@code allow}", then a security manager will not be 
> set at

Can/should the `{@systemProperty ...}` tag be used more than once for a given 
system property? I thought it should be used only once, at the place where the 
system property is defined. Maybe @jonathan-gibbons can offer some more 
guidance on this.

-

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

Re: RFR: 8267110: Update java.util to use instanceof pattern variable [v4]

[Patrick Concannon]

> Hi,
> 
> Could someone please review my code for updating the code in the `java.util` 
> package to make use of the `instanceof` pattern variable?
> 
> Kind regards,
> Patrick

Patrick Concannon has updated the pull request incrementally with one 
additional commit since the last revision:

  8267110: Reverted changes made to files in java.util.concurrent

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4088/files
  - new: https://git.openjdk.java.net/jdk/pull/4088/files/da615e7d..2c076a55

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4088&range=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4088&range=02-03

  Stats: 31 lines in 5 files changed: 16 ins; 0 del; 15 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4088.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4088/head:pull/4088

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

Re: RFR: 8267521: Post JEP 411 refactoring: maximum covering > 50K [v2]

[Weijun Wang]

> The code change refactors classes that have a `SuppressWarnings("removal")` 
> annotation that covers more than 50KB of code. The big annotation is often 
> quite faraway from the actual deprecated API usage it is suppressing, and 
> with the annotation covering too big a portion it's easy to call other 
> deprecated methods without being noticed.
> 
> The code change shows some common solutions to avoid such too wide 
> annotations:
> 
> 1. Extract calls into a method and add annotation on that method
> 2. Assign the return value of a deprecated method call to a new local 
> variable and add annotation on the declaration, and then assign that value to 
> the original l-value.
> 3. Put declaration and assignment into a single statement if possible.
> 4. Rewrite code to achieve #3 above.

Weijun Wang has updated the pull request incrementally with one additional 
commit since the last revision:

  typo on windows

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4138/files
  - new: https://git.openjdk.java.net/jdk/pull/4138/files/e3f0405a..d460efb8

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4138&range=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4138&range=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4138.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4138/head:pull/4138

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

Re: [External] : Re: JEP411: Missing use-case: Monitoring / restricting libraries

[Ron Pressler]

> On 21 May 2021, at 12:52, Peter Firmstone  wrote:
> 
> It's quite clear this will be pushed through anyway,
> 

No, not *anyway*, but given the fact that the community consists of millions of 
users, this
proposal has been well-publicised, only very few people have voiced objections, 
fewer
still who have voiced objections actually use the Security Manager for 
security, and no
one has yet made a single relevant argument, accepting this proposal so far 
seems like the
only reasonable outcome, perhaps with accommodations to offer alternatives for 
certain
use-cases. 

> so there's no harm in sharing the information, unless you don't have a list, 
> in which case it's not possible to share something you don't have.

I have already linked to one empirical study, I know that further scans of 
public repositories
have been conducted but *I* don’t have the data; those who do are free to share 
it if they
think there is no harm in that (although I think there might be).

> 
> The granularity is not arbitrary, you said by class, which is incorrect.
> 
> Granularity is by a combination of one or more of the following:
> 
>   • ProtectionDomain
>   • CodeSource
>   • Code signers
>   • ClassLoader
>   • Principals.

What I said is correct. Assigning a ProtectionDomain to a class is possible, 
though not to a method 
(certainly not in code you can’t modify). In fact, ProtectionDomain is defined 
as “a set of classes,”
i.e. class granularity. In particular, that is the granularity that 
instrumentation with doPrivileged 
aims to address, and that is one of the Security Manager’s most defining 
features.

Restricting access by principal at the application level does not require the 
Security Manager, so that 
part is irrelevant, and, in fact, not only Principal, but also Permission, and 
even CodeSource and 
ProtectionDomain are *not* being proposed for terminal deprecation or even 
deprecation by this JEP.

> 
> I would like to understand this pain that is being caused to a far greater 
> number of people?   So far information has been scarce and it seems more of 
> an excuse, as it's very light on detail.  I would guess it's the pain of 
> having to update policy files and making sure tests pass with security 
> enabled.

The pain is that the high cost of maintaining the Security Manager comes at the 
expense of
other security measures that, we believe, provide far more security value to 
the Java ecosystem
as a whole.

> 
> I think the results of locking down the JVM to principles of least privilege 
> are totally worth it and a saleable commodity in the current global 
> environment.

I absolutely accept the principle of least privilege. I do not accept that the 
marginal cost/benefit
of applying it at class granularity yields its best application.

> 
> Sure, theoretical things might, but there's no implementation in existence.  
> It has been quite affordable for me, so I wish to understand this pain, 
> because I currently don't, I'm already using the latest encryption, static 
> analysis, secure coding practices, validating input, sanitizing data etc.

There are, though. Here are some: JFR, the module system, crypto protocols and 
ad-hoc mechanisms for 
specific vulnerable components (serialization, XML etc.). Maintaining the 
Security Manager comes at their 
expense -- some require urgent improvements like adding more events to JFR and 
closing down gaps in the 
module system’s defences -- and we believe investing in them has a better 
security ROI overall.

> 
> Other techniques that are yet to be developed.   OpenJDK is deprecating 
> SecurityManager prior to the implementation of it's replacement, a little 
> more notice would have been nice.   I'm ready for you to deprecate 
> Serialization, we saw that coming, but this is just completely unexpected out 
> of left field.

First, any deprecation proposal could be said to be unexpected until it is 
proposed. But that is why 
we have a deprecation policy that makes the process gradual and gives people 
time to adjust. Second, I
don’t think this is "out of left field" at all. The writing on the wall was 
pretty clear when, after twenty-five 
years, few projects use the Security Manager and few libraries are properly 
instrumented for it, other platforms 
have decided not to adopt a similar model, and those few that have have already 
abandoned it some years ago.

> 
> Currently it is the only possible and practicable means that has been 
> implemented, I'm sure something better will come along, but everything better 
> is just theoretical at this time.
> 
> Nothing else is even remotely capable of locking the JVM down this securely.

I‘m sure you're responding to *some* argument, but not to mine. Any additional 
security measure would 
“lock something down” more than not having it. Whether or not it is worthwhile 
depends on the marginal 
benefit vs. the marginal cost. I believe that the marginal benefit for the 
marginal cost of the Se

Re: [External] : Re: JEP411: Missing use-case: Monitoring / restricting libraries

[Peter Firmstone]

On 21/05/2021 9:14 pm, Ron Pressler wrote:



On 21 May 2021, at 11:29, Peter Firmstone  wrote:


Can you share this list please?   If I see anything missing I can add it for 
you.

No, because this might give the false impression that this is a debate.



It's quite clear this will be pushed through anyway, so there's no harm 
in sharing the information, unless you don't have a list, in which case 
it's not possible to share something you don't have.






So I'm restricting permissions granted to only those required to perform the 
intended tasks of the software,  restricted even to a subset of which that 
software is capable, and you say it is not the principle of least privilege?

That is not what the Security Manager is doing, though. It is able to assign 
“least privilege” to some
arbitrary level of granularity and not others.



The granularity is not arbitrary, you said by class, which is incorrect.

Granularity is by a combination of one or more of the following:

1. ProtectionDomain
2. CodeSource
3. Code signers
4. ClassLoader
5. Principals.


How do you critique something if you don't understand how it works?

I stand by what I said. “X granularity” means up-to and no-less-than X, and 
assigning software privileges
per user at the application level does not require the Security Manager. I 
understand that your particular
codebase does use the Security Manager to assign all software privileges, and 
that removing it might require
costly changes to your codebase, but that is not the most common way, and it is 
certainly not the *only*
way this is done in Java codebases. I absolutely sympathise with the pain this 
proposal would cause you,
but please sympathise with the pain that not accepting it would cause what we 
believe is a far greater
number of people, and don’t try to universalise your particular situation.



You said class level granularity, which is incorrect.   True, it is not 
possible to upgrade, I guess you could say that's painful, as it will be 
for everyone else caught up by it.


I would like to understand this pain that is being caused to a far 
greater number of people?   So far information has been scarce and it 
seems more of an excuse, as it's very light on detail.  I would guess 
it's the pain of having to update policy files and making sure tests 
pass with security enabled.


I perform all tests with security enabled, because it will work if it 
isn't anyway.   Policy files are easily regenerated using tools.


What other pain is there?

I think the results of locking down the JVM to principles of least 
privilege are totally worth it and a saleable commodity in the current 
global environment.





So you are mistaken about what it is that I am critiquing. I am not critiquing 
the operation of the
Security Manager nor am I saying that it is incapable of preventing certain 
attacks. I am critiquing the
claim that no other technique can achieve better security more cheaply.



Sure, theoretical things might, but there's no implementation in 
existence.  It has been quite affordable for me, so I wish to understand 
this pain, because I currently don't, I'm already using the latest 
encryption, static analysis, secure coding practices, validating input, 
sanitizing data etc.




  I am saying that the marginal utility
if the Security Manager divided by its marginal cost is lower than the marginal 
utility divided by the marginal
cost of other security techniques.



Other techniques that are yet to be developed.   OpenJDK is deprecating 
SecurityManager prior to the implementation of it's replacement, a 
little more notice would have been nice.   I'm ready for you to 
deprecate Serialization, we saw that coming, but this is just completely 
unexpected out of left field.





We have no argument over the value of security or that of the principle of 
least privilege.
We merely disagree over whether the Security Manager specifically is the best 
possible means to those ends.



Currently it is the only possible and practicable means that has been 
implemented, I'm sure something better will come along, but everything 
better is just theoretical at this time.


Nothing else is even remotely capable of locking the JVM down this 
securely.






I don't wish to publicly show that I doubt your credibility, but you are making 
it difficult for me not to sir.  It is your user base you need to convince, so 
far, you are not very convincing.


I am sorry, but that not everyone would be convinced by every argument we make 
is something we take as a given.
It is the duty of those who disagree with proposals put forth by the 
maintainers to convince the maintainers,
not the other way around. I am trying to help you focus you on arguments that 
would be relevant. They are:


1. The *current* value the Security Manager to the Java ecosystem is high, 
evidenced by the great number of
companies that employ it for software security.

2. The Security Manager has the highest ROI compared 

Re: RFR: 8267110: Update java.util to use instanceof pattern variable [v3]

[Patrick Concannon]

> Hi,
> 
> Could someone please review my code for updating the code in the `java.util` 
> package to make use of the `instanceof` pattern variable?
> 
> Kind regards,
> Patrick

Patrick Concannon has updated the pull request with a new target base due to a 
merge or a rebase. The incremental webrev excludes the unrelated changes 
brought in by the merge/rebase. The pull request contains four additional 
commits since the last revision:

 - Merge remote-tracking branch 'origin/master' into JDK-8267110
 - 8267110: Reverted changes in java/util/Formatter as primitive to boxed types 
may have semantic/performance implications
 - Merge remote-tracking branch 'origin/master' into JDK-8267110
 - 8267110: Update java.util to use instanceof pattern variable

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/4088/files
  - new: https://git.openjdk.java.net/jdk/pull/4088/files/cd99dc49..da615e7d

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=4088&range=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=4088&range=01-02

  Stats: 4905 lines in 201 files changed: 2421 ins; 2058 del; 426 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4088.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4088/head:pull/4088

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

Re: [External] : Re: JEP411: Missing use-case: Monitoring / restricting libraries

[Ron Pressler]

> On 21 May 2021, at 11:29, Peter Firmstone  wrote:
> 
> 
> Can you share this list please?   If I see anything missing I can add it for 
> you.

No, because this might give the false impression that this is a debate. In a 
community
of millions, almost no decision will be universally accepted. So the goal here 
is to collect
information and have people try to convince the maintainers, not the other way 
around.

This is the only way to not get into interminable debates and to get things 
done. At the end of 
the day, either you trust that our intentions is to make Java more secure and 
more successful or 
you don’t. We hope that the majority of Java users do.

> 
> So I'm restricting permissions granted to only those required to perform the 
> intended tasks of the software,  restricted even to a subset of which that 
> software is capable, and you say it is not the principle of least privilege?

That is not what the Security Manager is doing, though. It is able to assign 
“least privilege” to some
arbitrary level of granularity and not others. Other techniques apply the same 
principle at different
granularities.

> 
> How do you critique something if you don't understand how it works?

I stand by what I said. “X granularity” means up-to and no-less-than X, and 
assigning software privileges
per user at the application level does not require the Security Manager. I 
understand that your particular
codebase does use the Security Manager to assign all software privileges, and 
that removing it might require
costly changes to your codebase, but that is not the most common way, and it is 
certainly not the *only*
way this is done in Java codebases. I absolutely sympathise with the pain this 
proposal would cause you,
but please sympathise with the pain that not accepting it would cause what we 
believe is a far greater
number of people, and don’t try to universalise your particular situation.

So you are mistaken about what it is that I am critiquing. I am not critiquing 
the operation of the 
Security Manager nor am I saying that it is incapable of preventing certain 
attacks. I am critiquing the 
claim that no other technique can achieve better security more cheaply. I am 
saying that the marginal utility 
if the Security Manager divided by its marginal cost is lower than the marginal 
utility divided by the marginal 
cost of other security techniques.

We have no argument over the value of security or that of the principle of 
least privilege.
We merely disagree over whether the Security Manager specifically is the best 
possible means to those ends.

> 
> I don't wish to publicly show that I doubt your credibility, but you are 
> making it difficult for me not to sir.  It is your user base you need to 
> convince, so far, you are not very convincing.
> 

I am sorry, but that not everyone would be convinced by every argument we make 
is something we take as a given.
It is the duty of those who disagree with proposals put forth by the 
maintainers to convince the maintainers,
not the other way around. I am trying to help you focus you on arguments that 
would be relevant. They are:


1. The *current* value the Security Manager to the Java ecosystem is high, 
evidenced by the great number of 
   companies that employ it for software security.

2. The Security Manager has the highest ROI compared to all other approaches.

Our assumption is that both 1 and 2 are false. If you’d like to convince the 
maintainers, support one or both 
of these arguments. Any other is irrelevant, because true or not it has no 
bearing on the proposal.

Short of that, my intention is not to convince you, because I agree that this 
proposal would harm you (even
though I believe it will help increase security for the Java ecosystem as a 
whole), and for that, I am truly 
sorry. My intention is to try to focus on productive avenues that might lessen 
that harm.


— Ron

Re: [External] : Re: JEP411: Missing use-case: Monitoring / restricting libraries

[Dalibor Topic]

On 21.05.2021 03:40, Peter Firmstone wrote:
If there are those of us who wanted to maintain a fork of Java 17, 
focused on security, we could backport new features after they've been 
reviewed for security.


Would we be welcomed to do that here?  Otherwise is it something we 
should do on GitHub?


Once a Project Lead resigns, or in the case of a repository in the JDK 
Updates Project, a repository lead maintainer resigns, someone 
sufficiently qualified to do so is welcome to step up and continue 
maintaining a release under the umbrella of the JDK Updates Project.


Please see http://openjdk.java.net/projects/jdk-updates/maintainers.html 
for details, and keep in mind that as any process, it's open to change 
through discussion on the jdk-updates-dev mailing list.


Ideas like differently focused repositories were discussed for JDK 11 in 
the past on that list, but the discussion was inconclusive, fwiw. It's 
worth keeping in mind that adequately maintaining an update release is 
by no means a trivial affair, and as such, there is usually little 
desire in the long run to balkanize the comparatively sparse community 
of OpenJDK update release maintainers across vastly different 
approaches, even if that may initially seem appealing for various reasons.


cheers,
dalibor topic


--
 Dalibor Topic
Consulting Product Manager
Phone: +494089091214 , Mobile: +491737185961
, Video: dalibor.to...@oracle.com


Oracle Global Services Germany GmbH
Hauptverwaltung: Riesstr. 25, D-80992 München
Registergericht: Amtsgericht München, HRB 246209
Geschäftsführer: Ralf Herrmann

Re: [External] : Re: JEP411: Missing use-case: Monitoring / restricting libraries

[Peter Firmstone]

On 21/05/2021 6:58 pm, Ron Pressler wrote:
These are just opinions, it's nice to have them, but they're not 
helping. I think you'll find usages of SecurityManager api's are far 
more widespread than your opinion suggests, but that's just my 
opinion too lol.
I guess you could say they’re my opinions because there is no 
conclusive proof, but they are based
on empirical data. At least you should scan GitHub and Maven Central. 
That few libraries are properly
instrumented is yet another piece of evidence. This isn’t conclusive, 
and that is why we’re discussing
this here in an attempt to collect more information, but we have done 
our homework.



Can you share this list please?   If I see anything missing I can add it 
for you.





Yes, I think we should keep it simple, and keep calling it the 
principle of least privilege, to avoid confusion, it's very 
beneficial for reducing the consequences of perimeter security 
failure, even if your experts think otherwise.

It is absolutely not the principle of least privilege,


So I'm restricting permissions granted to only those required to perform 
the intended tasks of the software,  restricted even to a subset of 
which that software is capable, and you say it is not the principle of 
least privilege?


Please refer this policy file: 
https://www.dropbox.com/s/0w1c140zts93cmw/defaultnonactvm-policy.txt?dl=0


It's even specific to the JVM used, vendor, version and installation path.


Notice how the attached grants permission to code and principal? That 
means the code without the Principal cannot attain that privilege, nor 
can the Principal use that permission without the specified code?


This means if an attacker breaches peripheral protection systems, such 
as authentication, they can not perform anything that requires 
permission outside of the granted scope?


I execute exactly the code paths I want executed, with the user role I 
want to perform that task, I grant only the permissions required to 
perform the intended task.


How is this absolutely not the principle of least privilege?


Java (the last remaining mainstream platform
with a mechanism that assigns permissions on a per-class basis).


Java Policy doesn't assign permissions on a per-class basis, it assigns 
them by ProtectionDomain, or by ClassLoader, or by CodeSource, or by 
Signer Certificates, or by Principal, or by a combination of Principal's 
and (ProtectionDomain or ClassLoader or CodeSource, or signer 
Certificates) and Certificates.  I don't really want to document all the 
possibilities here if yo don't mind.


How do you critique something if you don't understand how it works?

With test cases, I can generate policy files for the principle of least 
privilege for any Java software, including dependencies.


Not only that, but I do it without impacting performance or scalability.

Show me some evidence for your arguments, name your experts you speak of 
so often.


If you are going to argue, back it up with hard evidence, not nonsense.

I provide you with hard evidence, show me the same courtesy and respect 
please.


I'm not seeing any evidence that you have done much homework at all sir?

Previously you claimed SecurityManager didn't even work, that a 
Doctorate in Nuclear particle physics was required to use it.


I don't wish to publicly show that I doubt your credibility, but you are 
making it difficult for me not to sir.  It is your user base you need to 
convince, so far, you are not very convincing.



--
Respectfully,

Peter Firmstone

Re: [External] : Re: JEP411: Missing use-case: Monitoring / restricting libraries

[Ron Pressler]

> On 21 May 2021, at 04:55, Peter Firmstone  wrote:
> 
> 
> Yes everything is a compromise and there are trade-offs.   The way I see it, 
> the cheapest way to maintain security is to find others who share a common 
> pain point is to maintain a copy of OpenJDK  focused on security.   We can 
> audit upstream changes and add them later, with the security cost penalty, 
> without throwing out the huge amount of effort performed by JDK developers 
> over the years, there's no way we can re-implement it economically, so this 
> appears the most sensible option.
> 

That may be the way you see it, but I think you have to accept that many 
security experts don’t think
that focusing on the Security Manager and focusing on security are the same 
thing. We want to remove the 
SM *because* we want to focus on security measures that will have a larger 
impact in the real world. Even 
if you think that the SM is the best path to Java security you must accept that 
others do not, and that 
even if we’re misguided, we’re doing this because we believe it will help 
improve security, and that the 
SM harms it. So if you want to maintain the Security Manager in the JDK and 
ecosystem libraries, you can 
say so, but saying that that is “focusing on security” is a mischaracterisation 
because it depends on
assertions that you know are controversial at the very least.

> 
> These are just opinions, it's nice to have them, but they're not helping.   I 
> think you'll find usages of SecurityManager api's are far more widespread 
> than your opinion suggests, but that's just my opinion too lol.

I guess you could say they’re my opinions because there is no conclusive proof, 
but they are based
on empirical data. At least you should scan GitHub and Maven Central. That few 
libraries are properly 
instrumented is yet another piece of evidence. This isn’t conclusive, and that 
is why we’re discussing
this here in an attempt to collect more information, but we have done our 
homework.

> 
> Yes, I think we should keep it simple, and keep calling it the principle of 
> least privilege, to avoid confusion, it's very beneficial for reducing the 
> consequences of perimeter security failure, even if your experts think 
> otherwise.

It is absolutely not the principle of least privilege, which was formed long 
before the Security Manager
existed and is applied for all software, even that not written in Java (the 
last remaining mainstream platform 
with a mechanism that assigns permissions on a per-class basis). It is true 
that the SM allows smaller
units than the process or the user for the purpose of permissions, but if you 
think the principle means that
least privilege must be assigned at the smallest unit theoretically possible, 
then the Security Manager
doesn’t do that, and if you think it’s a matter of picking an effective unit 
granularity, then I believe
other measures apply it more effectively.

But there is something you keep missing here, and that is that the question of 
whether or not there are
circumstances in which the Security Manager will stop an attack — and there are 
— is irrelevant to the 
question of whether it is an effective security mechanism. That an elaborate 
trap will stop intruders who 
happen to try to enter my property at a specific spot doesn’t make it an 
effective mechanism, because a 
fence surrounding my property will stop even more intruders and do it more 
cheaply. “Beneficial” for us 
doesn’t mean “might help”, but what is the amount of total help offered per 
unit cost *compared to all 
other mechanisms’ benefit per cost*. In other words, we ask ourselves, is the 
Security Manager the best 
thing to invest in if we want security compared to all other things we can 
invest in?

If your codebase is trusted, then what you want to defend against are 
accidental vulnerabilities, 
i.e. bugs. Suppose your application requires permissions X and Y to run. What 
the Security Manager 
allows you is to notice that your application is made of two components, A and 
B, and after analysis 
conclude that while A requires permissions X and Y, B requires only X, and to 
grant only X to B. This 
can preemptively prevent exploits in your application that: 1. target component 
B and 2. require permission 
X. How much is preventing this particular scenario worth and are there ways to 
prevent more?

Just as the Security Manager doesn’t find it a good tradeoff to separate A and 
B at a method granularity, 
we don’t think it’s a good tradeoff to separate them at a class granularity. 
Grant your entire process 
permissions X and Y, and monitor it. Modern detection methods can automatically 
detect an unusual behaviour 
if B attempts Y and alert you to fix the bug, and we prefer investing the very 
high effort of maintaining
the SM at reducing the attack surface area of the JDK overall, reducing the 
chance of attacks on either A 
or B, whether they require permissions X or Y, and also being always on for 
everyone.