Hi Rémi, On Mon, Jun 02, 2014 at 02:43:37PM +0200, Remi Gacogne wrote: > Hi, > > > Well, maybe your latest patch is still the best way to go then. After all, > > it's very simple to see what can happen : if ciphers allow DHE and the > > admin has not configured its param, then there's a risk that some users > > will run it with too low a parameter and we'd rather warn them. So if > > the warning is emitted only when preparing an SSL context (and we don't > > need to emit it more than once), then only possibly affected users will > > get it, and those not using SSL or not using DHE will not see it. > > > > I'm just seeing SSL_CIPHER_description() which returns "DH" optionally > > followed by a size, maybe it would be a more portable way of checking the > > presence of DHE in your patch ? > > Sorry, I didn't manage to find the time to look at this until now.
no worries, same for me! > You are right, we could look for the "Kx=DH" string in > SSL_CIPHER_description() output, but I am not sure there is any > guarantee that it will stick, and I don't find it very elegant (one > internal snprintf() then one strcmp() for each ciphersuite). Well, I tend to think the opposite : you already had to deal with a flag change before and after 1.0.0, while I don't think the text version has changed in parallel. I tend to think to that the user-facing text name has less chances of changing each time the openssl team needs to reorganize the flags, especially if these flags are not for public use. > Therefore I have integrated the latest proposal (iterating over enabled > ciphersuites and looking at the cipher internals) in this new patch. Thank you, we'll review it here. Emeric is taking a look at it since he's the de-facto SSL maintainer. > I think it does what is expected from the user point of view, ie it only > warns if at least one DHE ciphersuite is enabled. Other than that, it > does not increase the CPU load with the default value, and still makes > it easy to increase the ephemeral DH strength. That sounds reasonable. Will keep you updated on this, Willy
