Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable rpc.enhancedClasses by default at runtime #9879

Merged
merged 3 commits into from
Dec 23, 2023

Conversation

niloc132
Copy link
Member

Logs warnings at compile time, indicating which classes need to be cleaned up to remove this feature.

Mitigation for #9709

Logs warnings at compile time, indicating which classes need to be
cleaned up to remove this feature.

Mitigation for gwtproject#9709
@niloc132
Copy link
Member Author

The good news is that all tests passed with this change https://github.com/niloc132/gwt/actions/runs/7267798597

The bad news is that this means that the rpc.enhancedClasses feature apparently has no tests - probably only bad news in retrospect, given that we are trying to turn it off.

@niloc132
Copy link
Member Author

In light of #9709 (comment), it might be wise to generalize these error messages slightly, and if the developer doesn't believe that JPA/JDO is in use, to just remove the annotations and see if that still works.

@niloc132
Copy link
Member Author

Testing to confirm the new behavior:

  1. Started with the DynaTable example, confirmed it started and worked correctly
  2. Added <extend-configuration-property name="rpc.enhancedClasses" value="com.google.gwt.sample.dynatable.client.Person" /> to the .gwt.xml file
  3. Confirmed that the compiler logged a warning:
    [java]             Rebinding com.google.gwt.sample.dynatable.client.SchoolCalendarService
    [java]                Invoking generator com.google.gwt.user.rebind.rpc.ServiceInterfaceProxyGenerator
    [java]                   Generating client proxy for remote service interface 'com.google.gwt.sample.dynatable.client.SchoolCalendarService'
    [java]                      [WARN] The class com.google.gwt.sample.dynatable.client.Person has JPA/JDO annotations or is explicitly configured as an enhanced class using the configuration property rpc.enhancedClasses. This makes the server vulnerable to an issue with deserialization of unsafe data. See https://github.com/gwtproject/gwt/issues/9709 for more information.
    
  4. Confirmed that the server would not load the policy and logged a server error:
    [java] 2023-12-23 12:24:51.073:INFO:oejshC.ROOT:qtp559361838-84: calendar: ERROR: Service deserializes enhanced JPA/JDO classes, which is unsafe. Review build logs to see which classes are affected, or set gwt.enhancedClasses.enabled to true to allow using this service. See https://github.com/gwtproject/gwt/issues/9709 for more detail.
    [java] 2023-12-23 12:24:51.074:INFO:oejshC.ROOT:qtp559361838-84: calendar: WARNING: Failed to get the SerializationPolicy 'D08326A5FC5842E352DF103522C617ED' for module 'http://127.0.0.1:8888/dynatable/'; a legacy, 1.3.3 compatible, serialization policy will be used.  You may experience SerializationExceptions as a result.
    
  5. Note also that the client could not load data correctly with the legacy policy, and had its own error.
  6. Added -Dgwt.enhancedClasses.enabled=true to the JVM args, confirmed that the above error was replaced by a warning and the application worked:
    [java] 2023-12-23 12:38:39.032:INFO:oejshC.ROOT:qtp448000133-82: calendar: WARNING: Service deserializes enhanced JPA/JDO classes, which is unsafe. See https://github.com/gwtproject/gwt/issues/9709 for more detail on the vulnerability that this presents.
    
  7. Next, removed the configuration property, and instead added @javax.persistence.Entity to the Person bean, org.datanucleus:javax.persistence:2.2.3 added to war/WEB-INF/lib/, and repeated the test from step 3.

@niloc132 niloc132 merged commit 2efa9c6 into gwtproject:main Dec 23, 2023
3 checks passed
niloc132 added a commit that referenced this pull request Jan 9, 2024
Logs warnings at compile time, indicating which classes need to be
cleaned up to remove this feature.

Mitigation for #9709
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants