Additional FIPS-related test changes

atheriel commented 2 years ago

This PR does the following:

Adds a suite of tests specifically for FIPS. This systematically pulls various cases that are skipped and tests that they generate the right error messages. It also tests the print() related changes.
Reorder the cert and key tests to skip as little as possible.

After this I see the following on CentOS7:

Linking to: OpenSSL 1.0.2k-fips  26 Jan 2017 (FIPS)
<snip>
-- Skipped tests  -------------------------------------------------
* fips_mode() is TRUE (34)
* packageVersion("curl") < "4.3.3" is TRUE (1)

[ FAIL 0 | WARN 0 | SKIP 35 | PASS 375 ]

And on RHEL8 (really, Rocky 8):

Linking to: OpenSSL 1.1.1k  FIPS 25 Mar 2021 (FIPS)
<snip>
-- Skipped tests  -------------------------------------------------
* fips_mode() is TRUE (37)
* packageVersion("curl") < "4.3.3" is TRUE (1)

[ FAIL 0 | WARN 0 | SKIP 38 | PASS 392 ]

jeroen commented 2 years ago

This actually breaks the test suite with FIPS RHEL 8. You can use a rocky-8 container to test, for example

docker run --rm -it opencpu/rocky-8 bash

That will have R preinstalled, so you only have to clone openssl and run export OPENSSL_FORCE_FIPS_MODE=1

jeroen commented 2 years ago

Btw I have pushed out a release of openssl to CRAN, I'll get back to this later.

atheriel commented 2 years ago

I'll see if I can track down the inconsistency.

atheriel commented 2 years ago

OK, it turns out there are some very interesting things going on here. CentOS7 uses OpenSSL 1.0.2, while RHEL8 uses 1.1.1. Some tests related to PKCS1, PKCS12, and DSA pass on the former but not the latter, both in FIPS mode. What is happening?

PKCS1 Support

The PKCS1 format -- also called the "traditional" format -- uses MD5 and cannot be made to use another cipher. So they should not be permitted when running in FIPS mode.

But it turns out that on 1.0.2, if you attempt to create a PKCS1 format private key, OpenSSL will silently produce a PKCS8 format key instead:

> library(openssl)
Linking to: OpenSSL 1.0.2k-fips  26 Jan 2017 (FIPS)
> sk1 <- read_key("tests/keys/id_dsa")
> fips_mode()
[1] TRUE
> write_pkcs1(sk1, password = "test", path = "example.pkcs1")
> readLines("example.pkcs1")[1]
[1] "-----BEGIN ENCRYPTED PRIVATE KEY-----"

(A normal PKCS1 file would begin with BEGIN DSA PRIVATE KEY.)

This does not happen with 1.1.1. I suppose the OpenSSL authors regard it as a bad choice, in retrospect. You can compare the implementations for 1.0.2 and 1.1.1 to see the difference.

This is why those tests do not need to be skipped on CentOS7. But I don't see any reason to test this workaround, so I think you are right to skip the tests on all FIPS systems.

PKCS12 Support

Prior to version 3, OpenSSL's default encryption for certifications was "40 bit RC2". Running the following on a non-FIPS system with OpenSSL 1.1.1:

> library(openssl)
> fips_mode()
[1] FALSE
> write_p12(ca = ca_bundle(), path = "ca.p12", password = "cats")

and inspecting it with the CLI:

$ openssl pkcs12 -info -in ca.p12 -noout
Enter Import Password:
MAC: sha1, Iteration 1
MAC length: 20, salt length: 8
PKCS7 Encrypted data: pbeWithSHA1And40BitRC2-CBC, Iteration 2048
<snip>

You can see the "40BitRC2" part. This algorithm is not FIPS approved. However, if you try to create a PKCS12 file with OpenSSL 1.0.2 in FIPS mode:

> library(openssl)
Linking to: OpenSSL 1.0.2k-fips  26 Jan 2017 (FIPS)
> fips_mode()
[1] TRUE
> write_p12(ca = ca_bundle(), path = "ca.p12", password = "cats")

This call will not error. Instead, you will see the following if you inspect it:

$ openssl pkcs12 -info -in ca.p12 -noout
Enter Import Password:
MAC: sha1, Iteration 1
MAC length: 20, salt length: 8
PKCS7 Encrypted data: pbeWithSHA1And3-KeyTripleDES-CBC, Iteration 2048
<snip>

That is, this version of OpenSSL swaps in an approved algorithm by default. It turns out that version 1.1.1 does not do this swapping, so it will fail. I actually think we should add a workaround for this, but I will do so in a different PR.

Key Generation

I looks like you can generate a 1024-bit DSA key on OpenSSL 1.0.2 in FIPS mode, even though keys shorter than 2048 should be prohibited:

> library(openssl)
Linking to: OpenSSL 1.0.2k-fips  26 Jan 2017 (FIPS)
> fips_mode()
[1] TRUE
> key <- dsa_keygen(1024)
> key$size
[1] 1024

This does not work on 1.1.1:

> library(openssl)
Linking to: OpenSSL 1.1.1k  FIPS 25 Mar 2021 (FIPS)
> fips_mode()
[1] TRUE
> key <- dsa_keygen(1024)
Error: OpenSSL error in dsa_builtin_paramgen2: key size invalid

After much, much digging I determined that this is because CentOS7 permits smaller keys to be generated for backwards compatibility. You have to set a special environment variable to meet FIPS approval:

> library(openssl)
Linking to: OpenSSL 1.0.2k-fips  26 Jan 2017 (FIPS)
> fips_mode()
[1] TRUE
> Sys.setenv("OPENSSL_ENFORCE_MODULUS_BITS" = "1")
> key <- dsa_keygen(1024)
Error: OpenSSL error in dsa_builtin_paramgen: key size invalid

I think I'll add a test specifically for this.

atheriel commented 2 years ago

Tests are now passing on both RHEL8 and CentOS7 platforms.

jeroen commented 2 years ago

Does this also pass in openssl3? You can test using e.g. fedora:

docker run --rm -it opencpu/fedora bash

atheriel commented 2 years ago

@jeroen That image runs Fedora 35, which only has 1.1.1. I'll try to get a FIPS OpenSSL 3 environment together when possible.

atheriel commented 2 years ago

No, the FIPS-related tests have no effect on version 3 -- at least on my test Fedora 36 environment.

The reason is that this commit added code that loads the non-FIPS modules unconditionally, making it impossible to actually be in FIPS mode (where only the fips and base modules are present). I'm not sure why this change was made, but I imagine it was because things were not working without it.

I don't know exactly how to fix the version 3-specifc issues yet, but I think it's out of scope for this PR.

atheriel commented 2 years ago

Update: tests pass on Fedora 36 with OpenSSL 3 when not in FIPS mode just fine. Many tests do indeed fail in new ways when running in FIPS mode. But again, I think that's out of scope for this PR.

jeroen / openssl