Skip to content

[Batch] Fix #15464: Update check for pfx file without password in batch create_certificate#15509

Merged
qwordy merged 1 commit intoAzure:devfrom
Zay2k:fix_batch_certificate_password
Oct 29, 2020
Merged

[Batch] Fix #15464: Update check for pfx file without password in batch create_certificate#15509
qwordy merged 1 commit intoAzure:devfrom
Zay2k:fix_batch_certificate_password

Conversation

@Zay2k
Copy link
Contributor

@Zay2k Zay2k commented Oct 13, 2020

Description

Add a check for file ending when determining certificate format of certificate uploaded to batch account. When a .pfx-certificate is uploaded without a password, the extra check is needed to not parse it as .cer-file.


This checklist is used to make sure that common guidelines for a pull request are followed.

@Zay2k Zay2k requested a review from bgklein as a code owner October 13, 2020 18:56
@ghost ghost added the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Oct 13, 2020
@ghost
Copy link

ghost commented Oct 13, 2020

Thank you for your contribution Zay2k! We will review the pull request and get back to you soon.

@Zay2k
Copy link
Contributor Author

Zay2k commented Oct 13, 2020

An improvement of this would be to do the check with OpenSSL.crypto or similar, like in keyvault module.

x509 = crypto.load_pkcs12(certificate_data).get_certificate()

@yonzhan
Copy link
Collaborator

yonzhan commented Oct 13, 2020

Batch

@yungezz
Copy link
Member

yungezz commented Oct 14, 2020

hi @qwordy , could you pls review the PR? thanks

@yungezz yungezz added the Batch az batch label Oct 14, 2020
@yungezz yungezz added this to the S177 milestone Oct 14, 2020
@yungezz yungezz modified the milestones: S177, S178 Oct 28, 2020
@yungezz
Copy link
Member

yungezz commented Oct 28, 2020

hi @Zay2k @qwordy what's the status of this PR, is it ok to merge?

@Zay2k
Copy link
Contributor Author

Zay2k commented Oct 28, 2020

hi @Zay2k @qwordy what's the status of this PR, is it ok to merge?

It is ok for me. What's the next step, should I merge dev into this, or do you just merge it?

@qwordy
Copy link
Member

qwordy commented Oct 29, 2020

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@qwordy qwordy merged commit d8d95f9 into Azure:dev Oct 29, 2020
@Zay2k Zay2k deleted the fix_batch_certificate_password branch October 29, 2020 10:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Batch az batch customer-reported Issues that are reported by GitHub users external to the Azure organization.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants