On 08/21/2015 03:51 PM, Dave Zhu (yanbzhu) wrote: > > Emeric, >> >> Code initially use 'ctx->default_passwd_callback_userdata' to allow us to >> manage a further way to manage passphrase via configuration. > > As far as I could tell, this field was not getting set anywhere in the > code. It¹s set with SSL_CTX_set_default_passwd_cb, which I did not find in > the codebase. I had then assumed that this was just a harmless copy/paste > of example code, which generally always use this CB.
Not exactly, it was passed to be sure to not reach a surprise the day we will start to use it . We have some pending projects to manage externalized passphrase. >> >> I notice also you continue to load DH parameters for each files. It was >> not a big deal by the past because for each file correspond a uniq >> certificate. >> >> With your patch, i don't know what will be the behavior. In the way it is >> loaded on SSL_CTX, the DH parameter doesn't seem specific to the used >> DSA/RSA/ECDSA certificate. So which one will be used, first or latest >> loaded? >> >> Some users will expect to use the DH parameter defined in the same file >> than the used certificate, but i'm really not sure it will be the case. > > > Unsure about this. I can change it to only load the first time the context > is created. It depends on what is expected by users. I would prefer to use the DH parameter depending of the used RSA/ECDSA certificate. It would be possible with your first proposal but it doesn't seem managed in the latest openssl-1.0.2 API. > >> >> Finally, i don't understand what will be the behavior about the >> certificate chain with openssl < 1.0.2. I see you manage to load the >> chain differently depending the version of openssl but i ignore if the >> behavior will differ. > > > In 1.0.2, we can make use of API to load multiple certificate chains into > a CTX. Prior to 1.0.2, you could not load the chains. So the checks ensure > that we make use of the new APIs, and can then serve the correct > certificate to users based on the user¹s cipher suite. I see, but if the feature is only half-supported for openssl version < 1.0.2 we should simply avoid to use it on these versions. > > -Dave > Anyway, I admit i'm a little scared with your latest patches, you re-code a large part of certificates loading and i'm currently unable to guarantee that there is no regressions on linked features (SNI, ocsp, crt_list, passphrase management). The first proposal was very less intrusive (the ones which don't used 1.0.2 API but secret callback). But i understand the point of view of the experts of the mailing list . Did you expore an intermediate solution: Continue to load keys/certificates etc in one new allocated SSL_CTX and when you fill the SNI tree, if the certificate comes into collision with an other, extract key/cert/chain from the early allocated SSL_CTX to fill/complete the already existing one? I don't know if Openssl API provide enougth primives to do that. But it will certainly have less impact on the existing behavior. R, Emeric
