Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve the Component Governance issue in the dotnet-helix-machines repository #13186

Closed
2 of 5 tasks
andriipatsula opened this issue Apr 17, 2023 · 29 comments
Closed
2 of 5 tasks
Assignees

Comments

@andriipatsula
Copy link
Member

andriipatsula commented Apr 17, 2023

  • This issue is blocking
  • This issue is causing unreasonable pain

The issue is blocking: #13166 and any PR to the dotnet-helix-machines repository.

Component Governance Component Detection: https://dev.azure.com/dnceng/internal/_build/results?buildId=2160248&view=logs&j=3dc8fd7e-4368-5a92-293e-d53cefc8c4b3&t=833edf1b-3669-5dbb-11e6-ee7d22230825&l=1967

Release Note Category

  • Feature changes/additions
  • Bug fixes
  • Internal Infrastructure Improvements

Release Note Description

Increased the installed Python cryptography and pyopenssl versions on most build and test agents to 41.0.1 and 23.2.0, respectively. Due to restrictions in our infrastructure, this change will be reflected on https://helix.dot.net as >=39.0.1 and >=23.0.0 rather than the exact versions installed.

The cryptography and pyopensll versions on all SLES 12 and Ubuntu 16.04 are significantly older due to platform issues we have not yet resolved. See dotnet/dnceng#293 and dotnet/dnceng#294 for details.

@garath
Copy link
Member

garath commented Apr 17, 2023

!30725 is merged!

@garath garath closed this as completed Apr 17, 2023
@riarenas riarenas reopened this Apr 19, 2023
@riarenas
Copy link
Member

The changes to pyopenssl and cryptography are being reverted in https://dnceng.visualstudio.com/internal/_git/dotnet-helix-machines/pullrequest/30805.

When we attempt this again, we need to make sure to test the changes in the following OSes by using the FORCE_QUEUE feature:

  • redhat7
  • sles12
  • ubuntu16

which are the older versions, and therefore the most sensitive to package updates.

@riarenas
Copy link
Member

I have extended the CG alert related to these changes while we figure how to get these older OSes compliant: https://dev.azure.com/dnceng/internal/_componentGovernance/dotnet-helix-machines/alert/7672729?typeId=15087584

@dougbu
Copy link
Member

dougbu commented Apr 21, 2023

I'm tempted to try just bumping cryptography to 3.4.8, rather than go all the way to 40.0.2. See latest CG alerts in almost any job w/in build #20230420.06. I'm having trouble finding the earliest version that CG won't warn about however. Thoughts❔

More generally, anyone know whether Python is something we can upgrade on older SLES and RHEL OSes❔

/cc @ilya-bin, @garath, @riarenas

@riarenas
Copy link
Member

More generally, anyone know whether Python is something we can upgrade on older SLES and RHEL OSes❔

Not without a lot of pain. SLES 12 especially has always been very sensitive to any python changes.

@garath
Copy link
Member

garath commented Apr 21, 2023

I can double-check but I believe SLES 12 is the reason for the current minimum version. Upgrading was never deemed a reasonable option.

@oleksandr-didyk
Copy link
Contributor

oleksandr-didyk commented Apr 26, 2023

The current image we have for SLES12 contains Python: 3.4.10

The version that CG finds acceptable for cryptography is 39.0.1, which needs python>=3.6

I tried checking out how hard would upgrading the Python version be, so I created a VM out of the produced image in Azure. On that VM, I cannot even install Python's dependencies to compile it due to:

:~> sudo zypper update -y
Refreshing service 'SUSE_Linux_Enterprise_Server_x86_64'.
Unexpected exception.
Receive: script died unexpectedly
History:
 - [11-Resource temporarily unavailable]

Please file a bug report about this.
See http://en.opensuse.org/Zypper/Troubleshooting for instructions.

Tried several ways of fixing the issue, no luck so far. I might be missing something or maybe there is a different way to verify if upgrading the version is a complete no-go.

Moreover, I see pyopenssl mentioned in the ticket and the PR, but:

  • the CG report does not contain an alert for that package
  • that package is not a transitive dependency of cryptography (source)

Is there a reason for why its upgrade is also required @andriipatsula? (since you were the author of the issue)

Additionally, maybe @garath or @riarenas have context into why upgrading Python on SLES 12 was never a reasonable option

@riarenas
Copy link
Member

riarenas commented Apr 26, 2023

Additionally, maybe @garath or @riarenas have context into why upgrading Python on SLES 12 was never a reasonable option

Nothing concrete, just that SLES 12 has had multiple issues related to python in the past.

@garath
Copy link
Member

