[FFe]: Include FIPS 140-2 into openssl package

The patchset defines OPENSSL_FIPS in the openssl code. Thus code within "#ifdef OPENSSL_FIPS" gets built for the libcrypto and libssl libraries. However, the libraries don't run in fips mode. The version we certify will.
This preliminary step to include the patchset now into 16.04 allows us to do only minor changes to the code for the 16.04 update version to be fips certified.

A test package is available at https://launchpad.net/~j-latten/+archive/ubuntu/myppa

Building the test package included running the tests in the openssl's test directory. If any fail, the build would fail.
The tests in openssl's test/ directory ran successfully for build of above test package.

Successfully installed the package on a VM and ran following tests provided by security team in lp:qa-regression-testing.
test-openssl.py
test-apache2.py
test-apache2-mpm-prefork.py
test-wget.py
test-ca-certficates.py

All were successful.

attaching debdiff

The bug title is misleading -- judging by the patch this is by far more than just adding a new selftest. This patch changes the runtime behaviour in multiple places too.

Can you please describe what FIPS is, where the patch comes from, how this got tested, how can we be sure that this does not break interoperability, etc?

Overview
-----------------
FIPS 140-2 is a U.S. Government computer security standard to accredit cryptographic modules. The certification process validates and certifies the crypto within the module or used by the module.

Canonical is pursuing FIPS 140-2 certification for several modules in 16.04, openssl, kernel crypto, strongswan and openssh server and client. Each module will require some additions and/or modifications to meet the FIPS 140-2 standard. From what I understand, we will certify on an update to 16.04. However, I was informed it would be a good idea to begin getting some of the changes into 16.04 now. Thus this openssl freeze exception.

Some general FIPS 140-2 requirements are
1. Selftests
    Each fips-approved crypto algorithm tests against a known test vector to verify its correctness.
2. Integrity check
    Verify that hmac-sha of the running library is the same as the hmac-sha of the shipped and nstalled binary.

The selftests and integrity check are done upon startup and initialization of the module and once passed, the module runs in fips mode, meaning only fips-approved crypto algorithms that have been certified are to be accessed. However, applications can link to the library and choose to use the non-approved algorithms. When they do so, they cannot claim to conform to fips.
(I refer to openssl as the module here.)

Implementation specifics
-----------------------------------
For openssl to run in fips mode, several things must occur successfully.
1. openssl must read a 1 from /proc/sys/crypto/fips_enabled.
2. The selftests must pass
3. The integrity check must pass

openssl does not run in fips mode unless all 3 things are successfully accomplished.
A linking application can also call FIPS_mode_set(), to enable fips_mode. If openssl is not already in fips mode,
FIPS_mode_set will run selftests and integrity check and they must both pass in order for fips mode to be enabled.

Patchset background
-------------------------------
Both Red Hat and Suse have already acquired FIPS 140-2 certification for some of the same modules we are wanting to certify.
The openssl community has also pursued and achieved fips 140-2 certification. https://www.openssl.org/docs/fips.html
However, the openssl community created an entirely separate openssl fips module to achieve this. Upon investigation and consultations, Canonical has decided to pursue fips 140-2 certification of openssl in a manner more similar to redhat and suse which requires making changes to the regular openssl rather than including a separate openssl fips module.

Redhat and Suse appear to have used the same fips patchset for openssl, with some minor differences between the two. The code in debdiff attached to this bug is based upon Red Hat and Suse's fips patches found in the opensuse and fedora openssl source, with some minor changes to accommodate updates to the fips standards and some code improvements. The openssl community's openssl fips module had a few updated self-tests, so I included these where appropriate.

The fips patchsets will not be included into the upstream openssl nor the upstream debian. They are to be maintained by Canonical and used ...

Short summary of above comments:

- FIPs 140-2 is a U.S. government security standard for crypto. it involves receiving accreditation for the crypto.

- This patch contains,
    - selftest required by FIPs
    - defines OPENSSL_FIPS
    - a few crypto additions/changes that are constrained by OPENSSL_FIPS define and having to be in fips mode to execute.

This patch does,
    - provide the additional code required for FIPs certification
    - upon openssl initialization and setup, the selftests will be executed. If a selftest fails, because openssl is not in fips mode,
     normal operation should not be interrupted.

- This patch does not
   - it does not enable fips mode, thus openssl will run as it normally does

The FIPs patch will not be included into the upstream source. This is a feature to be maintained by Canonical.

New debdiff.
Added a few more sentences to describe the patch to the patch header.
Also corrected a compiler warning.

