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

BUG: SSL: UNSAFE_LEGACY_RENEGOTIATION_DISABLED errors for Twilio sends #1284

Closed
1 of 3 tasks
cris-oddball opened this issue May 18, 2023 · 11 comments
Closed
1 of 3 tasks

Comments

@cris-oddball
Copy link

cris-oddball commented May 18, 2023

Description

On the deploy to PROD 5/17, numerous errors found in logs for Twilio sends, see additional info below.
Find the combo of OpenSSL, requests and urllib that all work together.
Related to the Alpine container OS upgrade.
Bug was not caught earlier but is reproducible in Perf when sending a Twilio message.

Steps to Reproduce

  • In perf, send a message through Twilio, observer logs for errors

Workaround

Prod was rolled back to resolve this last night in that env.
Possible urllib3 workaround noted in Additional Info below.

Impact/Urgency

High. Unable to deploy the current code to PROD; all Twilio messages currently will fail in dev, perf and staging.

Expected Behavior

  • Able to send messages via Twilio on the new container upgrade without SSL errors.

QA Considerations

  • when hotfix is deployed up the food chain, able to send messages via Twilio on the new container upgrade without SSL errors in perf, then staging.
  • Twilio sms send is added to QA Suite.

Additional Info & Resources

Example log - search "SSL: UNSAFE_LEGACY_RENEGOTIATION_DISABLED" on Perf morning of 5/18 for more examples. Some sensitive data - do not re-post in ticket.

requests.exceptions.SSLError: HTTPSConnectionPool(host='api.twilio.com', port=443): Max retries exceeded with url: /2010-04-01/Accounts/ACb1aa023dff676332332006a775c1f915/Messages.json (Caused by SSLError(SSLError(1, '[SSL: UNSAFE_LEGACY_RENEGOTIATION_DISABLED] unsafe legacy renegotiation disabled (_ssl.c:1131)')))
  • Github issue in urllib3 library - includes possible workaround.
  • Version 3.0.8-r4 of OpenSSL used in Alpine 3.17
  • Currently on urllib3 1.26.15 - have been using for a while
  • requests library?
@kalbfled
Copy link
Member

I tried upgrading requests and urllib3 in requirements.txt to the latest versions, which accommodate OpenSSL 3.0, but I encountered this Docker build error:

ERROR: Cannot install -r requirements-app.txt (line 17) and urllib3==2.0.2 because these package versions have conflicting dependencies.

The conflict is caused by:
    The user requested urllib3==2.0.2
    botocore 1.29.115 depends on urllib3<1.27 and >=1.25.4

The latest version of botocore, 1.29.135, has the same urllib3 dependency, so remedying our problem by upgrading Python packages won't work. I'm going to proceed with changing the Alpine image from 3.17 to 3.16, which packages OpenSSL v1.1.1.

kalbfled added a commit that referenced this issue May 18, 2023
…L v1.1.1, whereas Alpine v3.17 uses OpenSSL v3.0.8. The newer OpenSSL version causes problems with our Twilio integration until we can upgrade the "requests" and "urllib3" Python packages. #1284
@cris-oddball
Copy link
Author

Deployed to Perf.

Regression on Perf passes: 90 passed, 1 xfailed in 120.27s (0:02:00)

Twilio message is not sent. Will review logs. Notification ID for the test send is "id": "c8b27335-da3b-4aac-8941-d57355a15a23",

kalbfled added a commit that referenced this issue May 18, 2023
…s OpenSSL v1.1.1, whereas Alpine v3.17 uses OpenSSL v3.0.8. The newer OpenSSL version causes problems with our Twilio integration until we can upgrade the "requests" and "urllib3" Python packages. #1284"

This reverts commit e81a38c.
@cris-oddball
Copy link
Author

Re-deploying, sent that test message with an undeliverable number. Will re-test.

@cris-oddball
Copy link
Author

cris-oddball commented May 18, 2023

Deployed branch 1284-alpine-openssl to Perf (2nd try)

  • perf regression passes - 90 passed, 1 xfailed in 119.00s (0:01:58)
  • Twilio send is received, message status is delivered: id": "5db296b9-5ead-43d6-a84b-172b19da6228",
  • A callback was received for that message
  • Twistlock is even happy

@kalbfled feel free to open a PR for review, I will start writing test cases for the QA repo.

@cris-oddball
Copy link
Author

Merged to master, v1.6.3-1284-os is cut. I need to write the Twilio test cases, which may not be done until EOD Monday.

@cris-oddball
Copy link
Author

Tests written and working for dev and perf.
Staging requires the new container to be pushed to it, but we are sending a hotfix up to Prod, need to discuss with Kyle if he is just applying that to Prod or sending it up through Perf and Staging first.

@cris-oddball
Copy link
Author

Staging has the new container code, but I need the twilio phone number that can be used in Staging (the one from dev/perf doesn't work). @jakehova

@cris-oddball
Copy link
Author

This is blocked waiting for Kyle to discuss using a test Twilio number from the Enterprise account so that I can configure the QA suite with a number.
If we don't get one for staging, then I have to completely refactor the tests because send sms is parametrized and I can't just skip the two twilio tests the way they are written.
@k-macmillan @mjones-oddball

@cris-oddball
Copy link
Author

There is a way to skip a parametrized tests, so I did that for two of them, only if the suite is run in staging.

@cris-oddball
Copy link
Author

QA PR merged, closing ticket.

@kalbfled
Copy link
Member

See #1295.

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

No branches or pull requests

3 participants