Skip to content

Conversation

@rhoiberg
Copy link

@rhoiberg rhoiberg commented Mar 5, 2015

and more stringent errors.

Please take a look at these changes. They were necessary to compile with our project settings. I also added a project with a unit test that exercises storing the private key in the iOS keychain.

and more stringent errors.
@ricmoo
Copy link
Owner

ricmoo commented Mar 5, 2015

Heya rhoiberg,

There are a lot of changes here, and some file(s) that shouldn't be checked in (i.e. the .xcuserstate file, the entries in the Info.plist that are company specific, et cetera).

I like the idea of using XCTest, but haven't delved far enough into it yet. But the test cases are not very holistic, compared to test.m, which is admittedly far too home-brew... I would like to move to a real unit test framework. Adding secp256k1 is higher on the list of features though. :)

These changes also move the library into a full blown iOS App and include a lot more files (Xcode boiler plate and whatnot), which is beyond the scope of this simple library.

What build settings caused the need for the forward declarations?

I will certainly update the library to use NSInteger though, as that makes more sense, especially going forward with 64-bit architecture.

Thanks!
RicMoo

@rhoiberg
Copy link
Author

rhoiberg commented Mar 5, 2015

The unit test was a to offer an example of how to store the EC keys in the keychain and retrieve them from the keychain for signing. The changes to the library file are the most important.

The application does not really do anything, it was only created for the unit test. Thanks for this great open source library!

In addition to the NSUInteger changes, the following forward declarations were needed:

int ecc_make_key(uint8_t *p_publicKey, uint8_t *p_privateKey, int NUM_ECC_DIGITS, uint64_t *curve_p, uint64_t *curve_n, uint64_t *curve_GX, uint64_t *curve_GY);
int ecdh_shared_secret(const uint8_t *p_publicKey, const uint8_t *p_privateKey, uint8_t *p_secret, int NUM_ECC_DIGITS, uint64_t *curve_p, uint64_t *curve_b);
int ecdsa_sign(const uint8_t *p_privateKey, const uint8_t *p_hash, uint8_t *p_signature, int NUM_ECC_DIGITS, uint64_t *curve_p, uint64_t *curve_n, uint64_t *curve_GX, uint64_t *curve_GY);
int ecdsa_verify(const uint8_t *p_publicKey, const uint8_t *p_hash, const uint8_t *p_signature, int NUM_ECC_DIGITS, uint64_t *curve_p, uint64_t *curve_b, uint64_t *curve_n, uint64_t *curve_GX, uint64_t *curve_GY);

@import Foundation;

NSData *derEncodeInteger(NSData *value);
NSData *derEncodeSignature(NSData *signature);
NSRange derDecodeSequence(const unsigned char *bytes, NSUInteger length, NSUInteger index);
NSRange derDecodeInteger(const unsigned char *bytes, NSUInteger length, NSUInteger index);
NSData *derDecodeSignature(NSData *der, NSUInteger keySize);

@ricmoo
Copy link
Owner

ricmoo commented Mar 5, 2015

Do you know, by chance, which build flags caused the forward declarations to be required? I will add that flag to the Makefile.

@rhoiberg
Copy link
Author

Sorry it took me so long to get back to this. Here are the settings we use in Xcode. Pretty much all warnings are errors. There are also three analyzer warnings about garbage values. Could you please look into these? I am working on a library to support the FIDO protocol that we hope to open source that will use your code. As I was saying, the unit test that I submitted demonstrates how to store the private key in the iOS keychain or in the secure enclave for iPhone 5s and better. I now know how to create a stand alone unit test, if you have any interest.

warning
warnings2

@rhoiberg
Copy link
Author

I have updated the unit test so that you might find it more useful. Feel free to use what you think is useful to your project. Also added a project that has the same warning settings. I think the one you might be looking for is:

GCC_WARN_ABOUT_MISSING_PROTOTYPES = YES

@rhoiberg
Copy link
Author

@ricmoo are you still interested in working with us on this?

Later is OK. Would just like to know that you have interest.

@ricmoo
Copy link
Owner

ricmoo commented May 19, 2015

Hey rhoiberg!

Yes, I would still like to pull in the forward declarations and NSInteger changes (I need to reproduce the failure to compile due to warnings first though).

I'm a little wary about adding an xcodeproj file and depending on XCTest.h as these are Xcode specific and the keychain stuff is very iOS and OS X specific. It is certainly something people may be interested in though, dealing with private keys, so I would like to include it in an examples folder. Thus far, most people have not been storing any private key on the device, and using the library mostly for verifying a signature against a provided public key.

I should get around to this soon, it's just been busy the last few weeks.

@rhoiberg
Copy link
Author

I just added the project so that you could decide how to best use it. The main thing I need is that fixes for the forward declarations, NSInteger changes and to fix the code path that will result in garbage values indicated by the Xcode analyzer.

@rhoiberg
Copy link
Author

@ricmoo I worked with another developer and we believe we have fixed the analyzer warnings about garbage data. Please review.

@ricmoo
Copy link
Owner

ricmoo commented May 27, 2015

Wow. Why does that affect it?? And why does only the Y co-ord need to be cleared in ecc_make_key?

I should hopefully have time this weekend to get this looked at.

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