Hi Martin, Responses below. Thanks!
regards, Joy On Thu, Apr 7, 2016 at 5:27 AM, Martin Pitt <martin.p...@ubuntu.com> wrote: > 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. > > Ok, let me investigate a bit further and get back to you. > > > 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). > > Ok, I agree. But I am afraid will still be big. The fedora patch had already incorporated almost all the stuff needed from the openssl-fips module. But even so, I do think is a good idea to separate any additional changes I made. I will get started on this. > Thanks, > > Martin > > > ** Changed in: openssl (Ubuntu) > Status: New => Incomplete > > -- > You received this bug notification because you are subscribed to the bug > report. > 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 > -- You received this bug notification because you are a member of Ubuntu Bugs, which is subscribed to Ubuntu. https://bugs.launchpad.net/bugs/1553309 Title: [FFe]: Include FIPS 140-2 into openssl package To manage notifications about this bug go to: https://bugs.launchpad.net/ubuntu/+source/openssl/+bug/1553309/+subscriptions -- ubuntu-bugs mailing list ubuntu-bugs@lists.ubuntu.com https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs
