Make Connext use Connext's openssl on all platforms

[mikaelarguedas]

Ubuntu Focal, Homebrew and chocolatey now provide OpenSSL 1.1.1
RTI Connext 5.3.1 supports only 1.0.2 so we need to modify the library path and the path for connext's version of openssl be used when running tests with connext. Use system OpenSSL for other rmw implementations

related to ros2/ci#421

[cottsay]

Could you please create a PR to update the ROS 2 installation instructions to set the appropriate environment variables to make this change work?

[mikaelarguedas]

I'm not sure I got what you are referring to, could you please clarify which instructions you're thinking of?

I don't see the "normal" installation instructions detailing how to setup the security plugins for Connext. Are you referring to this page https://github.com/ros2/ros2_documentation/blob/master/source/Installation/Install-Connext-Security-Plugins.rst ? (noting that that page is not distro or platform specific)

Also the RTI_OPENSSL_* are made-up variables used only on CI and in this package. So they may not belong on official instructions

[mikaelarguedas]

If the documentation update is not a blocker it would be awesome to get this PR reviewed / merged to fix ci on MacOS and hopefully unblock ros2/ci#421

[cottsay]

Special goo for our CI system doesn't give me the warm fuzzies 🙁 but since we're just making the existing behavior consistent across platforms, this seems like an OK change to make.

I was looking for documentation so that if/when we spin-up additional macOS workers on ci.ros2.org, we make sure that they get the older openssl installed and get the magic variables set. If it isn't documented anywhere, it's probably not going to happen.

@mjcarroll, did you follow any directions besides the official ROS 2 documentation when you set up lore?

[nuclearsandwich]

@mjcarroll, did you follow any directions besides the official ROS 2 documentation when you set up lore?

Lore was being shared between the OSRF and ROS 2 CI farms so it had additional setup performed for that. I've undone some of the Jenkins-related things that were done but not any other operations, which I also don't have an enumeration of.

[nuclearsandwich]

Also the RTI_OPENSSL_* are made-up variables used only on CI and in this package. So they may not belong on official instructions

If we have to fabricate variables for our CI to work we should at least share what we needed to do to get it working. Even if we don't tell people to replicate our variables, explaining what function they serve and how to achieve the same effect if they want to use connext security should IMO be documented

[nuclearsandwich]

I talked about this PR with @cottsay a little bit and my least favorite thing about it is that it's being implemented "at runtime" in the tests rather than something we can configure as behavior whenever security is being used with connext. I can't think of a way to pin this behavior at the rmw_connext level that isn't worse than the problem it solves.

[nuclearsandwich]

• Linux
• Linux-aarch64
• macOS
• Windows

[mikaelarguedas]

If it isn't documented anywhere, it's probably not going to happen.

True

Even if we don't tell people to replicate our variables, explaining what function they serve and how to achieve the same effect if they want to use connext security should IMO be documented

Yeah for a bit more history, it is needed on CI only because we want to use that specific version only for Connext but want to be able to use the system one 1.1.1* for the other implementations.
Another reason the code (now commented) had to happen at runtime was because MacOS with SIP stripped the DYLD_LIBRARY_PATH, so setting that ahead of time had no effect.

I've been refraining from changing https://github.com/ros2/ros2_documentation/blob/master/source/Installation/Install-Connext-Security-Plugins.rst that is written + maintained by RTI. That page does already specify to install their version of OpenSSL.

Best way forward is still to use a version of Connext that works with OpenSSL 1.1.1* (like Connext 6.0.1) ;) and remove all custom workarounds

[mikaelarguedas]

For this change to fix CI, these RTI_OPENSSL_* variables need to be set on all macOS machines. Looks like the one used for testing this didnt have them set so the connext security tests failed

[mikaelarguedas]

@cottsay @nuclearsandwich friendly 🛎️ is there anything we can do to move this forward ?

There are currently no rmw implementation with passing security tests on MacOS, so this is challenging for validating new features before the freeze

[nuclearsandwich]

@cottsay @nuclearsandwich friendly bellhop_bell is there anything we can do to move this forward ?

I set up mini2 with Connext's openssl 1.0.2n installed to /Applications/rti_connext_5.3.1/openssl-1.0.2n/...

I posted this branch to ros2/ci which sets the RTI_OPENSSL_BIN and RTI_OPENSSL_LIBS environment variables to match the installation on mini2 and ran https://ci.ros2.org/job/test_ci_osx/308/ on mini2. With 128 security test failures it seems like either I did something wrong or there's an issue with the patch. I haven't had a chance to investigate further.

[nuclearsandwich]

I'd love to address these test failures as it would cut down on the macos noise A LOT. I just don't have the personal bandwidth atm to do much but change configs and run tests. Feel free to take that CI branch and iterate on it if need be.

[mikaelarguedas]

no code changes just a rebase.
Corresponding CI PR and jobs at ros2/ci#436

[sloretz]

@mikaelarguedas thanks for figuring this out. Is this then what needs to happen?

• Make sure lore, mini1, mini2, and mini3 have environment variables RTI_OPENSSL_BIN and RTI_OPENSSL_LIBS set to the 1.0.2 openssl version in some global way
  • It sounds like they already have this based on the passing build badges in the description of Use OpenSSL 1.0.2 for Connext on MacOS ci#436
• Investigate why the connext security test failures still happened on mini3
• Merge Use OpenSSL 1.0.2 for Connext on MacOS ci#436 which sets these variables if they're not set
• Merge this PR

If that's correct, it sounds like We could merge this and ros2/ci#426 now since it resolves the failures on most of the machines, and that's better than the current state of test failures on all of the machines. Then mini3 can be investigated separately. How does that sound?

[mikaelarguedas]

This PR had an unwanted side effect on windows which is now fixed at 28c4488.

• Windows
  Using this change (windows security tests succeed)

Is this then what needs to happen?

Yes as far as MacOS is concerned.
Ideally the MacOS machines would also be updated to use the latest openssl from brew before the release

For windows:

• Update windows to use the latest openssl: non-EOL openssl on windows ci#421
• Figure out why the tests are failing https://ci.ros2.org/job/ci_windows/10424

[clalancette]

This looks reasonable to me. I'm going to kick off one more CI on all platforms (just for Connext) to make sure it passes.

@mikaelarguedas Assuming that comes back green, can I just merge this? Does it have any other dependencies?

[clalancette]

• Linux
• Linux-aarch64 <-- expected failure, no connext on aarch64
• macOS
• Windows

[mikaelarguedas]

Assuming that comes back green, can I just merge this? Does it have any other dependencies?

Merging as is should have no impact (what already fails will keep failing, what passes will keep passing) but give us the groundwork to fix things on top.
For this to improve things on macos you'll need ros2/ci#436

[clalancette]

aarch64 is expected to fail for connext-only. macOS is already failing the builds that are failing. Thus I'll merge this one.

[nuclearsandwich]

Thanks @mikaelarguedas, and @clalancette for getting this over the edge.