GMEllipticCurveCrypto icon indicating copy to clipboard operation
GMEllipticCurveCrypto copied to clipboard

Added Unit Tests. Fix building for 64 bit.

Open rhoiberg opened this issue 10 years ago • 10 comments

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.

rhoiberg avatar Mar 05 '15 03:03 rhoiberg

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

ricmoo avatar Mar 05 '15 08:03 ricmoo

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);

rhoiberg avatar Mar 05 '15 18:03 rhoiberg

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

ricmoo avatar Mar 05 '15 22:03 ricmoo

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 avatar Apr 30 '15 21:04 rhoiberg

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 avatar Apr 30 '15 23:04 rhoiberg

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

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

rhoiberg avatar May 15 '15 20:05 rhoiberg

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.

ricmoo avatar May 19 '15 16:05 ricmoo

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 avatar May 19 '15 17:05 rhoiberg

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

rhoiberg avatar May 27 '15 01:05 rhoiberg

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.

ricmoo avatar May 27 '15 01:05 ricmoo