garath commented Apr 26, 2023

IIRC there is no official, tested build of Python after 3.4 available for SLES12.

@oleksandr-didyk
Copy link
Contributor

oleksandr-didyk commented Apr 27, 2023

Thanks for the info

I tried today on to upgrade SLES 12 to Python 3.6 and it was quite headachy - the distro does not have the Python version we need, so it would need to be compiled from source. That alone had some issues with missing DLLs / C header files. I also don't know if having two Python installation (with the 3.6 one having a different executable name) is something we can even support.

All in all, it seems like a no-go due to problematic execution / potential high maintenance. With this in mind - would it make more sense to review the support for SLES 12 in the first place? If we still have to support it, how problematic would it be to silence this specific CG alert all together?

Additionally, our RedHat 7 image has Python 3.6.12, so cryptography should work just fine. How were the initial 3 images with issues identified? Was it some build error or some deployment failure? CC: @riarenas (since you were the one that reverted the change)

@riarenas
Copy link
Member

riarenas commented Apr 27, 2023

The image factory logs are long gone, but redhat 7 was failing to pip install with the upgrade. I don't remember if it was due to cartography or pyopenssl.

Submitting a test pr that forces the queue along with the update would give you fresh logs.

@garath
Copy link
Member

garath commented Apr 27, 2023

would it make more sense to review the support for SLES 12 in the first place?

It's certainly worth a discussion, but given that SLES12 is officially supported by .NET 6 and 7, we will likely need to have some ability to use this OS until 2024.

Unfortunately, this is also mostly a "dnceng" problem because it's only the Helix client that requires python. If we had, say, a .NET version, we could use that as long as .NET remains supported.

how problematic would it be to silence this specific CG alert all together

This is possible with a security review. Let's discuss the details internally.

@dougbu
Copy link
Member

dougbu commented May 1, 2023

Suggest we focus on the cryptography on SLES 12 question here. In particular, we need to investigate other workarounds for that platform (besides building python on the machines) before either asking .NET LT to agree to dropping the platform or requesting an exception.
I'll create a PR to show the current failure symptoms in a few minutes. I'll also create a separate issue and PR to bump the cryptography version where we think we can.

@dougbu
Copy link
Member

dougbu commented May 1, 2023

See test build !30980

@dougbu
Copy link
Member

dougbu commented May 2, 2023

Found problem when cryptography is bumped to v39.0.2. Can't even build the wheel, let alone install it on our images:

  File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\site-packages\urllib3\contrib\pyopenssl.py", line 46, in <module>
    import OpenSSL.SSL
  File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\site-packages\OpenSSL\__init__.py", line 8, in <module>
    from OpenSSL import crypto, SSL
  File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\site-packages\OpenSSL\crypto.py", line 1553, in <module>
    class X509StoreFlags(object):
  File "C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\site-packages\OpenSSL\crypto.py", line 1573, in X509StoreFlags
    CB_ISSUER_CHECK = _lib.X509_V_FLAG_CB_ISSUER_CHECK
AttributeError: module 'lib' has no attribute 'X509_V_FLAG_CB_ISSUER_CHECK'

Will try just going to v39.0.1.

This may be the reason we originally bumped pyopenssl. That additional bump led to problems w/ our current pip version as well as additional problems w/ Python 3.6 on RHEL 7. Doable but expands the scope of any exceptions or workarounds needed to additional platforms.

@dougbu
Copy link
Member

dougbu commented May 2, 2023

Side question @oleksandr-didyk:

The version that CG finds acceptable for cryptography is 39.0.1, which needs python>=3.6

Where did you confirm cryptography v39.0.1 (and nothing less) is acceptable❔ I'm hoping we could go back a bit further and still resolve the issue if 39.0.1 doesn't build w/ our other Python dependencies.

@oleksandr-didyk oleksandr-didyk removed their assignment May 2, 2023
@oleksandr-didyk
Copy link
Contributor

@dougbu I looked at the relevant CG alerts indotnet-helix-machines:

first

The versions of OpenSSL included in cryptography 0.8.1-39.0.0 are vulnerable to a security issue.

second:

Upgrade cryptography from 3.4.6 to 39.0.1 to fix the vulnerability.

@oleksandr-didyk
Copy link
Contributor

oleksandr-didyk commented May 2, 2023

FYI, de-assigned myself from the issue since I'm no longer on FR in Prague

@tkapin
Copy link
Member

tkapin commented May 2, 2023

@oleksandr-didyk - you were working on this the past week, what is the summary of your findings? What are your recommendations to follow up with?

@oleksandr-didyk
Copy link
Contributor

