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

2021-05-27 Thread Weijun Wang
On Fri, 28 May 2021 03:12:35 GMT, Phil Race  wrote:

>> There *is* a tiny refactoring here: a new variable `s2` is introduced so the 
>> 2nd `doPrivileged` call on line 636 is now also in a declaration statement 
>> (for `s2`) and therefore annotatable. Without this I cannot add the 
>> annotation on line 635.
>
> Ok. But I will quote you
> "This happens when a deprecated method is called inside a static block. The 
> annotation can only be added to a declaration and here it must be the whole 
> class"
> 
> So the way you explained this before made it sound like any time there was 
> any SM API usage in a static block, the entire class needed to be annotated.
> 
> Why has the explanation changed ?

I should have been more precise. An annotation can only be added on a 
declaration, whether it's a variable, a method, or a class. Static block is not 
a declaration and the only one covers it is the class. But then if it's on a 
local variable declaration inside a static block, we certainly can annotate on 
that variable.

-

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


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

2021-05-27 Thread Phil Race
On Fri, 28 May 2021 02:50:55 GMT, Weijun Wang  wrote:

>> src/java.desktop/share/classes/java/awt/Component.java line 630:
>> 
>>> 628: }
>>> 629: 
>>> 630: @SuppressWarnings("removal")
>> 
>> I'm confused. I thought the reason this wasn't done in the JEP 
>> implementation PR is because of refactoring
>> that was needed because of the usage in this static block and you could not 
>> apply the annotation here.
>> Yet it seems you are doing exactly what was supposed to be impossible with 
>> no refactoring.
>> Can you explain ?
>
> There *is* a tiny refactoring here: a new variable `s2` is introduced so the 
> 2nd `doPrivileged` call on line 636 is now also in a declaration statement 
> (for `s2`) and therefore annotatable. Without this I cannot add the 
> annotation on line 635.

Ok. But I will quote you
"This happens when a deprecated method is called inside a static block. The 
annotation can only be added to a declaration and here it must be the whole 
class"

So the way you explained this before made it sound like any time there was any 
SM API usage in a static block, the entire class needed to be annotated.

Why has the explanation changed ?

-

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


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

2021-05-27 Thread Weijun Wang
On Thu, 27 May 2021 17:42:56 GMT, Phil Race  wrote:

>> Weijun Wang has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   update FtpClient.java
>
> src/java.desktop/share/classes/java/awt/Component.java line 630:
> 
>> 628: }
>> 629: 
>> 630: @SuppressWarnings("removal")
> 
> I'm confused. I thought the reason this wasn't done in the JEP implementation 
> PR is because of refactoring
> that was needed because of the usage in this static block and you could not 
> apply the annotation here.
> Yet it seems you are doing exactly what was supposed to be impossible with no 
> refactoring.
> Can you explain ?

There *is* a tiny refactoring here: a new variable `s2` is introduced so the 
2nd `doPrivileged` call on line 636 is now also in a declaration statement (for 
`s2`) and therefore annotatable. Without this I cannot add the annotation on 
line 635.

-

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


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

2021-05-27 Thread Weijun Wang
On Thu, 27 May 2021 20:16:25 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 with a new target base due to a 
> merge or a rebase. The pull request now contains eight commits:
> 
>  - move one annotation to new method
>  - merge from master, two files removed, one needs merge
>  - keep only one systemProperty tag
>  - fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java
>  - feedback from Sean, Phil and Alan
>  - add supresswarnings annotations automatically
>  - manual change before automatic annotating
>  - 8266459: Implement JEP 411: Deprecate the Security Manager for Removal

