[GitHub] [nifi-minifi-cpp] bakaid edited a comment on issue #597: MINIFICPP-933: Fix raw site to site with secure comms

[GitBox <gi...@apache.org>]

bakaid edited a comment on issue #597: MINIFICPP-933: Fix raw site to site with secure comms
URL: https://github.com/apache/nifi-minifi-cpp/pull/597#issuecomment-505803516
 
 
   @phrocker The fix itself looks good, but I think there might be other issues around pemPassWordCb.
   First, the callback ignores the `size` parameter OpenSSL supplies when copying the string to `buf`. If the password is exactly as long as the buffer, then the result will not be null-terminated, if the password is longer we will overwrite the buffer. I think we should check whether the password fits in the buffer and return -1, if not. (SSL_CTX_use_PrivateKey_file will use PEM_read_bio_PrivateKey internally, and will pass our callback to it: https://github.com/openssl/openssl/blob/2c18d164f52a77b9ecc1f835b83aec476334ebbc/ssl/ssl_rsa.c#L545, and this callback should return -1 on error: https://www.openssl.org/docs/man1.1.0/man3/PEM_read_bio_PrivateKey.html "The callback must return the number of characters in the passphrase or -1 if an error occurred.").
   Second, we read the entire password file, and pass that string to the callback, and expect that it includes exactly one \n character (we copy `pass->length() - 1` characters to the buffer). This might not be true if we are on Windows (\r\n), or if there is no newline at the end of the password file.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services