Server-side validate for rpc.enhancedClasses payloads

[niloc132]

Following #9709 and the PR #9879 to mitigate this by disabling the feature, this issue is to track efforts to make this feature safe to use again. There are several approaches that can be used to make this safe, and reasons that no one solution is a guaranteed fit for all applications, so I propose that we find a general extension point to add, and if populated by an application, permit the use of enhanced classes without warning or error.

Basic approaches:

1. Sign serialized data with a trusted private key and serialize the signature+public key with the data. Then, when reading the data back, the server can confirm the public key is trusted and that the signature is correct. A single server might create an in-memory key on startup (no persistence/coordination required), but then server restarts mean that clients must re-fetch all entities. A persisted key resolves this issue, at the cost of some additional configuration. A cluster of servers performing any kind of load balancing or handoff that allows entities from one server to be passed to another further requires coordination of keeping separate private keys but sharing trusted public keys (or using a common CA/etc), or all servers sharing the same keypair.
2. Authenticate first before deserializing. This assumes that no authenticated user could be malicious, not usually an acceptable assumption. onBeforeRequestDeserialized can be overridden today to read from the request or session to validate in this way.
3. Allow developer control/configuration of the ObjectInputStream and surrounding code. In some cases it could be sufficient to offer a limited set of allowed classes that ObjectInputStream.resolveClass could read, and to check each one as it is read that it matches the expected field to write. See ServerSerializationStreamReader.checkResolve for the current implementation here.

Option 1 seems to be the superior choice in terms of granting flexibility and not requiring trusting malicious clients, but it also likely requires the most flexible configuration. We could go so far as to delegate this as a simple interface such as

public interface SharedClassKeyValidation {
  /**
   * Given a plain payload, returns a signed payload that will be later accepted by verifyAndUnwrap.
   * It must always be true that verifyAndUnwrap(signAndAppend(payload)) returns the original payload.
   */
  String signAndAppend(String payload);
  /**
   * Given a signed payload, confirms the signature is valid and the key accepted, and returns the
   * underlying payload. If the signature or key is invalid, returns empty.
   */
  Optional<String> verifyAndUnwrap(String signedPayload);
}

and offer a simple (single server), in-memory only (won't survive restarts) implementation to start from, and configuration to provide a custom implementation.

[jnehlmeier]

Here are a few thoughts:

1. Using public/private key cryptography seems overkill in 99.9% of cases because it would only be useful if server A managed by company A sends data to the browser and the browser then sends this data to server B managed by company B. In that case Company A and B should share their public keys to verify signatures. However I feel like most of the time (if not always) all GWT-RPC servers are managed by the same company. In that case symmetric cryptography is enough. So I think the default implementation provided by GWT should be based on a SHA 256 hash encrypted using AES 256. The encryption key and initialization vector for AES can be generated in-memory using random data or derived from user provided passphrase using PBKDF2. The latter would allow server restarts and server clusters.
2. The API we add should not give direct access to the GWT-RPC payload in a way that would allow messing around with the overall payload of a GWT-RPC request/response.
3. The deployment of the GWT-RPC service should fail if the code does not know how to sign/verify.

Given the above I would suggest using the Servlet API init() methods to initialize the signing code. However because these methods are not marked final in our GWT-RPC base servlets the code cannot fail fast during deployment. Maybe we should think about making it final and introduce a second, empty initService() method that subclasses can override. However it would be a breaking change. If we don't want that we can only fail fast at runtime / request time.

API wise I could imagine

public class RemoteServiceServlet ... {

  // To support public/private key cryptography the public key would 
  // need to be distributed manually. The API below does not allow 
  // sending a public key along every response.
  interface SignatureHandler {
    
    byte[] createEncryptedSignature(byte[] data);

    boolean isValid(byte[] data, byte[] encryptedSignature);

  }

  private SignatureHandler signatureHandler;

  @Override
  public void init() {
    super.init();
    signatureHandler = createSignatureHandler();
  }

  protected SignatureHandler createSignatureHandler() {
    // SHA256 + AES256 based, allows configuration of AES keys via system properties. 
    // Should cover most cases
    return new DefaultSignatureHandler();
  }
}

The SignatureHandler would then be passed on to the code that requires it. The API is as simple as possible and is primarily meant to be used with symmetric cryptography. Public/private key cryptography can be implemented but requires external distribution of public keys because the public key will not be send along every GWT-RPC response.

[niloc132]

I think this can work - two payloads instead of one does offer the complication that it technically changes the wire format ("instead of one opaque string payload added to each object, there are now two"), but with the use of raw byte[] payloads we can base64 them individually and then join them on an unused character, or length-prefix each payload and concatenate them before base64.

1. With regard to symmetric vs private key, I have personal knowledge of applications using this strategy as insisted by their own security reviews, requiring that even two instances within the same cluster and company ought not share their private keys, so that, at least in theory, one key can be revoked at will.
2. That's a fair point, but relative unchanged between your API and mine - I tend to favor String or ByteBuffer over byte[] to prevent the implementation from changing the data. This would imply base64'ing the original payload first, and then joining on some unused character (the first thought I mentioned above).
3. I'm not sure why that should need to be true - if the service has no need for signing, there should be no error/warning. A quick survey of publicly accessible GWT apps using RPC seems to suggest that this is the typical case (that is, I found zero vulnerable to this), but of course I don't have a way to guarantee this is true universally.

I'm content to leave the other details to the implementor, if someone wants to volunteer or sponsor this work. I think however that you can sidestep the init() problem by either asserting that signatureHandler not be null when loading the policy file if it will be required, or by renaming the create method to merely be get (and let the implementation deal with making it a singleton or returning fresh instances if stateless, etc. I might also suggest that to reduce the risk of name collision with existing methods, it could be a bit more specific - createEnhancedClassSignatureHandler etc.

[jnehlmeier]

1. With regard to symmetric vs private key, I have personal knowledge of applications using this strategy as insisted by their own security reviews, requiring that even two instances within the same cluster and company ought not share their private keys, so that, at least in theory, one key can be revoked at will.

That's fine we should not forbid that. The above API allows public/private keys but does not easily allow every server in the cluster having its own key pair as you would need to find a way to figure out the public key to use during verification.

Technically it is possible with the above API, although not obvious. Since the implementation is responsible to create and verify the signature, nobody stops it from adding additional information to the signature. So createEncryptedSignature() could return bytes in the form of <encrypted signature>+<public key>. Thinking a step further, creative devs could even use protobuf or similar to encode whatever they want in the signature.

2. That's a fair point, but relative unchanged between your API and mine - I tend to favor String or ByteBuffer over byte[] to prevent the implementation from changing the data. This would imply base64'ing the original payload first, and then joining on some unused character (the first thought I mentioned above).

Good point about changing data. I guess we should settle on a readonly ByteBuffer then. I don't like String because it requires knowledge about character encoding.

3. I'm not sure why that should need to be true - if the service has no need for signing, there should be no error/warning. A quick survey of publicly accessible GWT apps using RPC seems to suggest that this is the typical case (that is, I found zero vulnerable to this), but of course I don't have a way to guarantee this is true universally.

This was under the impression that we would revert the commit producing warnings/errors for the time being but instead always have a default implementation for signing in place if the developer does not provide its own implementation. That way enhanced classes can simply be used or experimented with without thinking about signing.
That's why I tend to favor a breaking change making init() final, so GWT-RPC gains a spot in code to do initialization work that is required to execute.