Bionic: FastRTPS security tests fail

[mikaelarguedas]

On Bionic default openssl/libssl is 1.1.0:

With Connext

no support for 1.1.0 so we need to install custom version of 1.0.2 provided by RTI

With FastRTPS: it fails with "Invalid CA error".

Combinations tested:

• ca generated with 1.0.2(used in the tests) and fastrtps using SSL 1.1.0 runtime libs: fail
• ca generated with 1.1.0 and fastrtps using SSL 1.1.0 runtime libs: fail
• manually remove openssl/libssl 1.1.0 and install libssl and openssl 1.0.2 and generate certificates by hand: WORKS

Path forward

As I don't see forcing the use of openssl 1.0.2 as a viable path forward for bionic (as it conflicts with the system one and will force people to uninstall a ton of stuff), we need to figure out how to tweak certificate and key generation in sros2 to allow that use case.
A lead to explore is that is looks that our certificates are SSLv1 and fail, certs used in Fast-RTPS testing are SSLv3 and pass.

related to ros2/ros2#481

[mikaelarguedas]

CI jobs here

Associated PRs to review: #47 ros2/system_tests#265

Context for review:

This extension is now necessary with OpenSSL 1.1.
The reason is that we need to dissociate the certificates from the (root) certificate authority and the certificates generated and signed by the CA and used by our nodes.
We do that by specifying CA:true for the certificate of the Certificate Authority and CA:false for the other certificates (that belong to entities that are NOT Certificate Authorities)

Ready for review

[ruffsl]

@mikaelarguedas , In addition to key_cert_sign you used resolve this issue here, do you suppose there are any other extensions we should be adding to our auto generated x.509 certificates, like path_length to limit the length of CA chain could extended, or more common extentions needed for other transport certficiates like digital_signature, key_encipherment, key_agreement or ExtendedKeyUsage SERVER_AUTH and CLIENT_AUTH.

I don't think I ever read about any certificate extension requirements in the DDS security spec unlike the IETF's TLS spec, but perhaps OMG thought those would be self evident.

[mikaelarguedas]

Yeah I considered it but went for the minimal fix for now as this seems to match the extensions currently used but the DDS vendors we target. we can definitely consider other common extensions later on.

Regarding path length I didnt make assumptions on it, as currently it's always one but mostly because the certificate setup is still pretty primitive. But we could think of scenarios where this doesnt apply at which point it will mostly be putting an arbitrary number. Then enforcing a length of 1 for now wouldn't break anything.

I don't think I ever read about any certificate extension requirements in the DDS security spec ...

Yeah I had the same feeling and figured I'll take a closer look at the spec to see if it was just assumed good practice. I don't recall the spec covering certificate extensions at all but I haven't read 1.1 yet so maybe this is now covered.

[ruffsl]

Then enforcing a length of 1 for now wouldn't break anything.

To be conservative with the length, I think you'd want to set this is 0,
As then no derived child CA could ever be created using the root CA.
However, the root CA is self signed, so this more like a safety rather than a restriction on the authority,
given the root CA could re-sign and change its self imposed extensions.