search

jakartaee / rest

Jakarta RESTful Web Services
Other
353 stars 114 forks source link

TCK failures : 2 SeBootstrapIT tests fail in Linux & mac due to permission issue #1088

Closed alwin-joseph closed 2 years ago

alwin-joseph commented 2 years ago

2 below SeBootstrapIT tests fail when run in Linux and Mac for Jersey(M2)

with the below errors.

[ERROR] ee.jakarta.tck.ws.rs.sebootstrap.SeBootstrapIT.shouldBootInstanceUsingImplementationsDefaultIpPort  Time elapsed: 0.055 s  <<< ERROR!
java.util.concurrent.ExecutionException: jakarta.ws.rs.ProcessingException: Failed to start Grizzly HTTP server: Permission denied
        at ee.jakarta.tck.ws.rs.sebootstrap.SeBootstrapIT.shouldBootInstanceUsingImplementationsDefaultIpPort(SeBootstrapIT.java:314)
Caused by: jakarta.ws.rs.ProcessingException: Failed to start Grizzly HTTP server: Permission denied
Caused by: **java.net.SocketException: Permission denied**

[ERROR] ee.jakarta.tck.ws.rs.sebootstrap.SeBootstrapIT.shouldBootInstanceUsingDefaults  Time elapsed: 0.042 s  <<< ERROR!
java.util.concurrent.ExecutionException: jakarta.ws.rs.ProcessingException: Failed to start Grizzly HTTP server: Permission denied
        at ee.jakarta.tck.ws.rs.sebootstrap.SeBootstrapIT.shouldBootInstanceUsingDefaults(SeBootstrapIT.java:73)
Caused by: jakarta.ws.rs.ProcessingException: Failed to start Grizzly HTTP server: Permission denied
Caused by: **java.net.SocketException: Permission denied**

The tests were reported as passing in Windows.

The issue was fixed by running sudo /sbin/sysctl -w net.ipv4.ip_unprivileged_port_start=0 in linux machine to allow non root users to have access to all ports. Is there any change required in the test code to get the test pass without the above workaround? Should we be using a port number >1024 to run these 2 tests?

@mkarg @jansupol @spericas

jansupol commented 2 years ago

I believe so. See https://www.w3.org/Daemon/User/Installation/PrivilegedPorts.html

mkarg commented 2 years ago

Nope. The intention of this tests is to proof that auto-selection is working well. So they MUST use port 0.

alwin-joseph commented 2 years ago

Nope. The intention of this tests is to proof that auto-selection is working well. So they MUST use port 0.

Ok. In that case what additional change is required for passing the test in non-Windows machines. Should there be a test change to allow non root users to have access to all ports ? Or are the implementations expected to fix this at their end for accomplishing the auto-selection.

mkarg commented 2 years ago

JAX-RS does not restrict the algorithm or port range in any way, it simply instructs the tested product to pick a free port of its own choice. If the tested product decides to start on a port that actually is restricted, the vendor should either take care to run the TCK as admin always, or change the product implementation to pick a non-restricted port instead. This has nothing to do with JAX-RS, neither the API, spec nor TCK, but solely with what the vendor wants it to happen.

jansupol commented 2 years ago

It is not an implementation that picks the port, it is the test. I thought about some modification such as (assumingly the same port is not chosen again and again):

private static final int someFreeIpPort() throws IOException {
    int port = 0;
    while (port < 1024) {
        try (final ServerSocket serverSocket = new ServerSocket(0)) {
            port = serverSocket.getLocalPort();
        }
    }
    return port;
}

I agree that it is odd that a restricted port is chosen.

jansupol commented 2 years ago

Just remember, we run the TCK in the Eclipse CI. We cannot use admin privileges there.

mkarg commented 2 years ago

It is not an implementation that picks the port, it is the test.

Good catch! I actually missed that!

I think it is best to fix it in the way you proposed -- even if I think that it actually should be fixed by the JRE...

