-
Notifications
You must be signed in to change notification settings - Fork 459
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
remove bouncy castle dependency from edgeHub #6019
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good..
- Did you try removing the dependency as well?
- Did you get a chance to test it as well? Does it work?
- Is this all that we need to do?
I have not removed the dependency to the package yet, just the 'using'-s. I've put together this code quickly only just to see the magnitude of the work, but it was easier than I thought. |
|
||
public static class CertificateHelper | ||
{ | ||
// The private-key import on windows randomly seems failing, however according to the tests, the second time | ||
// after a failure it usually works. The number below is just a "big enough" number randomly chosen for | ||
// self-healing, but gives a limit to avoid endless try. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to combine the 2 fixes? I would rather keep them separate... unless we need to do this as part of BC removal, for some reason.
// self-healing, but gives a limit to avoid endless try. | ||
const int MaxCertImportRetryCount = 10; | ||
|
||
private static Oid oidRsaEncryption = Oid.FromFriendlyName("RSA", OidGroup.All); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: remove private qualifier
string ecLabel = "EC PRIVATE KEY"; | ||
|
||
var keyAlgorithm = certificate.GetKeyAlgorithm(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: remove empty lines..
|
||
try | ||
{ | ||
// On Windows the certificate in 'result' gives an error when used with kestrel: "No credentials are available in the security" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is Windows only, maybe we can keep it to release/1.1 only?
static X509Certificate2 AttachPrivateKey(X509Certificate2 certificate, string pemEncodedKey) | ||
{ | ||
int retryCount = 0; | ||
while (retryCount++ < MaxCertImportRetryCount) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this code is Windows specific.. then maybe we don't need it in master?
Is this still applicable? I do see PKCS-8 support. |
…still has reference to bouncy castle
…iotedge into vipeller/bouncy_remove
#6051 should fix the E2E checkin build failure. |
/azp run |
Azure Pipelines successfully started running 6 pipeline(s). |
A quick solution to see how bouncy castle could be removed. <del>What missing are: <del>- error handling <del>- Import apparently does not like PKCS-8 Now it contains the full solution
A quick solution to see how bouncy castle could be removed. <del>What missing are: <del>- error handling <del>- Import apparently does not like PKCS-8 Now it contains the full solution
A quick solution to see how bouncy castle could be removed.
What missing are:
- error handling
- Import apparently does not like PKCS-8Now it contains the full solution