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: Replace ECAlgorithm with EllipticCurve #5

Merged
merged 4 commits into from
Mar 5, 2019
Merged

fix: Replace ECAlgorithm with EllipticCurve #5

merged 4 commits into from
Mar 5, 2019

Conversation

Andrew-Lees11
Copy link
Contributor

This pull request replaces ECAlgorithm with Curve.

This is now a public struct that has a public representation of the curve for comparisons and checking on your key and all the internal variables that are used for encrypting and signing.

This was done to replace the "CurveID" String with an Struct version of enum limiting the comparable options.

@Andrew-Lees11 Andrew-Lees11 requested a review from djones6 March 5, 2019 13:28
Copy link
Contributor

@djones6 djones6 left a comment

Choose a reason for hiding this comment

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

Looks good, though this is a breaking change.

@@ -46,15 +46,14 @@ import OpenSSL
@available(OSX 10.13, *)
public class ECPrivateKey {
/// The Elliptic curve this key was generated from.
public let curveId: String
public let curve: Curve
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: change to public API

}

/// A prime256v1 curve.
public static let prime256v1 = Curve.p256
Copy link
Contributor

Choose a reason for hiding this comment

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

These are a new public API

@@ -48,7 +48,7 @@ import OpenSSL
@available(OSX 10.13, *)
public class ECPublicKey {
/// The Elliptic curve this key was generated from.
public let curveId: String
public let curve: Curve
Copy link
Contributor

Choose a reason for hiding this comment

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

Note: change to public API

@Andrew-Lees11
Copy link
Contributor Author

We have kept curveId so that this is no longer a breaking change.

/// A String description of the curve this key was generated from.
public var curveId: String {
return curve.description
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we leave the curveId as a constant and set as before during the init?

@djones6 djones6 changed the title fix: Replace ECAlgorithm with Curve fix: Replace ECAlgorithm with EllipticCurve Mar 5, 2019
@djones6 djones6 merged commit 6b7bc9e into master Mar 5, 2019
@Andrew-Lees11 Andrew-Lees11 deleted the curve branch March 5, 2019 14:45
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.

2 participants