GUACAMOLE-1286: Add support for providing a custom IV.#1153
Conversation
eugen-keeper
left a comment
There was a problem hiding this comment.
Please consider updating the README to document the new behavior extensions\guacamole-auth-json\doc\README.md
| // part of the request, use that, instead. | ||
| IvParameterSpec iv = NULL_IV; | ||
| if (ivStr != null && !ivStr.isEmpty()) { | ||
| byte[] ivBytes = BaseEncoding.base64().decode(ivStr); |
There was a problem hiding this comment.
This can throw IllegalArgumentException for malformed ivStr base64, but that exception is not handled here. It propagates to UserDataService.fromCredentials(), where the existing catch (IllegalArgumentException) logs Submitted data is not proper base64, which is misleading when only iv is invalid. Maybe consider catching IllegalArgumentException here and rethrowing as GuacamoleServerException("Invalid IV provided.").
| * Decrypts the given ciphertext using the provided key, returning the | ||
| * resulting plaintext. If any error occurs during decryption at all, a | ||
| * GuacamoleException is thrown. The IV used for the decryption process is | ||
| * a null IV (all binary zeroes). |
There was a problem hiding this comment.
The method now accepts an optional ivStr parameter for a custom IV. Please consider updating the Javadoc to reflect the new behavior.
Based on another situation that I've run into where I need to be able to provide an
IVfor the crypto module, I've tweaked the code for the JSON extension slightly to allow this value to be passed in as a query parameter along with the encrypted and signed JSON data. I've no idea if this is actually safe or not (from what I've read it seems that passing the IV in the open with the encrypted data is okay?), and am opening as a draft since I haven't even tested it, yet.