Two files were removed by JEP 403 and JEP 407, respectively. One method in 
`XMLSchemaFactory.java` got [its 
own](https://github.com/openjdk/jdk/commit/8c4719a58834dddcea39d69b199abf1aabf780e2#diff-593a224979eaff03e2a3df1863fcaf865364a31a2212cc0d1fe67a8458057857R429)
 `@SuppressWarnings` and have to be merged with the one here. Another file 
`ResourceBundle.java` had a portion of a method extracted into a [new 
method](https://github.com/openjdk/jdk/commit/a4c46e1e4f4f2f05c8002b2af683a390fc46b424#diff-59caf1a68085064b4b3eb4f6e33e440bb85ea93719f34660970e2d4eaf8ce469R3175)
 and the annotation must be moved there.

-

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


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

2021-05-27 Thread Chapman Flack
Hello, I see I am another person relatively late to stumble on this
"well publicized" JEP. (I am not sure how to recommend the publicity
could have been better handled, but apparently the avenues that were
used aren't ones that reached me.)

I maintain, on a volunteer basis, the extension for Java server-side
functions in the PostgreSQL RDBMS [1].

I got the news of this JEP through occasionally visiting the JDK 17
page to see what JEPs are proposed and targeted that might be of use
in the project, and one day I visited (could have been as late as May 17
according to the Wayback Machine) and there was nothing of great
interest (though I have some pleasant ideas for sealed classes and I
really wish JEP 412 were done incubating!), and I didn't check back
until just recently, and then I looked and saw this JEP.

I've read through the very long "Missing use-case: monitoring/
restricting libraries" thread, and some of the points raised there
have echoes here.

Any PostgreSQL server-side function implementation for a language foo
will be expected to say it is a "trusted" or "untrusted" language (or
provide both) as defined in the PostgreSQL docs [2].

A "trusted" one is expected to restrict certain actions (access to the
server filesystem, perhaps network connections, etc.).

OS-level controls are too coarse because the RDBMS process that the
language extension gets dlopened into certainly has reasons of its own
to manipulate files and sockets.

It might, perhaps, be shown that every available trusted language for
PostgreSQL could be imperfect or exploitable in some way; I think that's
beside the point, which is that they all are meant to take credible
steps to supply the expected layer of cheese with whatever small holes
may be present.

In the current architecture, Java backend functions can be restricted
with very fine grain, anything that a PolicyFile can specify.

It looks as if I will have to do a maintenance release, which will
have to supply the extra -Djava.security.manager=allow when running
on 17 (and spam the RDBMS log file with the apparently unsuppressable
warning every time a JVM starts [3]), and then I will have to detect
whatever subsequent Java release "degrades" the classes, and refuse to
execute trusted functions on that release or later.

Beyond that, I'll need to begin on some rearchitected major release
to be able to meet the requirements some other way.

Suppose I relax the requirements to merely restricting filesystem
and network operations. Will there be any simple, reliable way for
me to install some handler to filter those? I have seen JVMTI
instrumentation suggested. I suppose an interposing FileSystemProvider
could be an option for filesystem operations. SocketFactory and
ServerSocketFactory might offer ways to interpose on network
operations, except that there seems to be no documentation of how
the default factories are to be set: "specified by environment-specific
configuration mechanisms ... a framework could use a factory customized
for its own purposes" doesn't seem quite sufficient. I could be considered
to be providing a framework: is the expectation that I must read the
source and then JNI-poke an undocumented static in order to set a factory?

Assuming that hurdle cleared, socket restrictions might not be too bad.
The JRE probably doesn't do a lot of network operations on its own behalf,
so a "simple deep sandbox" in Ron's taxonomy is sufficient there; whatever
operations are being initiated will be coming from the application code.

That's not even remotely true for file operations though: the Java runtime
does stuff with files! Early PL/Java versions that tried to make do with a
"simple" SecurityManager (devoid of the AccessController and stack
awareness) clearly demonstrated the problem, falling over because the
runtime needed to open /dev/random to make an SSL connection, or timezone
files to print a date, or write temporary files in compiling a Templates
object, or load agent classes to accept a visualvm connection.

Those problems all were solved by axing the "simple" SecurityManager and
reverting to the J2SE one with stack awareness. All those things then
Just Worked.

The reason, of course, is that those doPrivileged calls throughout
the implementation are carriers of vital information about which operations
are "taint-free" Java runtime internal details and which ones are coming
from the application. It isn't the application developer's business to know
that TransformerFactory.newTemplates() needs to scribble transparently
in some files. That vital information is known by the developers working
on the JDK and is still needed for any sane out-of-JDK reimplementation
of filesystem access controls.

I'm sensitive to the argument that lots of third-partly libraries have
always whiffed on putting their own doPrivileged calls in the right
places, so application developers have had to clean up after them anyway,
and that will only get worse after this JEP. But it still s

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

2021-05-27 Thread Weijun Wang
> 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 with a new target base due to a merge 
or a rebase. The pull request now contains eight commits:

 - move one annotation to new method
 - merge from master, two files removed, one needs merge
 - keep only one systemProperty tag
 - fixing awt/datatransfer/DataFlavor/DataFlavorRemoteTest.java
 - feedback from Sean, Phil and Alan
 - add supresswarnings annotations automatically
 - manual change before automatic annotating
 - 8266459: Implement JEP 411: Deprecate the Security Manager for Removal

-

Changes: https://git.openjdk.java.net/jdk/pull/4073/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4073&range=04
  Stats: 2022 lines in 825 files changed: 1884 ins; 10 del; 128 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4073.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4073/head:pull/4073

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


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

2021-05-27 Thread Phil Race
On Fri, 21 May 2021 20:37:30 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 if not void. The local variable will be called `tmp` 
>> if later reassigned or `dummy` if it will be discarded.
>> 3. Put declaration and assignment into a single statement if possible.
>> 4. Rewrite code to achieve #3 above.
>> 
>> I'll add a copyright year update commit before integration.
>
> Weijun Wang has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   update FtpClient.java

src/java.desktop/share/classes/java/awt/Component.java line 630:

> 628: }
> 629: 
> 630: @SuppressWarnings("removal")

I'm confused. I thought the reason this wasn't done in the JEP implementation 
PR is because of refactoring
that was needed because of the usage in this static block and you could not 
apply the annotation here.
Yet it seems you are doing exactly what was supposed to be impossible with no 
refactoring.
Can you explain ?

-

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


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

2021-05-27 Thread Phil Race
On Mon, 24 May 2021 13:53:34 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:
> 
>   keep only one systemProperty tag

Marked as reviewed by prr (Reviewer).

I'm OK with this now given that the refactoring is already underway at 
https://github.com/openjdk/jdk/pull/4138

-

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


Integrated: 8267123: Remove RMI Activation

2021-05-27 Thread Stuart Marks
On Tue, 25 May 2021 18:04:45 GMT, Stuart Marks  wrote:

> This is the implementation of [JEP 407](https://openjdk.java.net/jeps/407).
> 
> This is a fairly straightforward removal of this component.
>  - Activation implementation classes removed
>  - Activation tests removed
>  - adjustments to various comments to remove references to Activation
>  - adjustments to some code to remove special-case support for Activation 
> from core RMI
>  - adjustments to several tests to remove testing and support for, and 
> mentions of Activation
>  - remove `rmid` tool
> 
> (Personally, I found that reviewing using the webrev was easier than 
> navigating github's diff viewer.)

This pull request has now been integrated.

Changeset: 7c85f351
Author:Stuart Marks 
URL:   
https://git.openjdk.java.net/jdk/commit/7c85f3510cb84881ff232548fbcc933ef4b34972
Stats: 21982 lines in 243 files changed: 0 ins; 21930 del; 52 mod

8267123: Remove RMI Activation

Reviewed-by: erikj, rriggs, alanb

-

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


Re: RFR: 8267587: Update java.util to use enhanced switch [v6]

2021-05-27 Thread Daniel Fuchs
On Wed, 26 May 2021 02:22:42 GMT, Tagir F. Valeev  wrote:

>> Inspired by PR#4088. Most of the changes are done automatically using 
>> IntelliJ IDEA refactoring. Some manual adjustments are also performed, 
>> including indentations, moving comments, extracting common cast out of 
>> switch expression branches, etc.
>> 
>> I also noticed that there are some switches having one branch only in 
>> JapaneseImperialCalendar.java. Probably it would be better to replace them 
>> with `if` statement?
>
> Tagir F. Valeev has updated the pull request incrementally with two 
> additional commits since the last revision:
> 
>  - JapaneseImperialCalendar: use switch expressions
>  - Use yield in java.util.Calendar.Builder.build

Globally looks good.
I haven't reviewed `src/java.base/share/classes/java/util/regex/Pattern.java` 
due to formatting issues. See also my other remark about `java.util.concurrent`.

src/java.base/share/classes/java/util/concurrent/FutureTask.java line 495:

> 493:  * @return a string representation of this FutureTask
> 494:  */
> 495: public String toString() {

Classes in java.util.concurrent are handled upstream. It would probably be 
better to leave them out of this patch. Or synchronize with @DougLea to see how 
to bring these changes in the upstream repo.

-

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


Re: Fuzzing for java.security.* (and other libraries)

2021-05-27 Thread Fabian Meumertzheim
Hi Sean,


On Thu, May 27, 2021 at 2:35 PM Sean Mullan  wrote:

> Hi Fabian,
>
> Thanks for posting this and your interest in helping to test and improve
> the quality of the Java core libraries. One comment/request below:
>
> On 5/17/21 9:09 AM, Fabian Meumertzheim wrote:
>
> (Crosspost from core-libs-dev@:
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-May/077483.html
> )
>
> I'm one of the maintainers of Jazzer (
> https://github.com/CodeIntelligenceTesting/jazzer), a new open-source
> fuzzer for the JVM platform. Jazzer has recently been integrated into
> Google's OSS-Fuzz (https://google.github.io/oss-fuzz/) to allow for free
> continuous fuzzing of important open-source Java projects. Jazzer has
> already found over a hundred bugs and eight security issues in libraries
> such as Apache Commons, PDFBox and the OWASP json-sanitizer.
>
> Jazzer finds unexpected exceptions and infinite loops by default, but can
> also be used to check domain-specific properties such as
> decrypt(encrypt(data)) == data. Since it tracks the coverage it achieves
> using instrumentation applied by a Java agent, it can synthesize
> interesting test data from scratch.
>
> If there is interest from your side, I could set up the Java core
> libraries themselves for fuzzing in OSS-Fuzz. Especially the parts that are
> frequently applied to untrusted input, such as java.security.* and
> javax.imageio.*, would benefit from fuzz tests. I have prepared basic fuzz
> tests for some of the classes in these packages at
> https://github.com/CodeIntelligenceTesting/oss-fuzz/tree/openjdk/projects/openjdk,
> which has already resulted in a few bug reports by running it locally
> (JDK-8267086 is one of them affecting java.security.*).
>
> All I would need from you is:
>
> * a list of email addresses to which the fuzzer findings should be sent
> (ideally associated with Google accounts for authentication to full reports
> on oss-fuzz.com),
>
> All fuzzer findings with security implications should be sent to the
> OpenJDK Vulnerability Group. See
> https://openjdk.java.net/groups/vulnerability/report for more
> information. Please send the detailed information (description, impacted
> release, and PoC) to *vuln-rep...@openjdk.java.net
> *.
>

Just to clarify the role of OSS-Fuzz: The fuzzing and report filing would
be performed automatically. Since not every finding will necessarily have
security implications (but all will be actual bugs), I'm hesitant to have
these reports submitted to vuln-report@. Ideally, we would find two or
three humans that agree to receive the findings reports and forward those
deemed security issues to that list.

Best,
Fabian

>
> Thanks,
> Sean
>
> * ideas for additional fuzz tests, in particular those where there are
> interesting properties to verify.
>
> The technical questions about setting up the OpenJDK in OSS-Fuzz have
> already been resolved (see also
> https://github.com/google/oss-fuzz/issues/5757).
>
> If you need more information on OSS-Fuzz or fuzzing in general, I am happy
> to help.
>
> Fabian (@fmeum on GitHub)
>
>
>


Re: Fuzzing for java.security.* (and other libraries)

2021-05-27 Thread Sean Mullan

Hi Fabian,

Thanks for posting this and your interest in helping to test and improve 
the quality of the Java core libraries. One comment/request below:


On 5/17/21 9:09 AM, Fabian Meumertzheim wrote:
(Crosspost from core-libs-dev@: 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-May/077483.html 
)


I'm one of the maintainers of Jazzer 
(https://github.com/CodeIntelligenceTesting/jazzer 
), a new 
open-source fuzzer for the JVM platform. Jazzer has recently been 
integrated into Google's OSS-Fuzz (https://google.github.io/oss-fuzz/ 
) to allow for free continuous 
fuzzing of important open-source Java projects. Jazzer has already 
found over a hundred bugs and eight security issues in libraries such 
as Apache Commons, PDFBox and the OWASP json-sanitizer.


Jazzer finds unexpected exceptions and infinite loops by default, but 
can also be used to check domain-specific properties such as 
decrypt(encrypt(data)) == data. Since it tracks the coverage it 
achieves using instrumentation applied by a Java agent, it can 
synthesize interesting test data from scratch.


If there is interest from your side, I could set up the Java core 
libraries themselves for fuzzing in OSS-Fuzz. Especially the parts 
that are frequently applied to untrusted input, such as 
java.security.* and javax.imageio.*, would benefit from fuzz tests. I 
have prepared basic fuzz tests for some of the classes in these 
packages at 
https://github.com/CodeIntelligenceTesting/oss-fuzz/tree/openjdk/projects/openjdk 
, 
which has already resulted in a few bug reports by running it locally 
(JDK-8267086 is one of them affecting java.security.*).


All I would need from you is:

* a list of email addresses to which the fuzzer findings should be 
sent (ideally associated with Google accounts for authentication to 
full reports on oss-fuzz.com ),
All fuzzer findings with security implications should be sent to the 
OpenJDK Vulnerability Group. See 
https://openjdk.java.net/groups/vulnerability/report 
 for more 
information. Please send the detailed information (description, impacted 
release, and PoC) to /vuln-rep...@openjdk.java.net 
/.


Thanks,
Sean
* ideas for additional fuzz tests, in particular those where there are 
interesting properties to verify.


The technical questions about setting up the OpenJDK in OSS-Fuzz have 
already been resolved (see also 
https://github.com/google/oss-fuzz/issues/5757 
).


If you need more information on OSS-Fuzz or fuzzing in general, I am 
happy to help.


Fabian (@fmeum on GitHub)




Integrated: 8267817: [TEST] Remove unnecessary init in test/micro/org/openjdk/bench/javax/crypto/full/AESGCMBench:setup

2021-05-27 Thread Dongbo He
On Thu, 27 May 2021 02:11:59 GMT, Dongbo He  wrote:

> decryptCipher will be reinitialized in decrypt, which will loses all 
> previously-acquired state. Therefore, it's not necessary to initialize in 
> setup.

This pull request has now been integrated.

Changeset: 85f61652
Author:Dongbo He 
Committer: Claes Redestad 
URL:   
https://git.openjdk.java.net/jdk/commit/85f616522b2dc8e7b4c31d760c3171ac74a5490f
Stats: 2 lines in 1 file changed: 0 ins; 2 del; 0 mod

8267817: [TEST] Remove unnecessary init in 
test/micro/org/openjdk/bench/javax/crypto/full/AESGCMBench:setup

Reviewed-by: redestad

-

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


Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v26]

2021-05-27 Thread Maurizio Cimadamore
> This PR contains the API and implementation changes for JEP-412 [1]. A more 
> detailed description of such changes, to avoid repetitions during the review 
> process, is included as a separate comment.
> 
> [1] - https://openjdk.java.net/jeps/412

Maurizio Cimadamore has updated the pull request with a new target base due to 
a merge or a rebase. The pull request now contains 37 commits:

 - Merge branch 'master' into JEP-412
 - * Add missing `final` in some static fields
   * Downgrade native methods in ProgrammableUpcallHandler to package-private
 - Add sealed modifiers in morally sealed API interfaces
 - Merge branch 'master' into JEP-412
 - Fix VaList test
   Remove unused code in Utils
 - Merge pull request #11 from JornVernee/JEP-412-MXCSR
   
   Add MXCSR save and restore to upcall stubs for non-windows platforms
 - Add MXCSR save and restore to upcall stubs for non-windows platforms
 - Merge branch 'master' into JEP-412
 - Fix issue with bounded arena allocator
 - Rename is_entry_blob to is_optimized_entry_blob
   Rename as_entry_blob to as_optimized_entry_blob
   Fix misc typos and indentation issues
 - ... and 27 more: https://git.openjdk.java.net/jdk/compare/7278f56b...8bbddfc2

-

Changes: https://git.openjdk.java.net/jdk/pull/3699/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3699&range=25
  Stats: 14501 lines in 219 files changed: 8847 ins; 3642 del; 2012 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3699.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3699/head:pull/3699

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


Re: RFR: 8180571: Refactor sun/security/pkcs11 shell tests to plain java tests and fix failures

2021-05-27 Thread Fernando Guallini
On Thu, 27 May 2021 09:55:10 GMT, Sibabrata Sahoo  wrote:

>> Refactor the following shell tests to Java:
>> - security/pkcs11/KeyStore/Basic.sh
>> - security/pkcs11/KeyStore/ClientAuth.sh
>> - security/pkcs11/KeyStore/SecretKeysBasic.sh
>> - security/pkcs11/Provider/ConfigQuotedString.sh
>> - security/pkcs11/Provider/Login.sh
>> - security/pkcs11/Config/ReadConfInUTF16Env.sh
>> 
>> Currently, most of the shell tests in the list may be ignored during 
>> execution time in most platforms since they are incorrectly filtered out by 
>> the OS name they are run on. For example, ClientAuth.sh is only run if the 
>> OS name is equal to ‘Linux’, but OS name may also include the architecture 
>> such as ‘Linux x86_64’. Those platform constraints are removed in this PR.
>> 
>> Additionally, further changes are introduced in the following test:
>> 
>> - ClientAuth: it was failing intermittently because the server side was 
>> binding to the wildcard address. The issue is fixed by binding to loopback 
>> address instead. Also, Thread.sleep is replaced with CountdownLatch to 
>> facilitate synchronization between client and server. Finally, a new ‘user1’ 
>> certificate was generated since the current one has expired.
>> 
>> - Basic: Remove redundant X509Certificate casting 
>> 
>> - SecretKeysBasic: it was already in the problem list since it reproduces 
>> the open bug JDK-8209398 and fails. Test is refactored to java and still 
>> reproduces the issue.
>> 
>> All the mentioned tests were run many times in multiple platforms to ensure 
>> stability
>
> test/jdk/sun/security/pkcs11/Config/ReadConfInUTF16Env.java line 25:
> 
>> 23: 
>> 24: /* @test
>> 25:  * @bug 8187023
> 
> Should it have the enhancement bug id too.

My understanding is that we should only include the id of product bugs, no test 
enhancements. That was a comment somebody added in a previous PR. The reason is 
that you can track changes in the test file with git history, whereas it is 
only possible to track the related product bug id by adding it manually to the 
class header

-

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


Re: RFR: 8264774: Implementation of Foreign Function and Memory API (Incubator) [v25]

2021-05-27 Thread Maurizio Cimadamore
> This PR contains the API and implementation changes for JEP-412 [1]. A more 
> detailed description of such changes, to avoid repetitions during the review 
> process, is included as a separate comment.
> 
> [1] - https://openjdk.java.net/jeps/412

Maurizio Cimadamore has updated the pull request incrementally with one 
additional commit since the last revision:

  * Add missing `final` in some static fields
  * Downgrade native methods in ProgrammableUpcallHandler to package-private

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3699/files
  - new: https://git.openjdk.java.net/jdk/pull/3699/files/e927c235..e1fcd2d3

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3699&range=24
 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3699&range=23-24

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

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


Re: RFR: 8267817: [TEST] Remove unnecessary init in test/micro/org/openjdk/bench/javax/crypto/full/AESGCMBench:setup

2021-05-27 Thread Claes Redestad
On Thu, 27 May 2021 02:11:59 GMT, Dongbo He  wrote:

> decryptCipher will be reinitialized in decrypt, which will loses all 
> previously-acquired state. Therefore, it's not necessary to initialize in 
> setup.

Looks good.

-

Marked as reviewed by redestad (Reviewer).

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


Re: RFR: 8180571: Refactor sun/security/pkcs11 shell tests to plain java tests and fix failures

2021-05-27 Thread Sibabrata Sahoo
On Tue, 18 May 2021 13:19:53 GMT, Fernando Guallini  
wrote:

> Refactor the following shell tests to Java:
> - security/pkcs11/KeyStore/Basic.sh
> - security/pkcs11/KeyStore/ClientAuth.sh
> - security/pkcs11/KeyStore/SecretKeysBasic.sh
> - security/pkcs11/Provider/ConfigQuotedString.sh
> - security/pkcs11/Provider/Login.sh
> - security/pkcs11/Config/ReadConfInUTF16Env.sh
> 
> Currently, most of the shell tests in the list may be ignored during 
> execution time in most platforms since they are incorrectly filtered out by 
> the OS name they are run on. For example, ClientAuth.sh is only run if the OS 
> name is equal to ‘Linux’, but OS name may also include the architecture such 
> as ‘Linux x86_64’. Those platform constraints are removed in this PR.
> 
> Additionally, further changes are introduced in the following test:
> 
> - ClientAuth: it was failing intermittently because the server side was 
> binding to the wildcard address. The issue is fixed by binding to loopback 
> address instead. Also, Thread.sleep is replaced with CountdownLatch to 
> facilitate synchronization between client and server. Finally, a new ‘user1’ 
> certificate was generated since the current one has expired.
> 
> - Basic: Remove redundant X509Certificate casting 
> 
> - SecretKeysBasic: it was already in the problem list since it reproduces the 
> open bug JDK-8209398 and fails. Test is refactored to java and still 
> reproduces the issue.
> 
> All the mentioned tests were run many times in multiple platforms to ensure 
> stability

test/jdk/sun/security/pkcs11/Config/ReadConfInUTF16Env.java line 25:

> 23: 
> 24: /* @test
> 25:  * @bug 8187023

Should it have the enhancement bug id too.

-

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


RFR: 8179880: Refactor javax/security shell tests to plain java tests

2021-05-27 Thread Sibabrata Sahoo
This change converts JTREG shell Test scripts to Java equivalent.

-

Commit messages:
 - 8179880: Refactor javax/security shell tests to plain java tests

Changes: https://git.openjdk.java.net/jdk/pull/4220/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=4220&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8179880
  Stats: 99 lines in 2 files changed: 11 ins; 80 del; 8 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4220.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4220/head:pull/4220

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