Maybe it would be a good idea to refactor this method so other tests can make use of it, too?

jansupol commented 2 years ago

Good idea. There is a util folder for common utility methods, it can be used for methods like this.

mkarg commented 2 years ago

Good, so I will do that.

mkarg commented 2 years ago

With PR #1090 the TCK test now uses a utility method that will bear an unprivileged IP port. Note that this only fixed the TCK test. For ports discovered by implementations, still it is up to the implementation to fix the same problem (unless already done).

I also filed bug report JDK-8280919 for this weird / inconsistent JRE behavior.

alwin-joseph commented 2 years ago

I see the modified someFreeIpPort() method in PR https://github.com/eclipse-ee4j/jaxrs-api/pull/1090 is not used for below failing tests : SeBootstrapIT.shouldBootInstanceUsingDefaults SeBootstrapIT.shouldBootInstanceUsingImplementationsDefaultIpPort

Does these tests use ports discovered by implementations?

These tests still fail for me when run against jersey-3.1.0-M2 .

mkarg commented 2 years ago

Both tests deliverately instruct the implementation to chose the port, so it is up to the vendor of the tested implementation to either solve this problem within his product. Please also follow the discussion of the actual cause JRE-8280919: They ask for a check on the original causing host. @alwin-joseph Please provide the needed information to the OpenJDK team. Thanks.

alwin-joseph commented 2 years ago

Hi Markus -

I think there are 2 issues here :

  1. 2 failing Bootstrap tests
    • ee.jakarta.tck.ws.rs.sebootstrap.SeBootstrapIT.shouldBootInstanceUsingImplementationsDefaultIpPort
    • ee.jakarta.tck.ws.rs.sebootstrap.SeBootstrapIT.shouldBootInstanceUsingDefaults

AFAIS both of these tests do not use any custom method to obtain a free unprivileged IP port and hence do not use ServerSocket(0). (Please correct me if wrong). The issues are at these lines:

For test shouldBootInstanceUsingImplementationsDefaultIpPort I used port 1030 instead of SeBootstrap.Configuration.DEFAULT_PORT as below and it passed (it breaks the intention of the test, but I was just checking) 

        final SeBootstrap.Configuration requestedConfiguration = bootstrapConfigurationBuilder.protocol("HTTP")
                .host("localhost").port(1030).rootPath("/root/path").build();

Hi @jansupol Are the ports chosen by Jersey/vendor here when SeBootstrap.Configuration.DEFAULT_PORT is chosen as the port number ?

  1. Some Bootstrap tests that uses someFreeIpPort()/unprivilegedPort()/ServerSocket(0) method to get a free unprivileged port. The PR https://github.com/eclipse-ee4j/jaxrs-api/pull/1090 covers this but I have not seen any issue with these tests yet. I have not been able to confirm myself that ServerSocket(0) returns a port < 1024 too.
jansupol commented 2 years ago

~Jersey depends on Grizzly to choose the port if -1. We may change this for M3.~

Eh wrong. Jersey uses 443 for HTTPS and 80 for HTTP when starting Grizzly with -1.

mkarg commented 2 years ago

In fact this proofs that the test itself is correct! The problem you experience is that Jersey intentionally chooses privileged ports 443/80 by default. To pass the tests on you machine, either the machine must allow that or Jersey must use unprivileged defaults.

mkarg commented 2 years ago

As this is not a bug of the TCK but a problem of the specific execution environment and product under test, can we close this issue? The solution either is (a) fixing the bug in the JRE, (b) fixing the bug in the tested product, or (c) using containers for testing so root in the test is not root on the host.

