Skip to content

ECC ecc_sign+verify_hash_raw > ecc_sign+verify_hash_rfc7518#235

Merged
karel-m merged 7 commits intodevelopfrom
pr/ecc_sign+verify_hash_rfc7518
Jun 21, 2017
Merged

ECC ecc_sign+verify_hash_raw > ecc_sign+verify_hash_rfc7518#235
karel-m merged 7 commits intodevelopfrom
pr/ecc_sign+verify_hash_rfc7518

Conversation

@karel-m
Copy link
Copy Markdown
Member

@karel-m karel-m commented Jun 21, 2017

related to #228 point 7/

@karel-m karel-m mentioned this pull request Jun 21, 2017
9 tasks
Copy link
Copy Markdown
Member

@sjaeckel sjaeckel left a comment

Choose a reason for hiding this comment

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

👍

Comment thread src/pk/ecc/ecc_sign_hash.c Outdated
int ecc_sign_hash_raw(const unsigned char *in, unsigned long inlen,
void *r, void *s,
prng_state *prng, int wprng, ecc_key *key)
static int ecc_sign_hash_ex(const unsigned char *in, unsigned long inlen,
Copy link
Copy Markdown
Member

@sjaeckel sjaeckel Jun 21, 2017

Choose a reason for hiding this comment

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

_ecc_sign_hash_ex

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

_ecc_sign_hash_ex
even better _ecc_sign_hash

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed

Comment thread src/pk/ecc/ecc_sign_hash.c Outdated
}

/* make up a key and export the public copy */
for (;;) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I just realized we're also probably looping here forever... as we should keep it consistent either we add a limit here or we remove it again in DH

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I have added max_iterations, I am just not sure if it is worth another new define.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if ((err = ltc_mp.ecc_ptmul(u1, mG, mG, m, 0)) != CRYPT_OK) { goto error; }
if ((err = ltc_mp.ecc_ptmul(u2, mQ, mQ, m, 0)) != CRYPT_OK) { goto error; }

/* find the montgomery mp */
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IIUC this removal leads to mp being used uninitialized.
If that's okay then we can also remove it entirely.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It was mistake, I was removing some code related to curves with custom A and removed also this one.

@sjaeckel sjaeckel modified the milestone: v1.18.0 Jun 21, 2017
Comment thread src/pk/ecc/ecc_sign_hash.c Outdated
void *e, *p;
int err;
void *r, *s, *e, *p;
int err, max_iterations = 20;
Copy link
Copy Markdown
Member

@sjaeckel sjaeckel Jun 21, 2017

Choose a reason for hiding this comment

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

probably we could introduce PK_MAX_RETRIES or something like that which can be used in all PK algorithms where we have to do retry-loops (c.f. DH_MAKE_KEY_MAX_ITERATIONS)? Otherwise we should use a separate one here as well.

@karel-m karel-m merged commit 6f85293 into develop Jun 21, 2017
@karel-m karel-m deleted the pr/ecc_sign+verify_hash_rfc7518 branch June 21, 2017 12:33
jforissier added a commit to jforissier/optee_os that referenced this pull request Jun 26, 2017
ecc_sign_hash_raw() and ecc_verify_hash_raw() have been removed from
upstream LibTomCrypt [1]. This patch imports the LTC modification and
updates tee_ltc_provider.c accordingly. Tested with: xtest -l 15 4006.

Link: [1] libtom/libtomcrypt#235
Link: libtom/libtomcrypt#228
Signed-off-by: Jerome Forissier <jerome.forissier@linaro.org>
Tested-by: Jerome Forissier <jerome.forissier@linaro.org> (HiKey)
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