microsoft / SCXcore

System Center Cross Platform Provider for Operations Manager
Microsoft Public License
36 stars 31 forks source link

Add support for building with SSL v1.1.0 #82

Closed sarojcare closed 7 years ago

sarojcare commented 7 years ago

The changes in CR is regarding OpenSSL 1.1 support in SCX.

Testing: Pbuild passed. Manual testing on Suse machine with openssl 0.9.8 and openssl 1.1.0 has passed.

Note: As at present omi 1.3.0-70 release bits are not available under omi-kits, build has done with copying these packages manually.

jeffaco commented 7 years ago

Note: I updated my review comment slightly - I think you need a temporary branch in SCXcore-kits so that consuming projects can test with this before we're ready to commit everything for final release. Be sure you read my review summary message from GitHub and not from the e-mail that was generated.

sarojcare commented 7 years ago

I'd like to see more about your test plan too:

Clearly you generated a universal kit with all three versions of SSL supported.

1.Did you try installing it on all relevant flavors? Your resulting kit should be tested on RPM 0.9.8 and RPM 1.0.0 (I guess we don't have RPM 1.1.0 yet), as well as Debian 0.9.8, Debian 1.0.0, and Debian 1.1.0 (Debian 9). To be sure this works, I think you're looking at a minimum of 5 installations on test systems. Extra credit if you test on relevant Ubuntu systems as well.

I tested the bits on following types of machines. I have tried install, Uninstall and couple of Enumeration scenaios.

Plat Form OpenSSL Version


Debian 9 1.1 Debian 5 0.9.8 Ubuntu 14 1.0 Suse 10[32 bit] 0.9.8 Suse 10 [64bit]) 0.9.8
AIX 1.0 Solaris-10 0.9.7

Also covered the certificate creation with scxsslconfig binary bits.

2.Did you verify that a "local build" (not combined kit) still works properly? Looks like we went out of our way to support that, it would be nice if it still worked.

Yes it works

3.It's good to get this into mainline. What changes are you making (if any) so that mainline builds will continue with SSL 1.1 working even though omi-kits doesn't have the SSL 1.1 bits yet? I don't see a branch to omi-kits with the new SSL 1.1 bits, and I don't see a change to Build-SCXcore repository to use it.

My concern is this: Can dependent projects (OMS) take this in master and run with it to do their integration regardless of release dates (from master)? As is, I don't believe that's the case. Frankly, as is, without the changes to omi-kits, I'm not even sure these changes won't break the nightly builds.

Planning to checking in omi 1.3.0-70 bundles into omi-kits directory temporarily. Once final bits are out we will check in the final bits. Let me know if you have any concerns with this approach.

I guess I'm asking for the following:

1.A clear test plan on exactly what you did to verify correctness, and

China team will do qualification of the bits on all supported platforms. I will share the test plans with restricted crypto strings.

2.A mechanism so this still works as expected in nightly master branch builds before OMI releases it's next version (with OpenSSL v1.1 support), and

Planning to checking in omi 1.3.0-70 bundles into omi-kits directory temporarily.

  1. When this is committed, you need to notify dependent projects (certainly OMS, probably Azure) that this is available for integration testing, but not yet ready for release (until OMI releases). And to make it available to them, I guess you need a temporary branch in SCXcore-kits , which they use to pick up your kits from (or which they should use to pick up your kits from).

Finally, when OMI is released, then the temporary branches in omi-kits and SCXcore-kits can be deleted and repository Build-SCXcore can be changed to go back to using the regular stuff.

sarojcare commented 7 years ago

I have re-assessed temporarily checking omi 1.3.0-70 dev kits into master omi-kits branch. Looks like it will cause more issues.

I will think about it from nightly build prospective and let you know the plans.

jeffaco commented 7 years ago

You said:

Planning to checking in omi 1.3.0-70 bundles into omi-kits directory temporarily. Once final bits are out we will check in the final bits. Let me know if you have any concerns with this approach.

You can't do this, as downstream providers, if they happen to build, will pick up the wrong bits. We won't allow that, omi-kits should always have the "right stuff". The solution here is to put your test version into a branch, and then modify infrastructure to use that branch. This way, your stuff is using the new branch, but everything else continues along like nothing ever happened.

Once OMI releases, we'll go ahead and put a tested & supported version in mainline. Then the temporary branch can be deleted, and that's that.

When you're all done, you should do something similar with scx-kits. That is, check in your working stuff (but not tested nor ready for release) into a branch. Then others can pick it up as needed, or continue using the master branch for tested/validated builds. That way, scx-kits still contains known good stuff in master, but if you want the new OpenSSL v1.1 test code, just grab it from another branch.

This is easy to do, and is the right way to handle this. If you have questions, reach out to me and I'll answer whatever questions you have.

sarojcare commented 7 years ago

I will check-in this once we have the release build from omi. Hence nothing to worry from breaking nightly builds prospective.