alwin-joseph commented 2 years ago 
2. Some Bootstrap tests that uses someFreeIpPort()/unprivilegedPort()/ServerSocket(0)  method to get a free unprivileged port. The PR [Fixed #1088 #1090](https://github.com/eclipse-ee4j/jaxrs-api/pull/1090) covers this but I have not seen any issue with these tests yet. I have not been able to confirm myself that ServerSocket(0) returns a port < 1024 too.

Hi Markus - Just highlighting this again.

The 2 failing tests does not use someFreeIpPort()/unprivilegedPort()/ServerSocket(0) method. I do not see any issue with the ServerSocket/JRE here as those tests using it in the TCK are passing. Infact I could see that ServerSocket returned port >1024 for the other tests at least when I tried. Hence I cannot reproduce or justify the JDK bug you created. Apologies if I was not explicit earlier.

My concern was only the 2 failing tests which uses default ports and not the port number generated by someFreeIpPort(). The solutions for these 2 failing tests are probably

(b) fixing the bug in the tested product, or (c) using containers for testing so root in the test is not root on the host.

and not

(a) fixing the bug in the JRE

mkarg commented 2 years ago

@alwin-joseph I need to disagree.

I hope it is clear now that both has nothing to do with the TCK, but only with either the JRE, Jersey or the particular machine you run the test on.

alwin-joseph commented 2 years ago

Sorry I had few more questions.

  • FREE_PORT: The intention of this test is to proof that the tested product will start correctly when the product selected a free port of its own choice. Apparently the tested product choose a port on which it actually cannot run on that host, so the TCK test fails deliberately to proof that the product selects a wrong port.

Can you please list which are those tests in SeBootstrapIT where the product will select a free port of its own choice and failed with Jersey (before PR 1090)?

The default port (zero) instructs the JRE (via the tested product, here: Jersey via Grizzly) to auto-select a free port by using ServerSocket(0) internally (inside Grizzly). I cannot see why it shall not be a bug of the JRE that the JRE returns a privileged port. Can you please elaborate why you think that it is fine for the JRE to return a privileged port to Grizzly on that host while the OpenJDK team accepts that this is a bug of the JRE? So can you please provided the information to the OpenJDK team requested in https://bugs.openjdk.java.net/browse/JDK-8280919?

I agree that if ServerSocket(0) returns a privileged port then it is an unexpected behaviour(JRE bug). But I haven't been able to reproduce this case.

  • DEFAULT_PORT: The intention of this test is to proof that the tested product will start correctly when the product selected its own default port of its own choice. Neither the TCK nor the JAX-RS Spec mandate that privileged ports neither MUST nor SHOULD be privileged ports. The tested product (here: Jersey via GlassFish) deliberately uses privileged ports here to be RFC compliant. It is the product's decision to use 443/80, and it is not a bug at all. To make that test run on that particular host, either Jersey needs explicit configuration outside of this TCK test so it will choose a different default port, or the person that wants to run the TCK against Jersey on that machine has to configure that machine to use containers. It is not something to be changed by the TCK, as the TCK test deliberatey fails here to proof that Jersey cannot run in this environment because Jersey selected an invalid default port on that machine.

Agree on this. From what I understand below are the two tests where the product selects default port of its choice. -shouldBootInstanceUsingDefaults -shouldBootInstanceUsingImplementationsDefaultIpPort These tests do not use ServerSocket indirectly or directly, instead the ports 443/80 are chosen by Jersey here and hence they fail. Please correct me if otherwise.

mkarg commented 2 years ago
  • FREE_PORT: The intention of this test is to proof that the tested product will start correctly when the product selected a free port of its own choice. Apparently the tested product choose a port on which it actually cannot run on that host, so the TCK test fails deliberately to proof that the product selects a wrong port.

Can you please list which are those tests in SeBootstrapIT where the product will select a free port of its own choice and failed with Jersey (before PR 1090)?

I cannot tell you where Jersey failed as I did not test Jersey but you did. So I tell you the tests where any tested implementation could fail due to selecting a free port. This is at least SeBootstrapIT::shouldBootInstanceUsingSelfDetectedFreeIpPort, as it enforces the selection of a free port by the tested implementation. In case the tested implementation does not have statically fixed defaults but instead opts for dynamically selecting a free port as its default, also SeBootstrapIT::shouldBootInstanceUsingDefaults and SeBootstrapIT::shouldBootInstanceUsingImplementationsDefaultIpPort as both ask the tested implementation to use its defaults (the first one implicitly by not specifying a port, the second one explicitly by specifying DEFAULT_PORT).

The default port (zero) instructs the JRE (via the tested product, here: Jersey via Grizzly) to auto-select a free port by using ServerSocket(0) internally (inside Grizzly). I cannot see why it shall not be a bug of the JRE that the JRE returns a privileged port. Can you please elaborate why you think that it is fine for the JRE to return a privileged port to Grizzly on that host while the OpenJDK team accepts that this is a bug of the JRE? So can you please provided the information to the OpenJDK team requested in https://bugs.openjdk.java.net/browse/JDK-8280919?

I agree that if ServerSocket(0) returns a privileged port then it is an unexpected behaviour(JRE bug). But I haven't been able to reproduce this case.

You should have, as this is what actually caused your test drive to fail in the first place.

  • DEFAULT_PORT: The intention of this test is to proof that the tested product will start correctly when the product selected its own default port of its own choice. Neither the TCK nor the JAX-RS Spec mandate that privileged ports neither MUST nor SHOULD be privileged ports. The tested product (here: Jersey via GlassFish) deliberately uses privileged ports here to be RFC compliant. It is the product's decision to use 443/80, and it is not a bug at all. To make that test run on that particular host, either Jersey needs explicit configuration outside of this TCK test so it will choose a different default port, or the person that wants to run the TCK against Jersey on that machine has to configure that machine to use containers. It is not something to be changed by the TCK, as the TCK test deliberatey fails here to proof that Jersey cannot run in this environment because Jersey selected an invalid default port on that machine.

Agree on this. From what I understand below are the two tests where the product selects default port of its choice. -shouldBootInstanceUsingDefaults -shouldBootInstanceUsingImplementationsDefaultIpPort These tests do not use ServerSocket indirectly or directly, instead the ports 443/80 are chosen by Jersey here and hence they fail. Please correct me if otherwise.

Correct.

alwin-joseph commented 2 years ago
  • FREE_PORT: The intention of this test is to proof that the tested product will start correctly when the product selected a free port of its own choice. Apparently the tested product choose a port on which it actually cannot run on that host, so the TCK test fails deliberately to proof that the product selects a wrong port.

Can you please list which are those tests in SeBootstrapIT where the product will select a free port of its own choice and failed with Jersey (before PR 1090)?

I cannot tell you where Jersey failed as I did not test Jersey but you did. So I tell you the tests where any tested implementation could fail due to selecting a free port. This is at least SeBootstrapIT::shouldBootInstanceUsingSelfDetectedFreeIpPort, as it enforces the selection of a free port by the tested implementation.

This test and all others except the SeBootstrapIT::shouldBootInstanceUsingDefaults and SeBootstrapIT::shouldBootInstanceUsingImplementationsDefaultIpPort passed when I ran with Jersey.

In case the tested implementation does not have statically fixed defaults but instead opts for dynamically selecting a free port as its default, also SeBootstrapIT::shouldBootInstanceUsingDefaults and SeBootstrapIT::shouldBootInstanceUsingImplementationsDefaultIpPort as both ask the tested implementation to use its defaults (the first one implicitly by not specifying a port, the second one explicitly by specifying DEFAULT_PORT).

Are you saying that even when these 2 tests - SeBootstrapIT::shouldBootInstanceUsingDefaults and SeBootstrapIT::shouldBootInstanceUsingImplementationsDefaultIpPort - are run that could mean a free port is used when statically fixed defaults are not present and hence the failure of these 2 tests could mean issue with ServerSocket(0) within the implementation ?

But Jersey does have default ports 443/80 specified (as mentioned before) hence IMO shouldBootInstanceUsingDefaults & shouldBootInstanceUsingImplementationsDefaultIpPort are not using ServerSocket(0). Am I right ?

The default port (zero) instructs the JRE (via the tested product, here: Jersey via Grizzly) to auto-select a free port by using ServerSocket(0) internally (inside Grizzly). I cannot see why it shall not be a bug of the JRE that the JRE returns a privileged port. Can you please elaborate why you think that it is fine for the JRE to return a privileged port to Grizzly on that host while the OpenJDK team accepts that this is a bug of the JRE? So can you please provided the information to the OpenJDK team requested in https://bugs.openjdk.java.net/browse/JDK-8280919?

I agree that if ServerSocket(0) returns a privileged port then it is an unexpected behaviour(JRE bug). But I haven't been able to reproduce this case.

You should have, as this is what actually caused your test drive to fail in the first place.

The only failure I reported were the tests shouldBootInstanceUsingDefaults & shouldBootInstanceUsingImplementationsDefaultIpPort for which I assumed the privileged ports 443/80 were direclty used as defaults from Jersey.

mkarg commented 2 years ago

In case the tested implementation does not have statically fixed defaults but instead opts for dynamically selecting a free port as its default, also SeBootstrapIT::shouldBootInstanceUsingDefaults and SeBootstrapIT::shouldBootInstanceUsingImplementationsDefaultIpPort as both ask the tested implementation to use its defaults (the first one implicitly by not specifying a port, the second one explicitly by specifying DEFAULT_PORT).

Are you saying that even when these 2 tests - SeBootstrapIT::shouldBootInstanceUsingDefaults and SeBootstrapIT::shouldBootInstanceUsingImplementationsDefaultIpPort - are run that could mean a free port is used when statically fixed defaults are not present and hence the failure of these 2 tests could mean issue with ServerSocket(0) within the implementation ?

Yes, the JAX-RS Bootstrap API allows such a behaviour. Having said that, IIUC what @jansupol told us, then Jersey de-facto will never do that but always use 443/80. But as this is a JAX-RS TCK discussion and not a Jersey discussion, I need to explain the whole truth.

But Jersey does have default ports 443/80 specified (as mentioned before) hence IMO shouldBootInstanceUsingDefaults & shouldBootInstanceUsingImplementationsDefaultIpPort are not using ServerSocket(0). Am I right ?

I confirm this is what @jansupol told us.

The only failure I reported were the tests shouldBootInstanceUsingDefaults & shouldBootInstanceUsingImplementationsDefaultIpPort for which I assumed the privileged ports 443/80 were direclty used as defaults from Jersey.

@spericas @jansupol So why did you ask me to change the TCK code to guarantee that this won't happen?

spericas commented 2 years ago

@spericas @jansupol So why did you ask me to change the TCK code to guarantee that this won't happen?

I was likely @jansupol. I believe he is on vacation this week.

mkarg commented 2 years ago

So how to proceed? I'd like to propose that I undo the unnecessary port checks, as that case never happend on any machine. Counterproposals?

spericas commented 2 years ago

@jansupol Can you can comment on this issue?

jamezp commented 2 years ago

I can say RESTEasy is passing the SeBootstrapIT as it is now. The test seems fine to me.

jansupol commented 2 years ago

Jersey 3.1.0-M3 does not use privileged ports any longer, ports 8080 and 8443 are used instead.

mkarg commented 2 years ago

Okay so as it is not actually needed, I will remove the explicit port number check again.

mkarg commented 2 years ago

Jersey 3.1.0-M3 does not use privileged ports any longer, ports 8080 and 8443 are used instead.

Off-Topic: Is there a public Maven repo where I can pull Jersey 3.1.0-M3 from?

arjantijms commented 2 years ago

It's in the default staging repo for Jakarta / EE4J projects:

https://jakarta.oss.sonatype.org/content/repositories/staging/org/glassfish/jersey/core/jersey-server/3.1.0-M3/

mkarg commented 2 years ago

As PR #1103 is merge, I think we can close this issue?

alwin-joseph commented 2 years ago

Issue fixed in Jersey 3.1 M3 version