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

Changed en/decryption cipher to aes-256-cbc to work with Electron 4+. #880

Merged
merged 2 commits into from
Apr 17, 2019

Conversation

tonyanziano
Copy link
Contributor

@tonyanziano tonyanziano commented Apr 17, 2019

Fixes #microsoft/BotFramework-Emulator#1431 (comment)

Description

The Emulator recently upgraded from Electron 2.0.12 to Electron 4.1.1. Electron 4 ships with only one SSL library (BoringSSL) instead of the two (BoringSSL & OpenSSL) in previous versions. (Source #1)

This means that many of the recognized hashing functions used in the crypto library have been cut, one of which is the aes256 function which is used by botframework-config. This breaks any encryption & decryption in the emulator, so encrypted bots can no longer be used.

However, aes256 was just an alias for aes-256-cbc. (Source #2) (Source #3) This means that we can just change the cipher name to aes-256-cbc and we will achieve backwards compatibility while no longer breaking the Emulator.

Specific Changes

  • changed the cipher name used in en/decryption from aes256 to aes-256-cbc

Testing

image

@coveralls
Copy link

coveralls commented Apr 17, 2019

Pull Request Test Coverage Report for Build #2202

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.05%) to 88.287%

Totals Coverage Status
Change from base Build #2200: 0.05%
Covered Lines: 3237
Relevant Lines: 3524

💛 - Coveralls

@cwhitten cwhitten requested a review from cleemullins April 17, 2019 20:55
@cleemullins cleemullins requested a review from jspruance April 17, 2019 21:20
@cleemullins
Copy link
Contributor

This looks good to me, but I would like @jspruance to signoff.

@cleemullins
Copy link
Contributor

... I assume this same change needs to be made to the V3 JS SDK?

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

Successfully merging this pull request may close these issues.

4 participants