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

Clarify ambiguity in encryption spec/example #87

Merged
merged 1 commit into from Jun 10, 2021
Merged

Clarify ambiguity in encryption spec/example #87

merged 1 commit into from Jun 10, 2021

Conversation

tomstoneham
Copy link
Contributor

From #43, as well as packets.go, the final flag should be the first item in each payload packet in an encrypted message. However, the example currently places the final flag last in the payload packet, introducing ambiguity for those implementing the protocol. This commit changes the example to conform to the actual implementation behaviour.

From #43, as well as [packets.go](1), the final flag should be the first
item in each payload packet in an encrypted message. However, the
example currently places the final flag last in the payload packet,
introducing ambiguity for those implementing the protocol. This commit
changes the example to confirm to the actual implementation behaviour.

1: https://github.com/keybase/saltpack/blob/e19b1910c0c5e2e309b248ea32d1d05e6b868dc4/packets.go#L62
@tomstoneham
Copy link
Contributor Author

Not sure why CI is failing, this commit only changes files in spec/. Might need to take a look at .travis.yml.

@oconnor663
Copy link
Contributor

Thanks for the correction! It took me a while to page all this back in, but I think the original confusion might've been that the final flag is the last field (of 2) in the signcryption mode, but the first field (of 3) here in the encryption-only mode.

@oconnor663 oconnor663 merged commit abd2f0d into keybase:master Jun 10, 2021
@oconnor663
Copy link
Contributor

@malgorithms can you do a pull for this fix on the webserver?

@tomstoneham
Copy link
Contributor Author

Thanks for the quick merge @oconnor663! Have a good one.

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.

None yet

2 participants