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

Fix android failed build #128

Merged
merged 3 commits into from
Apr 9, 2021
Merged

Conversation

aik-jahoda
Copy link
Contributor

Add certificates needed for android build.
Follow up for dotnet/runtime#50230

cc @elinor-fung

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (assuming the files work on Android)

@elinor-fung
Copy link
Member

elinor-fung commented Mar 30, 2021

It looks like these added certs are still using RC2 for encryption, which is not supported on Android. We need these .noRC2 files versions to not use RC2 encryption. Dumping info for one of the added certs under TestDataCertificates:

>  openssl pkcs12 -in ./TestDataCertificates/testclienteku.contoso.com.noRC2.pfx -info                      
...
PKCS7 Encrypted data: pbeWithSHA1And40BitRC2-CBC, Iteration 2048

The .noRC2 versions of all the certs were created in order to avoid using RC2 (they use TripleDES instead). Dumping info for one of the certs under TestData:

> openssl pkcs12 -in ./TestData/testclienteku.contoso.com.noRC2.pfx -info 
...
PKCS7 Encrypted data: pbeWithSHA1And3-KeyTripleDES-CBC, Iteration 2000

@aik-jahoda
Copy link
Contributor Author

I added new commit,
the info is:

PKCS7 Encrypted data: pbeWithSHA1And3-KeyTripleDES-CBC, Iteration 2048

@elinor-fung
Copy link
Member

I just realized that the versions of testselfsignedclienteku.contoso.com.pfx and testselfsignedservereku.contoso.com.pfx under the TestDataCertificates folder are also using RC2 now. The existing versions in the TestData folder were already only using TripleDES instead of RC2, which is why we didn't have to add noRC2 versions originally. Could you create noRC2 versions of those too? Sorry, I know this is obnoxious.

@wfurt
Copy link
Member

wfurt commented Apr 2, 2021

I'm wondering if we should make the noRC2 simply the default rather than creating separate files.
I had similar problem as some super new versions of openssl (maybe in combination with distro) disable very old ciphers - like rc2 so we may see more failures going forward.

@elinor-fung
Copy link
Member

cc @bartonjs for thoughts on noRC2/RC2 as the default for cert test data
I'd guess we'd hit the same problems / go with the same default for X509Certificates.TestData

@bartonjs
Copy link
Member

bartonjs commented Apr 2, 2021

thoughts on noRC2/RC2 as the default for cert test data

We probably want to keep some in X509Certificates, since that's how Windows XP generated PFXes (and everyone else copied that behavior), but outside X509 it's not important... they can all be 3DES so long as they open everywhere.

@aik-jahoda
Copy link
Contributor Author

I'll remove all *.noRC2.pfx and replace *.pfx with triple des.

@aik-jahoda
Copy link
Contributor Author

@elinor-fung can you please verify the current version fulfil your requirements?

Copy link
Member

@elinor-fung elinor-fung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified that we can load these on Android. Thanks, @aik-jahoda!

@aik-jahoda aik-jahoda merged commit 7b3780a into dotnet:main Apr 9, 2021
@aik-jahoda aik-jahoda deleted the jajahoda/fixAndroid branch April 9, 2021 13:27
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