Hi Martin, Dividing up the patch proved to be a challenge but was the right thing to do. I divided it up into a patch series of 6, with the first 5 patches being those from fedora. The 6th patch was all my corrections and updates. I ran all the prior testcases successfully.
Weird, but the fedora patches were not independent of each other. Each patch relied on of the others to fulfill missing symbols and compile successfully. I usually like patches to be independent in that they compile successfully, even in a patch series. Thanks and let me know what else is needed. regards, Joy On Fri, Apr 8, 2016 at 9:04 AM, Joy Latten <joy.lat...@canonical.com> wrote: > Hi Martin, > > I will get to work on all the resolutions we mentioned. Thanks! > I will send you email when completed and list them. > > regards, > Joy > > On Fri, Apr 8, 2016 at 2:07 AM, Martin Pitt <martin.p...@ubuntu.com> > wrote: > >> Joy Latten [2016-04-08 5:07 -0000]: >> > > -# define SHA1_Init private_SHA1_Init >> > Those defines are within an OPENSSL_FIPS so were never used in regular >> > openssl. >> >> Ah, I see that this doesn't actually get shipped in libssl-dev, so >> sorry for the noise. >> >> > > The changes in crypto/evp/p_sign.c and crypto/evp/p_verify.c don't >> look >> > > FIPS related, change the default behaviour, and should probably be >> split >> > > out into a separate patch with justification/origin and at least >> > > proposed upstream. >> > > >> > > >> > I did not think these change the default behaviour. They are adding PSS >> or >> > X931 padding to rsa >> > if requested via a flag. >> >> Right, and both flags are already exported in >> usr/include/openssl/evp.h in the current (unpatched) libssl. So, while >> this code looks correct, it looks like a backported patch from >> upstream which is unrelated to the FIPS changes. >> >> Again, I just noticed that during review. If that's part of the >> original RedHat/SUSE patch etc., then by all means keep it (taking >> unmodified patches from known, reliable, and declared origins trumps >> pretty much everything else). But if that was one of the changes from >> Ubuntu/you, it should be split out and sent upstream (or say which >> upstream commit it was). >> >> > > It also concerns me that crypto/fips/ seems to reimplement RNG, >> > > HMAC, and RSA algorithms which should already be in openssl >> > > itself. [...] The reimplemented RNG (crypto/fips/fips_rand.c) has >> > > no author information at all. >> > >> > Openssl community implements a lot of the fips approved algorithms into >> the >> > openssl-fips module, rather than into regular openssl. >> > This means for us to acquire some of these fips approved algorithms, we >> > must take them from the openssl-fips module source. >> >> Ah, so that's where they are coming from? I seems a bit dubious that >> fips_rand.c is one of the very few files which does *not* have an >> author information. So I guess this is another case of "as a reviewer >> of this big patch I have not the slightest idea where this came from" >> (cf. "split and declare patches by origin" again) >> >> > fips_utl.h is from the upstream openssl-fips module. It is a local >> header >> > file that is not exported into /usr/include/openssl. >> > But if you prefer I can move the routines into fips_test_suite.c where >> they >> > are being used. Let me know if you feel strongly about this and I will >> move >> > them. >> >> No, I don't feel strongly about it, it just jumped my eye as a >> potential trap. Again, if that's in the Ubuntu modified portion it'd >> be nice to clean up, but do prefer unmodified patches over cleanup >> like this. >> >> > > crypto/o_init.c disables checking for $OPENSSL_FORCE_FIPS_MODE. What's >> > > the rationale for this? >> > > >> > > >> > Oh wow! Yeah, that is very odd... carried over from the fedora patch. I >> > will remove that. >> >> Just FTR, this would be a good example of keeping the fedora patch >> as-is, and putting back the env check would then go into the Ubuntu >> followup patch. >> >> Thanks! >> >> -- >> 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
