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

Jump to bottom

Support for serde serialize and deserialize #48

Merged
merged 1 commit into from Nov 11, 2019
Merged

Support for serde serialize and deserialize #48

merged 1 commit into from Nov 11, 2019

Conversation

DebugSteven
Copy link
Collaborator

Start of adding support for serde serialize and deserialize

@DebugSteven
Copy link
Collaborator Author

This was supposed to be a draft PR. There isn't a way to switch it to be a draft PR after you open one so I'll leave this open until it's complete/closed.

@hdevalence
Copy link
Contributor

@mystor pointed out that the serde-related compilation failure happens because serde's derive by default expects the serde crate to be called serde, but that this can be overridden. I tested adding

#[cfg_attr(feature = "serde", serde(crate = "our_serde"))]

below each

#[cfg_attr(feature = "serde", derive(our_serde::Serialize, our_serde::Deserialize))]

and it resolved the problem.

@hdevalence
Copy link
Contributor

hdevalence commented Oct 25, 2019 •
edited

As noted in the PR, X25519 secret keys are unreduced scalars, but the serde implementation for curve25519-dalek Scalars rejects unreduced values on deserialization. I think it would be possible to use https://serde.rs/remote-derive.html to create a private stub struct which serializes the bytes of a scalar without the reduction check.

@DebugSteven DebugSteven force-pushed the serde-support branch 2 times, most recently from 36f0c4a to d2b32ee Compare October 26, 2019 00:29
@hdevalence hdevalence self-requested a review November 6, 2019 20:44
hdevalence
hdevalence reviewed Nov 6, 2019
View reviewed changes
Cargo.toml Show resolved Hide resolved
hdevalence
hdevalence reviewed Nov 6, 2019
View reviewed changes
src/x25519.rs Outdated Show resolved Hide resolved
hdevalence
hdevalence reviewed Nov 6, 2019
View reviewed changes
src/x25519.rs Outdated Show resolved Hide resolved
hdevalence
hdevalence reviewed Nov 6, 2019
View reviewed changes
src/x25519.rs Outdated Show resolved Hide resolved
hdevalence
hdevalence reviewed Nov 6, 2019
View reviewed changes
src/x25519.rs Outdated Show resolved Hide resolved
@hdevalence
Copy link
Contributor

This looks great!

@DebugSteven DebugSteven force-pushed the serde-support branch 2 times, most recently from 853cc8b to 0c3981d Compare November 11, 2019 15:45
hdevalence
hdevalence approved these changes Nov 11, 2019
View reviewed changes
Copy link
Contributor

@hdevalence hdevalence left a comment

Choose a reason for hiding this comment

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

LGTM!

@DebugSteven DebugSteven merged commit e253718 into dalek-cryptography:develop Nov 11, 2019
@DebugSteven DebugSteven deleted the serde-support branch November 11, 2019 22:34
@DebugSteven DebugSteven changed the title WIP: support for serde serialize and deserialize Support for serde serialize and deserialize Nov 22, 2019
@hdevalence hdevalence mentioned this pull request Nov 26, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hdevalence hdevalence hdevalence approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@DebugSteven @hdevalence