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

Properly decode password from aws using m2crypto #47989

Merged
merged 2 commits into from Jun 6, 2018

Conversation

Projects
None yet
4 participants
@dwoz
Contributor

dwoz commented Jun 6, 2018

M2Crypto requires bytes when loading keys. Fixes issue #47955

Tests written?

Yes

Commits signed with GPG?

Yes

@dwoz dwoz requested review from terminalmage and rallytime Jun 6, 2018

@salt-jenkins salt-jenkins requested a review from saltstack/team-cloud Jun 6, 2018

@gtmanfred

This comment has been minimized.

Contributor

gtmanfred commented Jun 6, 2018

This has some lint problems.

@rallytime

This looks good to me but there were several lint errors. I have fixed them here: dwoz#1

(I wanted to keep this PR moving, so the back-port to 2018.3.1 has the original fix, and my lint fix.)

Once that lint fix is merged in, this looks good to me.

@@ -4762,7 +4762,7 @@ def get_password_data(
rsa_key = kwargs['key']
pwdata = base64.b64decode(pwdata)
if HAS_M2:
key = RSA.load_key_string(rsa_key)
key = RSA.load_key_string(rsa_key.encode())

This comment has been minimized.

@terminalmage

terminalmage Jun 6, 2018

Member

When you do not pass an explicit encoding to .encode(), Python 2 will use its default encoding (i.e. ascii). Are we certain that the key will only include ascii? If not, we should be using salt.utils.stringutils.to_bytes(rsa_key).

This comment has been minimized.

@dwoz

dwoz Jun 6, 2018

Contributor

Yes, Keys are only ever ascii. We could actually make that an explicit ascii decode.

@dwoz dwoz force-pushed the dwoz:awscloud branch from 5ca468c to 23ab272 Jun 6, 2018

rallytime added a commit that referenced this pull request Jun 6, 2018

@dwoz dwoz merged commit 1ce7d6c into saltstack:2018.3 Jun 6, 2018

7 of 9 checks passed

jenkins/PR/salt-pr-rs-cent7-n Pull Requests » Salt PR - RS CentOS 7 #19603 — ABORTED
Details
default Build finished.
Details
WIP ready for review
Details
jenkins/PR/salt-pr-clone Pull Requests » Salt PR - Clone #25746 — SUCCESS
Details
jenkins/PR/salt-pr-docs-n Pull Requests » Salt PR - Docs #17813 — SUCCESS
Details
jenkins/PR/salt-pr-linode-cent7-py3 Pull Requests » Salt PR - Linode CentOS 7 - PY3 #5548 — SUCCESS
Details
jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests » Salt PR - Linode Ubuntu14.04 #23480 — SUCCESS
Details
jenkins/PR/salt-pr-linode-ubuntu16-py3 Pull Requests » Salt PR - Linode Ubuntu16.04 - PY3 #10520 — SUCCESS
Details
jenkins/PR/salt-pr-lint-n Pull Requests » Salt PR - Code Lint #22444 — SUCCESS
Details

@dwoz dwoz deleted the dwoz:awscloud branch Jul 2, 2018

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