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 <email address hidden> 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
>