Tidying up the OpenSSL private key password logic

[Jouni Malinen]

On Fri, Dec 01, 2017 at 06:08:21PM -0500, David Benjamin wrote:
> I've attached two patches that I think tidy up the logic around
> OpenSSL private key loading and passwords.

Thanks!

> The first just removes an unnecessary strdup. That parameter isn't
> mutated or anything, it's just a generic data argument to the same
> callback that you pass in.

I tried to figure out why the strdup() call was there in the first
place, but could not really find any good reason for this.. This came in
with one of the code cleanups in 2004, but the initial implementation
actually used the configuration value directly.

> The second avoids using the SSL(_CTX) default password callback
> altogether. Since you all use it for one-off calls anyway, it ends up
> being a little cumbersome as you must set and unset them. Further, you
> end up mutating the SSL_CTX after SSLs have been created, which isn't
> generally safe. Rather, I think it's cleaner to just pass the password
> into the PEM_read_bio_PrivateKey call yourself. The SSL-level
> functions are merely convenience routines on top of this. This also
> allows abstracting away the DER/PEM fallback code. (It also avoids a
> mess of OpenSSL version variability.)

Yes, this looks much cleaner. I really never liked the need to register
a callback for the simple task of reading a passphrase protected file. I
guess I tried to avoid using the lower layer routines for this, but
based on this patch, this looks so much better that it is justifiable to
use them.

> Note: you probably want to run tests on this. I wasn't sure how to do
> that, but I have checked that they compile on my system.

Seemed to work fine with the couple of hwsim test cases that ended up in
the callback function. I applied both (with some cleanup on the second
one to get rid of the #ifdef blocks in the callers).

-- 
Jouni Malinen                                            PGP id EFC895FA