Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add native lib src files and android binaries #1

Merged
merged 2 commits into from
Mar 2, 2023

Conversation

cmonfortep
Copy link
Collaborator

@cmonfortep cmonfortep commented Mar 2, 2023

Task/Issue URL: https://app.asana.com/0/0/1204068537088006/f

Description

Adds native library src code to repository.
Include android libsodium builds.

Structure will be:

  • root
  • root/native_lib/<sync_crypto_src>
  • root/native_lib/third-party/

Steps to test this PR

Feature 1

  • [ ]
  • [ ]

Copy link
Contributor

@ayoy ayoy left a comment

Choose a reason for hiding this comment

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

LGTM!

@cmonfortep
Copy link
Collaborator Author

@ayoy just removed the unuse method. good catch

@cmonfortep cmonfortep merged commit c6c2f69 into main Mar 2, 2023
#include "sodium.h"

// Seems mad that you still need to define this?!
#define min(x, y) (x < y) ? x : y

Choose a reason for hiding this comment

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

x and y should have parenthesis in the preprocessor definition, to prevent incorrect evaluation order when certain operators are used.

e.g.

min(3 | 4, 1 | 2);

Will be evaluated in the following order:

(3 | (4 < 1) | 2) ? (3 | 4) : (1 : 2)

Resulting in a return value of 7, when the correct answer is 3. See the example here:
https://godbolt.org/z/cvq34PbEh

https://en.cppreference.com/w/c/language/operator_precedence

Copy link
Contributor

Choose a reason for hiding this comment

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

@MarkIngramUK thanks for the comment, do you fancy opening a PR for this?

#define min(x, y) (x < y) ? x : y

// Contexts must be 8 characters long but are otherwise arbitrary
#define DDGSYNC_STRETCHED_PRIMARY_KEY_CONTEXT "Stretchy"

Choose a reason for hiding this comment

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

Forgive my question, I'm not familiar with libsodium, but, are these hash salts? If they are, is there any security concern about having the raw string values embedded in the binary?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what the best practice is tbh. But, if this was externalised then each consumer would have to pass in the same context as a parameter. All consumers will be open source (eventually) so I don't see any particularly added value in externalising. I will ping Stefan to validate.

Choose a reason for hiding this comment

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

These are not hash salts but the "context" which is a string describing what the key is going to be used for. It doesn't have to be secret, just unique for each key derived from the primary key.

Choose a reason for hiding this comment

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

Thanks for the clarification @stefanorri !

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants