A nominal search for some of the relevant Cipher methods do not reveal any other (direct) usage by us across the IdP stack.

Appreciate all your help. We don’t do a lot of this anywhere else and this is exactly why, but we’ll search the code ourselves just in case after addressing this.

(I’ve been working with Rich on this)

I’ve attached an improved patch (DataSealer.patch) that fixes another issue with the Shib calls to Cipher.

Shib was definitely using the Cipher API improperly. It looks to me like the FIPS implementation is functioning within the API guarantees, but feel free to verify.

Note: I have NOT checked the rest of the Shibboleth codebase for other instances of similar misuse of Cipher.

More specifically:

• Cipher.getOutputSize() returns a number that is intended to be used for sizing the output buffer.
  However, per the API spec: “The actual output length … may be smaller than the length returned by this method.”
  Shib was effectively assuming that getOutputSize() returned the actual output length, and it was doing the wrong thing in cases where getOutputSize() did not return the actual output length.
  It looks like the non-FIPS implementation just happens to have getOutputSize() return the actual output length, but the FIPS implementation returns a number larger than the actual output length.
  
  

• In case it isn’t clear what Shib was doing wrong in the case where getOutputSize() is too big:
  When converting a byte array to a String without specifying a length, the byte array’s length is used and null characters are included in the String. So, for example:
  byte[] b = new byte[2];
  b[0] = 'x';
  String s = new String(b, StandardCharsets.UTF_8);
  s.length(); // Returns 2
  "x".equals(s); // Returns false because “x” != “x\0”
  
  

• So, the first fix is: Don’t assume the size of the output buffer is the size of the output data; Instead, track the actual output data size and pass it to anything that reads the output buffer.
  
  

• The additional problem that I fixed in my improved patch is:
  The Cipher API supports either streaming (“multiple-part”) or all-at-once (“single-part”) encryption/decryption.
  Shib is not streaming, so it may use the all-at-once behavior … But it was instead using the streaming behavior … And it was using it wrong.
  If you are using the streaming functions, then you are supposed to call getOutputSize() and create a new byte array output buffer for every call to .update() or .doFinal(), and then concatenate the buffers. However, Shib was not doing that.
  The fix is to switch to using the all-at-once behavior (call the four-parameter doFinal()) instead of using the streaming behavior (calling update() followed by the two-parameter doFinal()).
  
  

Much appreciated. Seems like the code was probably wrong anyway, but I’ll dig into it, I want to make sure this isn’t a case of the FIPS code violating any API guarantees.