oleksandr-didyk commented May 2, 2023

@tkapin it is outlined in this comment -> #13186 (comment)

TLDR: having SLES 12 use Python 3.6.X is too expensive from implementation / maintenance perspective, we should look into other approaches (such as the requesting permission to silence this specific alert mentioned above).

One thing that I forgot to mention is that I created a post in our internal communications channels based on the suggestion from Michael, which resulted in creation of the issue that Doug has linked (#7377)

@tkapin
Copy link
Member

tkapin commented May 2, 2023

What version of SLES do we have? There is the SP5 released per https://www.suse.com/lifecycle/#suse-linux-enterprise-server-12, do we have the latest SP?

@oleksandr-didyk
Copy link
Contributor

What version of SLES do we have? There is the SP5 released per suse.com/lifecycle/#suse-linux-enterprise-server-12, do we have the latest SP?

We have SP5, which is the latest (source)

@dougbu dougbu self-assigned this May 3, 2023
@oleksandr-didyk
Copy link
Contributor

FYI, de-assigned myself from the issue since I'm no longer on FR in Prague

To some up my findings - as stated in the two CG reports for cryptography that we have in the repo (1 and 2) recommend upgrading to >=39.0.1.

The recommended version of cryptography requires >=Python3.6 (source), while our SLES 12 VM image has 3.4.10. As outlined in #13186 (comment), upgrading to the acceptable Python version proved to be quite headachy with increased maintenance for the solution down the line.

The newest version we can possibly install on SLES 12 is 3.3.2. +3.4.X would require at least Python 3.6 to be available on the machine.

I haven't tried building the wheel for the package using an older version of Python since there are API differences introduced in 3.6 and I deemed it as too much of a risk even if the wheel does get produced.

@tkapin
Copy link
Member

tkapin commented May 3, 2023

What are the available options to solve this problem then @oleksandr-didyk ?

@oleksandr-didyk
Copy link
Contributor

oleksandr-didyk commented May 3, 2023

What are the available options to solve this problem then @oleksandr-didyk Oleksandr Didyk FTE ?

Initially we could:

  • upgrade cryptography on the affected machine - as for reasons mentioned here I would argue that this option is too costly. From comments in this issue + internal comms it seems @dougbu is doing some additional work in this path and can verify / argue against this conclusion
  • request and exception for the this specific package - as mentioned in internal comms and in-person meetings this is something that is possible and has occurred in the past (not sure if the past instances related to Python packages or not). A potential issue with this solution is that some other alert can pop-up in the future relating in some shape or form to cryptography and we might need to silence it as well
  • drop support for the OS in the first place - no OS means no issues. However, as mentioned in discussions, this would receive great pushback from the product teams since .NET 6 and .NET 7 should support the OS + the original issue is not with the product itself, rather with the infra used for building / testing. Dropping support for an OS just because of that doesn't look plausible

As such from my perspective it looks like seeking an exception for the alert is the way to go.

@dougbu
Copy link
Member

dougbu commented Jun 5, 2023

update: on RHEL 7. v37.0.4 is the last binary distribution of cryptography available by default that matches that platform and v39.0.2 doesn't build correctly and the errors are a mess.

a few things may help:


I stopped trying to use the newer Python source packages for cryptography and found the following does help:

  • bump the pip, setuptools, and wheel package versions before trying to install cryptography
    • this installs cryptography v40.0.2 w/o any compilation i.e., the upgraded setuptools or wheel packages can find the right cryptography for RHEL 7
    • just updating pip is not sufficient
    • I updated the @development, rh-python36, and rh-python36-python-devel system packages w/ rhui-rhel-7-server-rhui-optional-rpms enabled before that attempt. need to recreate my VM to see if these upgrades were required…

I'm not sure how to confirm these packages work end to end w/o rebuilding on the Helix Machines CI but that's probably fine.

@dougbu
Copy link
Member

dougbu commented Jun 5, 2023

wow, my new VM didn't need anything other than a pip upgrade. I must have corrupted something on the previous test machine.

my new suspicion is that holding back the pip version on RHEL7 w/ comments about RHEL6 is Just Wrong:tm:. will test in CI…

@dougbu
Copy link
Member

dougbu commented Jun 26, 2023

After much experimentations and unblocking many problems (e.g., w/ FORCE_QUEUEs), this is finally waiting for rollout. dotnet/dnceng#293 and dotnet/dnceng#294 track work on the couple of images that still have an old cryptography version.

@dougbu
Copy link
Member

dougbu commented Jul 13, 2023

Fix rolled out on 12 July 2023.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants