Hello Joy, thanks for your answers. I'll cut out the ones that are resolved now from my POV.
Joy Latten [2016-04-06 19:48 -0000]: > crypto in regular openssl when in fips mode. The openssl-fips module is not > only bigger than this patch, but is separate and a bit more complex. > Since it is separate, it would almost be akin to maintaining 2 versions of > openssl. One advantage though is that it is maintained upstream. :-) > Again before I got here, Canonical consulted with external security > consultants who recommended we pursue the method that redhat and suse did. Yeah, that's a shame, though I realize this decision is not up to you or me. If we have funding for maintaining this patch both in Xenial (which should be fairly easy as we'll only backport security fixes) and, more importantly, in newer releases which will get newer upstream openssl releases and thus require heavy porting and testing, then so be it. > > crypto/dsa/dsa.h: > > # define DSA_F_DSAPARAMS_PRINT_FP 101 > > +# define DSA_F_DSA_BUILTIN_KEYGEN 127 > > +# define DSA_F_DSA_BUILTIN_PARAMGEN 128 > > # define DSA_F_DSA_BUILTIN_PARAMGEN2 126 > > > > Patches like this are utterly dangerous. As soon as a new upstream > > version defines their own new constant further down, the FIPS patch will > > most likely still apply, but silently introduce a conflict as two > > different constants now have the same value. > > > > Yes! What you have stated is true in general. Fortunately in the case you > pointed to above, this should not be a problem. Those are error codes. When > adding new error codes in openssl, is standard practice that you run "make > errors" which in turn creates all those defines for errors. That is how the > above happened. This doesn't help at all, though. If upstream does a new release and calls "make errors", then releases a tarball with that, our patch just gets statically applied on top of that and thus will *not* adjust the error codes for potential conflicts again. So with that you'd get two constants with the same value. OTOH, if our package build would call "make errors" again, this would mean that (1) we'd have an ABI break (as existing reverse dependencies that use these error codes all need to be rebuilt for the new value, as it's a macro in a public header file), and (2) the patch should not contain this autogenerated part in the first place. > > There is some pointless whitespace change in e. g. crypto/evp/c_alld.c > > which further blow up the patch. > > > > Sorry about that, I thought I had caught all the whitespaces. I can > correct that. Just for avoidance of doubt: Please only clean this up if it's in the Canonical-modified portion of the patch. If that comes from RedHat/SUSE patches, it's magnitudes better and easier for long-term maintenance to import them unmodified (as much as possible) and accept some pointless noise, rather than heavily editing those. That's why I deem it very important to (1) clearly document where these patches originate, and (2) split them up into "taken as-is from https://opensuse.org/whereever"; and a separate "canonical modifications to the FIPS patch". > Please see above. I can break into smaller patches for easier > reviewing and bundle with some logic. But in terms of maintenance, > I honestly don't know if it will matter. It is just messy to me, > regardless. Sorry, I have spent some time thinking about it. I can > separate out such that everything under crypto/fips is in one patch, > and all else in another. But they won't be independent of each other > in regards to successfully applying and building. No no, please don't break them apart in arbitrary ways like this. The point is to break them apart by origin, e. g. "taken from the upstream FIPS branch", "taken from RedHat", "Canonical origin". This will make it tremendously easier to review (as the patches which have existed for 10 years in other distros only need a shallow review), maintain (as it's much simpler to update those patches from their original sources when rebasing to a newer upstream version), and understand (as it's clear what the actual Ubuntu delta is towards upstream, RH, and SUSE). Thanks, Martin ** Changed in: openssl (Ubuntu) Status: New => Incomplete -- You received this bug notification because you are a member of Ubuntu Touch seeded packages, which is subscribed to openssl in Ubuntu. https://bugs.launchpad.net/bugs/1553309 Title: [FFe]: Include FIPS 140-2 into openssl package Status in openssl package in Ubuntu: Incomplete Bug description: This is a request for a Feature Freeze Exception to include FIPS 140-2 selftest into the openssl package in preparation for the FIPS 140-2 compliance for 16.0.4. This patchset will : - add ability to config, compile, run with fips option enabled - add the selftest files to crypto/fips directory. - minor changes to several algorithms in crypto directory to ensure the selftest compile successfully when fips is enabled. The selftest will be initiated externally at this point and not internally. Hope to have a test package ready early next week. To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/openssl/+bug/1553309/+subscriptions -- Mailing list: https://launchpad.net/~touch-packages Post to : touch-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~touch-packages More help : https://help.launchpad.net/ListHelp