The patch changes behaviour even in !fips mode, e. g. in apps/speed.c:

         for (i = 0; i < DSA_NUM; i++)
- dsa_doit[i] = 1;
+ if (!FIPS_mode() || i != R_DSA_512)
+ dsa_doit[i] = 1;

(additional check for R_DSA_512), and it even modifies code that doesn't touch any FIPS flag:

- idea_set_encrypt_key(key16, &idea_ks);
+ if (doit[D_CBC_IDEA]) {
+ idea_set_encrypt_key(key16, &idea_ks);
+ }

It also removes some of the upstream OpenSSL FIPS code, in crypto/cmac/cmac.c.

> The FIPs patch will not be included into the upstream source.

Apparently there already is some FIPS support upstream (you pointed out https://www.openssl.org/docs/fips.html yourself, and the patch seems to disable that). Isn't there some middle ground where we could use the upstream tests and FIPS integration as much as possible, and make the patch less ridiculously big?

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.

There is some pointless whitespace change in e. g. crypto/evp/c_alld.c which further blow up the patch.

There are a few extra calls to OPENSSL_init_library() now, without a comment. Is this guaranteed to be idempotent (it might reset seeds, counters and the like)? Is this a performance hit?

crypto/evp/evp.h:
-# define EVP_CIPH_FLAG_FIPS 0x4000
+# define EVP_CIPH_FLAG_FIPS 0x400

This also looks very dubious -- as this is a flag that is commonly passed to functions, this could be an ABI break.

The added file crypto/fips/fips.h says "Copyright (c) 2003 The OpenSSL Project.", other files like crypto/fips/fips_drbg_hash.c have "Written by Dr Stephen N Henson (<email address hidden>) for the OpenSSL project". So this does not seem to be originating from Red Hat, SUSE or Canonical, or is that just a copy&paste error? If it actually comes from upstream somewhere, can these parts please split out into a separate patch (e. g. one for stuff that is being taken from some upstream branch, one that is being taken from RedHat/SUSE, and then perhaps one with the modifications from Canonical).

There is a lot of added dead code, like do_bn_print_name(), parse_line(), or tidy_line(). I noticed these as I was looking for usage of strcpy(). These functions don't get any buffer size, don't do any overflow checking, etc.

There are no references to where these patches are taken from (RedHat/SUSE), and which changes were done relative to them. Please add them, so that it's a bit easier for someone else in the future to re-merge them. As I said above it would also be useful to split them up by origin, in order to have a realistic chance to maintain them in the future.

These are some eye-brow risers from the first 15% of t...

Hi Martin,

My apology for the delay. I had a morning full of meetings and I needed to
look at the code to answer.
I have addressed the first half of your email and will continue with the
second half next. Will send another email

regards,
Joy

On Wed, Apr 6, 2016 at 4:33 AM, Martin Pitt <email address hidden> wrote:

> The patch changes behaviour even in !fips mode, e. g. in apps/speed.c:
>
> for (i = 0; i < DSA_NUM; i++)
> - dsa_doit[i] = 1;
> + if (!FIPS_mode() || i != R_DSA_512)
> + dsa_doit[i] = 1;
>
> (additional check for R_DSA_512), and it even modifies code that doesn't
> touch any FIPS flag:
>

hmmm... for non-fips mode, which is regular openssl, this should not change
anything.
According to the if statement, when in non-fips mode (regular openssl), the
!FIPS_mode() will evaluate to TRUE, thus causing the if statement to
always evaluate to TRUE and set the dsa_doit[i] = 1; just at it normally
would have. The other part, the new part, "i != R_DSA_512"
is for fips because it does not allow small DSA modulus bits size, such as
512 bits.
So I think this would evaluate out as, if not in fips mode then do as
normally and dsa_doit[R_DSA_512] = 1, but if in fips mode and i =
R_DSA_512, then
do not set dsa_doit[R_DSA_512] = 1. So I think should be ok.

>
> - idea_set_encrypt_key(key16, &idea_ks);
> + if (doit[D_CBC_IDEA]) {
> + idea_set_encrypt_key(key16, &idea_ks);
> + }
>
> I think this should be ok. D_CBC_IDEA is defined to be 10. So, is index 10
in the "doit" array.
So if "idea" or "idea-cbc" is passed in as an argument, doit[D_CBC_IDEA] =
1; otherwise is 0.
So the change above sets the key only if we plan on doing idea crypto.
Later on, further down we use the key in following code snippet,
Notice when we use the key to encrypt, we do check first for
doit[D_CBC_IDEA]. So may actually be a general code improvement.

# ifndef OPENSSL_NO_IDEA
    if (doit[D_CBC_IDEA]) {
        for (j = 0; j < SIZE_NUM; j++) {
            print_message(names[D_CBC_IDEA], c[D_CBC_IDEA][j], lengths[j]);
            Time_F(START);
            for (count = 0, run = 1; COND(c[D_CBC_IDEA][j]); count++)
                idea_cbc_encrypt(buf, buf,
                                 (unsigned long)lengths[j], &idea_ks,
                                 iv, IDEA_ENCRYPT);
            d = Time_F(STOP);
            print_result(D_CBC_IDEA, j, count, d);
        }
    }
# endif

It also removes some of the upstream OpenSSL FIPS code, in
> crypto/cmac/cmac.c.
>

Yes, true. First, OPENSSL_FIPS was not previously defined, so this code was
never compiled nor executed before.
Over time, the FIPS standards have changed and so some of the prior code is
no longer applicable.
In the case of CMAC, those FIPS_* calls it was making within the
OPENSSL_FIPS define no longer existed.

>
> > The FIPs patch will not be included into the upstream source.
>
> Apparently there already is some FIPS support upstream (you pointed out
> https://www.openssl.org/docs/fips.html yourself, and the patch seems to
> disable that). Isn't there some middle ground where we could use the
> upstream tests and FIPS integration as much as possib...

Hi Martin,

This email addresses the second half, below.

regards,
Joy

On Wed, Apr 6, 2016 at 4:33 AM, Martin Pitt <email address hidden> wrote:

> The patch changes behaviour even in !fips mode, e. g. in apps/speed.c:
>
> for (i = 0; i < DSA_NUM; i++)
> - dsa_doit[i] = 1;
> + if (!FIPS_mode() || i != R_DSA_512)
> + dsa_doit[i] = 1;
>
> (additional check for R_DSA_512), and it even modifies code that doesn't
> touch any FIPS flag:
>
> - idea_set_encrypt_key(key16, &idea_ks);
> + if (doit[D_CBC_IDEA]) {
> + idea_set_encrypt_key(key16, &idea_ks);
> + }
>
> It also removes some of the upstream OpenSSL FIPS code, in
> crypto/cmac/cmac.c.
>
> > The FIPs patch will not be included into the upstream source.
>
> Apparently there already is some FIPS support upstream (you pointed out
> https://www.openssl.org/docs/fips.html yourself, and the patch seems to
> disable that). Isn't there some middle ground where we could use the
> upstream tests and FIPS integration as much as possible, and make the
> patch less ridiculously big?
>
> 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.
>
> There is some pointless whitespace change in e. g. crypto/evp/c_alld.c
> which further blow up the patch.
>
> There are a few extra calls to OPENSSL_init_library() now, without a
> comment. Is this guaranteed to be idempotent (it might reset seeds,
> counters and the like)? Is this a performance hit?
>
> crypto/evp/evp.h:
> -# define EVP_CIPH_FLAG_FIPS 0x4000
> +# define EVP_CIPH_FLAG_FIPS 0x400
>
> This also looks very dubious -- as this is a flag that is commonly
> passed to functions, this could be an ABI break.
>

I don't care for this change either. I will revert it. I tracked the
origins to the original openssl fips in openssl community, back then it was
0x400 and was changed recently to 0x4000.

> The added file crypto/fips/fips.h says "Copyright (c) 2003 The OpenSSL
> Project.", other files like crypto/fips/fips_drbg_hash.c have "Written
> by Dr Stephen N Henson (<email address hidden>) for the OpenSSL project". So
> this does not seem to be originating from Red Hat, SUSE or Canonical, or
> is that just a copy&paste error? If it actually comes from upstream
> somewhere, can these parts please split out into a separate patch (e. g.
> one for stuff that is being taken from some upstream branch, one that is
> being taken from RedHat/SUSE, and then perhaps one with the
> modifications from Canonical).
>

Yes, most of the source in this patch has openssl copyright and is from the
open source openssl community at some point in time.
From what I have gleaned, openssl community did their fi...

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
...

I reviewed the remainder of the patch:

crypto/evp/evp_locl.h
-# define SHA1_Init private_SHA1_Init
-# define SHA224_Init private_SHA224_Init
-# define SHA256_Init private_SHA256_Init
-# define SHA384_Init private_SHA384_Init
-# define SHA512_Init private_SHA512_Init
-# define DES_set_key_unchecked private_DES_set_key_unchecked

This looks like an API break. E. g. SHA1_Init is being used by tons of stuff in https://codesearch.debian.net/results/SHA1_Init .

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.

crypto/fips/fips.c, verify_checksums() : This dynamically swaps out a dlopen()ed libssl.so to libcrypto.so. This smells like a portion of the upstream OpenSSL approach with using a plugin? As this patch patches the original library source code, is that still actually needed?

Note: I mostly skipped over the fips/*_selftest.c bits, they are both structurally rather simple and also not verifiable at all for non-experts in the algorithms. The same goes for crypto/fips/fips_drbg_ctr.c and similar algorithms. There is some rather fiddly pointer arithmetic and assumptions about buffer sizes there -- has there been some vetting of this with running the tests both in FIPS and in normal mode through valgrind?

It also concerns me that crypto/fips/ seems to reimplement RNG, HMAC, and RSA algorithms which should already be in openssl itself. Yes, there might be politics involved, but have there been any attemps to at least consolidate this parts and work with upstream to unify the algorithms? It's certainly fine if some of them get disabled in FIPS mode, or augmented with extra runtime tests, but a complete reimplementation seems dubious -- it wouldn't be the first time that an US government promoted/approved RNG turned out to be a complete fraud, so some references about the origin of this to lower the scepticism would be appreciated. If that was really written by Steven Henson there should be little doubts about his credentials -- but again, it's not at all clear where these patches originate from. But particularly the reimplemented RNG (crypto/fips/fips_rand.c) has no author information at all.

crypto/fips/fips_utl.h contains the full definition of functions. This is rather unclean, and could lead to linker errors or at least duplicated symbols. It's only being included by two tests, but this is a potential bug if the file gets included into production code some day.

What is this doing in crypto/opensslconf.h.in?

+#ifndef OPENSSL_FIPS
+#define OPENSSL_FIPS
+#endif

I thought OPENSSL_FIPS was meant to be defined (or not) by configure to enable/disable FIPS support. This looks like it would now enable support even if it's disabled, and this depends on the order of #include in .c files that use opensslconf.h.

crypto/o_init.c disables checking for $OPENSSL_FORCE_FIPS_MODE. What's the rationale for this?

Hi Martin,

My responses below. Thanks!

regards,
Joy

On Thu, Apr 7, 2016 at 6:29 AM, Martin Pitt <email address hidden> wrote:

> I reviewed the remainder of the patch:
>
> crypto/evp/evp_locl.h
> -# define SHA1_Init private_SHA1_Init
> -# define SHA224_Init private_SHA224_Init
> -# define SHA256_Init private_SHA256_Init
> -# define SHA384_Init private_SHA384_Init
> -# define SHA512_Init private_SHA512_Init
> -# define DES_set_key_unchecked private_DES_set_key_unchecked
>
> This looks like an API break. E. g. SHA1_Init is being used by tons of
> stuff in https://codesearch.debian.net/results/SHA1_Init .
>

Those defines are within an OPENSSL_FIPS so were never used in regular
openssl.
The SHA ones were removed because the private_* routines above do not exist.
And the private_DES_set_key_unchecked was removed so doesn't exist either.
So, this should be ok.

>
> 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. Let me investigate further as to why it was not
upstreamed.
My original guess was because the upstream openssl-fips module offers new
FIPS_rsa_* routines that do this padding.

crypto/fips/fips.c, verify_checksums() : This dynamically swaps out a
> dlopen()ed libssl.so to libcrypto.so. This smells like a portion of the
> upstream OpenSSL approach with using a plugin? As this patch patches the
> original library source code, is that still actually needed?
>
>
Yes, this is needed. This is part of the integrity check that is required
by fips.
We dlopen libcrypto.so and dlsym FIPS_mode_set. I interpreted this as a
check to ensure we are running a fips capable libcrypto.so.
We get the path to the file and then dlclose. We then compute the hmac of
path/libcrypto.so and compare the results against the hmac we ship
to verify this is our binary. Same thing is done for libssl.so. We do not
yet include the hmac file to check against, so the integrity check fails
and prevents running in fips mode.

The openssl-fips module does this hmac integrity check of itself and
libcrypto.so differently.

> Note: I mostly skipped over the fips/*_selftest.c bits, they are both
> structurally rather simple and also not verifiable at all for non-
> experts in the algorithms. The same goes for crypto/fips/fips_drbg_ctr.c
> and similar algorithms. There is some rather fiddly pointer arithmetic
> and assumptions about buffer sizes there -- has there been some vetting
> of this with running the tests both in FIPS and in normal mode through
> valgrind?
>

No, at least not yet, but is a good idea.

> It also concerns me that crypto/fips/ seems to reimplement RNG, HMAC,
> and RSA algorithms which should already be in openssl itself. Yes, there
> might be politics involved, but have there been any attemps to at least
> consolidate this parts and work with upstream to unify the algorithms?
> It's certainly fine if some of them get disable...

Hi Martin,

Responses below. Thanks!

regards,
Joy

On Thu, Apr 7, 2016 at 5:27 AM, Martin Pitt <email address hidden> 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
> main...

Joy Latten [2016-04-08 5:17 -0000]:
> 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.

Right, the split patches will of course not be any smaller, but it'll
be a magnitude easier (or even make it feasible at all) to actually
maintain them.

So if the RedHat/Fedora patch already incorporates the files that were
taken from upstream FIPS, *and* RD/Fedora is maintaining this patch,
then a relatively simple split of "unmodified patch taken from Fedora
from $URL" and another "Ubuntu changes" patch would suffice.

If OTOH we cannot/don't want to rely on Fedora to maintain this
long-term, then please split it by the origins that do that
maintenance -- i. e. patches/files taken from the upstream FIPS
module, patches taken from SUSE, and again of course the Ubuntu
patches.

I. e. please split them by origin/sources for merging.

This is by far the biggest concern of mine here. I guess all my others
(doubtful algorithm reimplementation etc.) will probably stay as it's
not in your or my power to do much about it -- but we at least need to
know where which bit come from and where to update it from.

Thanks!

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!

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, a...

Code Review Resolutions:
1. Original one patch divided up into a patch-series of 6 patches. The first 5 patches are the original patches from fedora. The 6th patch authored by me to fix compiler warnings and use updated fips compliant algorithms and tests from upstream openssl and openssl fips module.
2. Restored error codes to those from openssl upstream and any news ones associated with fips were given a value of 200+
to avoid collisions with openssl upstream updates.
3. Restored defines that had been changed in evp/evp.h
4. Removed fips-prng references in fips-rand.c since no longer allowed in fips mode and was specifically added for fips.

New test package in https://launchpad.net/~j-latten/+archive/ubuntu/myppa

All testcases were run and succeeded.

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

New test package and debdiff. All the same testing completed successfully.
New test package, https://launchpad.net/~j-latten/+archive/ubuntu/myppa

> Dividing up the patch proved to be a challenge but was the right thing to do.

Many thanks for doing this!

Can you please fix the "Origin: http://dl.fedoraproject.org/pub/fedora/linux/development" fields still? They should point to a particular patch in a place like http://pkgs.fedoraproject.org/cgit/rpms/openssl.git/tree/, but that does not have "openssl-1.0.2g-fips-ctor.patch", only "openssl-1.0.2a-fips-ctor.patch". Although the patch there is almost identical, except for some patch header noise. So I suppose pointing to those is fine (bonus points if you just add the DEP-3 patch header but otherwise leave the patch intact, but that's not a biggie).

But e. g. your openssl-1.0.2g-fips-ec.patch has quite a lot of changes compared to http://pkgs.fedoraproject.org/cgit/rpms/openssl.git/plain/openssl-1.0.2a-fips-ec.patch (Note, Ubuntu modifications should go into openssl-1.0.2g-ubuntu-fips-cleanup.patch). Same for http://pkgs.fedoraproject.org/cgit/rpms/openssl.git/plain/openssl-1.0.2f-new-fips-reqs.patch.

Current Fedora rawhide's package is openssl1.0.2g as well, just like our's, so these patches ought to be identical?

Maybe you took them from a different branch, but the Fedora 24 version http://pkgs.fedoraproject.org/cgit/rpms/openssl.git/plain/openssl-1.0.2f-new-fips-reqs.patch?h=f24 is also different than your's.

> Weird, but the fedora patches were not independent of each other.

That's quite normal, and it would actually be a surprise if patches that are this big were independent.

I'll upload this now so that we can see the autopkgtests against this version, and we have at least a few days of testing this in the wild before the final release. But please still clean up the patches as above (Origin: and patches differing from Fedora) with a follow-up upload.

Thanks for bearing with me!

Hi Martin,

I will fix the Origin today. I was not sure of the naming convention for
the patches, so I kept the same name as in fedora but used the version of
openssl that we were patching. If you prefer, I can instead use exact same
name as fedora. I actually pulled my patches from Fedora Rawhide's source
tree,
https://dl.fedoraproject.org/pub/fedora/linux/development/rawhide/Everything/source/tree/Packages/o/
directory. I downloaded openssl source rpm and the fips patches were in the
SOURCES directory. The SRPM is openssl-1.0.2g-3.fc25.src.rpm. I used this
because it seem to be the most recent at the time.

I just did a diff with my ctor patch and the one in fedora's SRPM I used
and is pretty much the same.
Please advice if I should indicate above URL in Origin for DEP3 header and
use the exact same patch names.

Also, thanks so much Martin for helping me with all this!! :-)

On Wed, Apr 13, 2016 at 1:48 AM, Martin Pitt <email address hidden> wrote:

> > Dividing up the patch proved to be a challenge but was the right thing
> to do.
>
> Many thanks for doing this!
>
> Can you please fix the "Origin:
> http://dl.fedoraproject.org/pub/fedora/linux/development" fields still?
> They should point to a particular patch in a place like
> http://pkgs.fedoraproject.org/cgit/rpms/openssl.git/tree/, but that does
> not have "openssl-1.0.2g-fips-ctor.patch", only "openssl-1.0.2a-fips-
> ctor.patch". Although the patch there is almost identical, except for
> some patch header noise. So I suppose pointing to those is fine (bonus
> points if you just add the DEP-3 patch header but otherwise leave the
> patch intact, but that's not a biggie).
>
> But e. g. your openssl-1.0.2g-fips-ec.patch has quite a lot of changes
> compared to
> http://pkgs.fedoraproject.org/cgit/rpms/openssl.git/plain/openssl-1.0
> .2a-fips-ec.patch (Note, Ubuntu modifications should go into openssl-1.0
> .2g-ubuntu-fips-cleanup.patch). Same for
> http://pkgs.fedoraproject.org/cgit/rpms/openssl.git/plain/openssl-1.0
> .2f-new-fips-reqs.patch.
>
> Current Fedora rawhide's package is openssl1.0.2g as well, just like
> our's, so these patches ought to be identical?
>
> Maybe you took them from a different branch, but the Fedora 24 version
> http://pkgs.fedoraproject.org/cgit/rpms/openssl.git/plain/openssl-1.0
> .2f-new-fips-reqs.patch?h=f24 is also different than your's.
>
> > Weird, but the fedora patches were not independent of each other.
>
> That's quite normal, and it would actually be a surprise if patches that
> are this big were independent.
>
> I'll upload this now so that we can see the autopkgtests against this
> version, and we have at least a few days of testing this in the wild
> before the final release. But please still clean up the patches as above
> (Origin: and patches differing from Fedora) with a follow-up upload.
>
> Thanks for bearing with me!
>
> ** Changed in: openssl (Ubuntu)
> Status: Incomplete => In Progress
>
> ** Changed in: openssl (Ubuntu)
> Assignee: (unassigned) => Joy Latten (j-latten)
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1553309
>
> Title:
> [F...

> I was not sure of the naming convention for the patches, so I kept the same name as in fedora but used the version of openssl that we were patching.

The patch name is not that important. But it's very important to give the precise URL where you took it from, and that the patch actually matches the patch in that URL. This is the case for some of the patches, but not for openssl-1.0.2g-fips-ec.patch and openssl-1.0.2f-new-fips-reqs.patch, or you took them from a different place than http://pkgs.fedoraproject.org/cgit/rpms/openssl.git/.

> I downloaded openssl source rpm and the fips patches were in the SOURCES directory.

Ah, I see. But according to git, the fips-ec patch hasn't been changed in Fedora git for a year. Also, I downloaded the exact same srpm and compared patches -- the srpm has the same patches as Fedora git (not surprisingly), and the same differences towards the patches in your package:

$ interdiff -p1 fedora/openssl-1.0.2a-fips-ec.patch openssl-1.0.2g/debian/patches/openssl-1.0.2g-fips-ec.patch|diffstat
 b/crypto/fips/Makefile | 64 ---
 crypto/ec/ec2_smpl.c | 5
 crypto/ec/ec_curve.c | 4
 openssl-1.0.2a/crypto/fips/cavs/fips_ecdhvs.c | 456 ---------------------
 openssl-1.0.2a/crypto/fips/cavs/fips_ecdsavs.c | 486 -----------------------
 openssl-1.0.2a/crypto/fips/fips_ecdh_selftest.c | 242 -----------
 openssl-1.0.2a/crypto/fips/fips_ecdsa_selftest.c | 165 -------
 openssl-1.0.2a/version.map | 4
 8 files changed, 1426 deletions(-)

$ interdiff -p1 fedora/openssl-1.0.2f-new-fips-reqs.patch openssl-1.0.2g/debian/patches/openssl-1.0.2g-new-fips-reqs.patch|diffstat
 b/crypto/fips/fips_dh_selftest.c | 6
 b/crypto/fips/fips_ecdh_selftest.c | 240 ++++++++++++++++++++++++++++++++++++
 b/crypto/fips/fips_ecdsa_selftest.c | 165 ++++++++++++++++++++++++
 openssl-1.0.2f/crypto/bn/bn_rand.c | 8 -
 4 files changed, 411 insertions(+), 8 deletions(-)

For the record: http://people.canonical.com/~ubuntu-archive/proposed-migration/update_excuses.html#openssl looks good (linux/armhf still running, but that should not be relevant), but I blocked this to -proposed for now. I'll let this into xenial later tonight for testing, but we still need a followup upload with cleaning up the patch diffs and patch origins. Thanks!

Hi Martin,

Cool!
Started looking into those patch diffs...
for the openssl-1.0.2a-fips-ec.patch one, I had a bunch of undefined
symbols and so cleaned these up, causing my diff to be slightly off... my
bad.
Should have saved that for the last patch that was for my cleanup... sorry,
I hated not being able to get a clean compile in between patches. :-)
So let me move those changes into my cleanup patch.
Oh, and also, that patch installed "fips/cavs/fips_ecdhvs.c and
fips/cavs/fips_ecdsavs.c which are testcases I did not want to include. I
ignored them, but should have just removed them in my cleanup patch.

Do you agree that I should move these things into my cleanup patch? Let me
know and I will get it done today. This probably follows for the others too.
I am in my team sprint, but this is a priority.

regards,
Joy

On Wed, Apr 13, 2016 at 11:40 AM, Martin Pitt <email address hidden>
wrote:

> For the record: http://people.canonical.com/~ubuntu-archive/proposed-
> migration/update_excuses.html#openssl looks good (linux/armhf still
> running, but that should not be relevant), but I blocked this to
> -proposed for now. I'll let this into xenial later tonight for testing,
> but we still need a followup upload with cleaning up the patch diffs and
> patch origins. 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:
> In Progress
>
> 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
>

Joy Latten [2016-04-13 18:08 -0000]:
> Started looking into those patch diffs...
> for the openssl-1.0.2a-fips-ec.patch one, I had a bunch of undefined
> symbols and so cleaned these up, causing my diff to be slightly off... my
> bad.

Ah, that makes sense.

> Oh, and also, that patch installed "fips/cavs/fips_ecdhvs.c and
> fips/cavs/fips_ecdsavs.c which are testcases I did not want to include. I
> ignored them, but should have just removed them in my cleanup patch.

Is that really necessary? Adding two .c files seems rather harmless if
nothing refers to it, i. e. removing them from the Makefile only (in
the ubuntu patch) should suffice?

> Do you agree that I should move these things into my cleanup patch?

That would be good indeed, as it avoids confusion for the next person
who looks at this why the patches are different.

Please also update the Origin:, preferablyto the git.fedora ones as
then they are one click away from comparing/for updating.

Thank you!

Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)

This bug was fixed in the package openssl - 1.0.2g-1ubuntu3

---------------
openssl (1.0.2g-1ubuntu3) xenial; urgency=medium

  * Add fips support to openssl, LP: #1553309
    - debian/patches/openssl-1.0.2g-fips.patch: [PATCH 1/6] Add selftest, fips
      support, crypto compliance and define OPENSSL_FIPS.
    - debian/patches/openssl-1.0.2g-fips-ec.patch: [PATCH 2/6] Add fips compliance
      for EC curves.
    - debian/patches/openssl-1.0.2g-fips-md5-allow.patch: [PATCH 3/6] Allow md5 in
      fips mode.
    - debian/patches/openssl-1.0.2g-fips-ctor.patch: [PATCH 4/6] Re-factor integrity
      check for fips mode.
    - debian/patches/openssl-1.0.2g-new-fips-reqs.patch: [PATCH 5/6] New fips
      requirements.
    - debian/patches/openssl-1.0.2g-ubuntu-fips-cleanup.patch: [PATCH 6/6] Cleanup
      compiler warnings, use upstream error codes, DSA, DSA2, fips_utl.h; add
      additional upstream tests to fips_test_suite; allow all EC curves.

 -- Joy Latten <email address hidden> Tue, 12 Apr 2016 15:33:50 -0500

Ok, I will get to work on these changes now.
I will keep the first 5 patches original to fedora. And then in my cleanup
patch do the stuff to get rid of undefined symbols, etc...
And that way I can point my Origin to the git.fedora.

Thanks!!

regards,
Joy

On Wed, Apr 13, 2016 at 3:32 PM, Martin Pitt <email address hidden> wrote:

> Joy Latten [2016-04-13 18:08 -0000]:
> > Started looking into those patch diffs...
> > for the openssl-1.0.2a-fips-ec.patch one, I had a bunch of undefined
> > symbols and so cleaned these up, causing my diff to be slightly off... my
> > bad.
>
> Ah, that makes sense.
>
> > Oh, and also, that patch installed "fips/cavs/fips_ecdhvs.c and
> > fips/cavs/fips_ecdsavs.c which are testcases I did not want to include. I
> > ignored them, but should have just removed them in my cleanup patch.
>
> Is that really necessary? Adding two .c files seems rather harmless if
> nothing refers to it, i. e. removing them from the Makefile only (in
> the ubuntu patch) should suffice?
>
> > Do you agree that I should move these things into my cleanup patch?
>
> That would be good indeed, as it avoids confusion for the next person
> who looks at this why the patches are different.
>
> Please also update the Origin:, preferablyto the git.fedora ones as
> then they are one click away from comparing/for updating.
>
> Thank you!
>
> Martin
> --
> Martin Pitt | http://www.piware.de
> Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)
>
> --
> 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:
> In Progress
>
> 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
>

New debdiff with fixed Origin and cleaner fedora patches.

Hi Martin, my ppa has a debdiff that is against my prior version. You may find this more useful than the ppa I just attached above. here is a pointer, https://launchpadlibrarian.net/253756858/openssl_1.0.2g-1ubuntu3~ppa6_1.0.2g-1ubuntu3~ppa7.diff.gz

Hi Martin,
I also ran an interdiff when I re-factored to ensure alignment with original fedora patches. 2 or 3 of them did not apply cleanly, for various reasons, so I had to make very small changes. I also named each patch in debian/patches to be same as in fedora.

For interdiff of
openssl-1.0.2g-fips.patch, for some reason "Configure" shows up in diff yet I did not make any changes to patch. Visually compared to make sure code is the same and no regression.
openssl-1.0.2a-fips-ec.patch, we do not ship a "version.map" file, so when applying patch it prompts for location of file... so I removed it. So will show up in diff.
openssl-1.0.2a-fips-ctor.patch failed to apply altogether, because it is looking for a line of code that contains "secure_getenv" and not "getenv". upstream has "getenv" for that line of code, but fedora must have other patches applied before this one that changes it to "secure_getenv". So I corrected and this will show up in interdiff.

Corrected Origin in all the patches from fedora.

Hope this is all ok.

Also, ran same testing on latest ppa version (ppa7) and they all passed.

Thanks! There's still an awful amount of patch noise, but indeed some of it is unavoidable as you say. But this is incrementally better than before, thanks for the cleanup!

I uploaded this now: https://launchpad.net/ubuntu/+source/openssl/1.0.2g-1ubuntu4

Hi Martin,
I have a newbie question, what else should I do for this feature freeze?
Thanks! :-)

regards,
Joy

On Fri, Apr 15, 2016 at 12:14 AM, Martin Pitt <email address hidden>
wrote:

> Thanks! There's still an awful amount of patch noise, but indeed some of
> it is unavoidable as you say. But this is incrementally better than
> before, thanks for the cleanup!
>
> I uploaded this now: https://launchpad.net/ubuntu/+source/openssl/1.0
> .2g-1ubuntu4
>
> --
> 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:
> Fix Released
>
> 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
>

Hey Joy,

Joy Latten [2016-04-19 23:18 -0000]:
> I have a newbie question, what else should I do for this feature freeze?

Formally, nothing. The latest package is in xenial, so now it's "lean
back and enjoy", err, I mean "continue testing it" :-)

It would really be good and adequate if you subscribe to OpenSSL bug
reports here:

   https://launchpad.net/ubuntu/+source/openssl/+bugs

"Subscribe to bug mail" in the menu at the right. Just in case there
are some bugs/crashes reported that are related to FIPS (or maybe you
can help with the unrelated ones as well).

Thanks,

Martin
--
Martin Pitt | http://www.piware.de
Ubuntu Developer (www.ubuntu.com) | Debian Developer (www.debian.org)

I have subscribed to openssl bug reports.