Question about CVE-2015-5291
The Debian bug about this CVE has been sitting for a week now with no apparent action so I thought I'd look at it. https://bugs.debian.org/801413
The versions in Debian are a bit old and since there has been an ABI break since then, actually updating to the new versions is very difficult.
I had a look at which exact commits fixed this. Would applying these commits be enough to fix the CVE?
Author: Simon Butcher <firstname.lastname@example.org> Date: Wed Sep 30 00:45:21 2015 +0100 Added max length checking of hostname
Author: Simon Butcher <email@example.com> Date: Tue Sep 29 23:27:20 2015 +0100 Added max length checking of hostname
Author: Simon Butcher <firstname.lastname@example.org> Date: Thu Oct 1 00:24:36 2015 +0100 Added bounds checking for TLS extensions IOTSSL-478 - Added checks to prevent buffer overflows.
Author: Simon Butcher <email@example.com> Date: Thu Oct 1 01:17:10 2015 +0100 Reordered extension fields and added to ChangeLog Reordered the transmission sequence of TLS extension fields in client hello and added to ChangeLog.
Author: Manuel Pégourié-Gonnard <firstname.lastname@example.org> Date: Fri Oct 2 09:53:52 2015 +0200 Add extra check before integer conversion end < p should never happen, but just be extra sure
We recommend you use the last release for the branch most suited for your needs, and do not recommend you only apply the commits you list as otherwise you'll miss other security related improvements and fixes.
The Debian PolarSSL/mbed TLS package appears to be using version 1.3.9 at the moment (see here), so we would suggest you move to 1.3.14 to avoid missing other fixes.
Obviously that would be the best solution and that's ok for unstable/testing, but that can't be done for any of the stable releases since the SOVERSION of libpolarssl was changed. Uploading it now would break all the reverse dependencies which are using a different ABI.
I suppose one alternative is to take 1.3.14 and then revert the changes which caused the break in ABI, instead of trying to fix this bug on top of 1.3.9 currently in Debian jessie.
Sounds like a viable option. Actually since ABI changes were only introduced when adding new features, I don't think you'd need to revert the changes themselves, just disable the corresponding features in config.h.
% git diff polarssl-1.3.9..mbedtls-1.3.14 include/polarssl/config.h | grep '^+#' +#define POLARSSL_SSL_ENCRYPT_THEN_MAC +#define POLARSSL_SSL_EXTENDED_MASTER_SECRET +#define POLARSSL_SSL_FALLBACK_SCSV +#define POLARSSL_SSL_CBC_RECORD_SPLITTING
So, just disabling those four things in
config.h (either by applying a patch to it or by invoking
scripts/config.pl in a pre-build script) should be enough to restore ABI compatibility.
If you want to check that disabling those 4 options is indeed enough to preserve ABI, you can use a small program that includes
mbedtls/ssl.h and then prints the size of the various structures defined in that header: they should be identical between 1.3.9 and 1.3.14+patch.
Of course you'll also need to patch
library/Makefile to restore the old soversion if you go that way.
It should be noted that since the 1.3 branch is now in maintenance mode, we will never change the ABI in that branch again. So, patching
config.h now might be worth the effort since once it's done you can then apply all future upgrades to the 1.3 branch without any more trouble.
That might the best way to guarantee you're able to release security fixes in a timely fashion in the future.
That sounds like a good idea. I hadn't actually checked what parts of the ABI were broken, but if it's just new functions then what you suggest should be ok.
Actually the issue is not new functions, if new members in structures (mostly
ssl_session, etc) that causes ABI incompatibilities. Since we take care to skip those members when the corresponding feature is not enabled, disabling the flags in
config.h should result in structure definitions that are the same as before the feature is introduced.
Ok, I ran some tests and I was wrong, disabling those four flags does not seem to be enough to restore ABI compatibility, due to other changes that are not controlled by
config.h flags, as well as a prototype change in an internal but technically public function.
It might still be possible to isolate the changes that caused ABI changes and exclude them somehow, but it's not as easy as I initially thought.
This would need more investigation, but unfortunately I'm unlikely to have the time for that in the next few days. So maybe hand-picking the changes that fix the recent severe issue is the best short-term solution, though it would be more satisfying long-term to be able to incorporate all fixes, including for the less severe issues.
I did some investigation and came up with this here: https://github.com/jcowgill/mbedtls/commits/debian-jessie-compatibility
It should preserve the ABI, although there are a lot of changes. I am a little uneasy about applying it to something like Debian which will then have to maintain it (and be responsible for any bugs).
I posted the libabigail diffs here: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=801413#25
Nice job! I guess now the decision belongs to the Debian developers who will have to take core of future updates in Debian stable. On one hand, your ABI-restoring patches are fairly big, but on the other hand, handpicking all security fixes (not just the most severe) would likely result in an even bigger set of patches, that is unfortunately likely to keep growing in the future, while the ABI-restoring patches should remain more stable now that the 1.3 branch is in maintenance mode.