Created attachment 2137 [details] update openssl ver check with openssl-1.0.0, they've started a new binary compatibility scheme. in the past, only patchset versions were compatible (so 0.9.8[abcdefgh...]). but now, minor versions are compatible as well. so 1.0.[01234...] should be acceptable. as such, the seed_rng() check in entropy.c needs updating. perhaps something like the (compile-only tested) attached patch. --- a/entropy.c +++ b/entropy.c @@ -211,9 +211,14 @@ seed_rng(void) #endif /* * OpenSSL version numbers: MNNFFPPS: major minor fix patch status - * We match major, minor, fix and status (not patch) + * We match major, minor, fix and status (not patch) for <1.0.0. + * After that, we acceptable compatible minor versions (so we + * allow 1.0.1 to work with 1.0.0). */ - if ((SSLeay() ^ OPENSSL_VERSION_NUMBER) & ~0xff0L) + u_long bldver = OPENSSL_VERSION_NUMBER & ~0xff0L; + u_long runver = SSLeay() & ~0xff0L; + if ((bldver >> 12) < 0x10000 && bldver != runver) || + (bldver >> 12) >= 0x10000 && (runver >> 12) < (bldver >> 12))) fatal("OpenSSL version mismatch. Built against %lx, you " "have %lx", (u_long)OPENSSL_VERSION_NUMBER, SSLeay());
Comment on attachment 2137 [details] update openssl ver check >+ (bldver >> 12) >= 0x10000 && (runver >> 12) < (bldver >> 12))) This is going to drop the status nybble off when it's shifted, which means you can build against a dev version of openssl and run against a release one. There's no guarantee that's going to be binary compatible, though. The current check will catch that case, though.
Comment on attachment 2137 [details] update openssl ver check Also: >+ * After that, we acceptable compatible minor versions you're accepting compatible *fix* versions. >+ (bldver >> 12) >= 0x10000 && (runver >> 12) < (bldver >> 12))) that's not going to stop newer major or minor library versions from being built against old headers.
I think it would be simpler and more correct to keep the existing logic and just change the mask size, eg: u_long version_mask = SSLeay() >= 0x10000000 ? ~0xffff0L : ~0xff0L; if ((SSLeay() ^ OPENSSL_VERSION_NUMBER) & version_mask) fatal("OpenSSL version mismatch. Built against %lx, you " "have %lx", (u_long)OPENSSL_VERSION_NUMBER, SSLeay());
Created attachment 2139 [details] Improved test This check is a little more strict: It matches the patch version (so 1.2.0 will not work with an OpenSSH built against 1.0.1). This is a bit more paranoid than the original patch, but looser than what we have at present. It also checks disables the laxity if the build or runtime versions are not official releases. My rationale here is that binary compatibility might be broken in unreleased versions.
Created attachment 2140 [details] Improved improved test Darren's right, as usual.
"improved improved test" patch applied and will be in openssh-6.0 (due very soon)
Thinking about it some more, the cases you need to consider: #1: you upgrade openssl to a newer fix version. obviously you don't want ssh to stop working and with this diff, it won't. #2: you upgrade ssh with something built against the same major and minor version but a newer fix version. Right now, you can't deploy that unless you upgrade openssl first. is #2 a reasonable thing to do? I would argue that it is. Damien's counter-argument is from the OpenSSL home page: "OpenSSL 1.0.1 is now available, including new features".
(In reply to comment #7) > Thinking about it some more, the cases you need to consider: > #1: you upgrade openssl to a newer fix version. obviously you don't > want ssh to stop working and with this diff, it won't. > > #2: you upgrade ssh with something built against the same major and > minor version but a newer fix version. Right now, you can't deploy > that unless you upgrade openssl first. > > is #2 a reasonable thing to do? I would argue that it is. > > Damien's counter-argument is from the OpenSSL home page: "OpenSSL 1.0.1 > is now available, including new features". #2 would allow "bad" practice in the general sense. Meaning, while it may be reasonable for a binary built against an older lib to be expected to run with a newer lib, it is not reasonable to expect a binary built with a newer lib to run with an older lib.
Note that beta versions on the same fix release (1.0.z should be ABI compatible. Only when the major or minor release changes there should be ABI breakers (that is when x or y in x.y.z changes). Also as the patch level (the letter after version) changes there should be strictly only bugfixes, these should be even forward-backwards compatible. So for the after 1.0 versions I'd suggest the version_mask to be ~0xfffffL
(In reply to comment #9) > Note that beta versions on the same fix release (1.0.z should be ABI > compatible. Only when the major or minor release changes there should > be ABI breakers (that is when x or y in x.y.z changes). You'd hope so, however from the CHANGES file in openssl 1.0.1 under "Changes between 1.0.0h and 1.0.1" (a "fix" release, in openssl's parlance) shows, amongst other things: *) Functions FIPS_mode_set() and FIPS_mode() which call the underlying FIPS modules versions. [Steve Henson] *) [...] This enables the following EC_METHODs: EC_GFp_nistp224_method() EC_GFp_nistp256_method() EC_GFp_nistp521_method() so, new functions introduced in "fix" releases. Given this, we are yet to be convinced that "fix" releases both are forward and backward ABI compatible. > Also as the patch level (the letter after version) changes there should > be strictly only bugfixes, these should be even forward-backwards > compatible. Patch level is covered by the 0xff0 mask in both cases. > So for the after 1.0 versions I'd suggest the version_mask to be > ~0xfffffL That'd allow development and release versions to mix too. For now we're only considering release versions.
Set all RESOLVED bugs to CLOSED with release of OpenSSH 7.1