Message ID | 20240812171649.163687-1-prestwoj@gmail.com (mailing list archive) |
---|---|
State | Accepted, archived |
Headers | show |
Series | sae: support default group for H2E | expand |
Context | Check | Description |
---|---|---|
tedd_an/pre-ci_am | success | Success |
prestwoj/iwd-alpine-ci-fetch | success | Fetch PR |
prestwoj/iwd-ci-gitlint | success | GitLint |
prestwoj/iwd-ci-fetch | success | Fetch PR |
prestwoj/iwd-alpine-ci-setupell | success | Prep - Setup ELL |
prestwoj/iwd-ci-setupell | success | Prep - Setup ELL |
prestwoj/iwd-ci-makedistcheck | success | Make Distcheck |
prestwoj/iwd-ci-incremental_build | success | Incremental build not run PASS |
prestwoj/iwd-alpine-ci-makedistcheck | success | Make Distcheck |
prestwoj/iwd-alpine-ci-incremental_build | success | Incremental build not run PASS |
prestwoj/iwd-ci-build | success | Build - Configure |
prestwoj/iwd-alpine-ci-build | success | Build - Configure |
prestwoj/iwd-ci-makecheckvalgrind | success | Make Check w/Valgrind |
prestwoj/iwd-ci-clang | success | clang PASS |
prestwoj/iwd-ci-makecheck | success | Make Check |
prestwoj/iwd-alpine-ci-makecheckvalgrind | success | Make Check w/Valgrind |
prestwoj/iwd-alpine-ci-makecheck | success | Make Check |
prestwoj/iwd-ci-testrunner | success | test-runner PASS |
Hi James, On 8/12/24 12:16 PM, James Prestwood wrote: > This was seemingly trivial at face value but doing so ended up > pointing out a bug with how group_retry is set when forcing > the default group. Since group_retry is initialized to -1 the > increment in the force_default_group block results in it being > set to zero, which is actually group 20, not 19. This did not > matter for hunt and peck, but H2E actually uses the retry value > to index its pre-generated points which then breaks SAE if > forcing the default group with H2E. > > To handle H2E and force_default_group, the group selection > logic will always begin iterating the group array regardless of > SAE type. > --- > src/sae.c | 41 ++++++++++++++++++----------------------- > 1 file changed, 18 insertions(+), 23 deletions(-) Applied. Do we have test for H2E & force behavior? Regards, -Denis
Hi Denis, On 8/12/24 11:14 AM, Denis Kenzior wrote: > Hi James, > > On 8/12/24 12:16 PM, James Prestwood wrote: >> This was seemingly trivial at face value but doing so ended up >> pointing out a bug with how group_retry is set when forcing >> the default group. Since group_retry is initialized to -1 the >> increment in the force_default_group block results in it being >> set to zero, which is actually group 20, not 19. This did not >> matter for hunt and peck, but H2E actually uses the retry value >> to index its pre-generated points which then breaks SAE if >> forcing the default group with H2E. >> >> To handle H2E and force_default_group, the group selection >> logic will always begin iterating the group array regardless of >> SAE type. >> --- >> src/sae.c | 41 ++++++++++++++++++----------------------- >> 1 file changed, 18 insertions(+), 23 deletions(-) > > Applied. Do we have test for H2E & force behavior? Meant to send that, its on the list now. > > Regards, > -Denis >
diff --git a/src/sae.c b/src/sae.c index 9bce8faa..97c0af05 100644 --- a/src/sae.c +++ b/src/sae.c @@ -152,39 +152,34 @@ static int sae_choose_next_group(struct sae_sm *sm) { const unsigned int *ecc_groups = l_ecc_supported_ike_groups(); bool reset = sm->group_retry >= 0; + unsigned int group; - /* - * If this is a buggy AP in which group negotiation is broken use the - * default group 19 and fail if this is a retry. - */ - if (sm->sae_type == CRYPTO_SAE_LOOPING && sm->force_default_group) { - if (sm->group_retry != -1) { - l_warn("Forced default group but was rejected!"); - return -ENOENT; - } - - sae_debug("Forcing default SAE group 19"); + /* Find the next group in the list */ + while ((group = ecc_groups[++sm->group_retry])) { + /* + * Forcing the default group; only choose group 19. If we have + * already passed 19 (due to a retry) we will exhaust all other + * groups and should fail. + */ + if (sm->force_default_group && group != 19) + continue; - sm->group_retry++; - sm->group = 19; + /* Ensure the PT was derived for this group */ + if (sm->sae_type == CRYPTO_SAE_HASH_TO_ELEMENT && + !sm->handshake->ecc_sae_pts[sm->group_retry]) + continue; - goto get_curve; + break; } - do { - sm->group_retry++; + if (!group) + return -ENOENT; - if (ecc_groups[sm->group_retry] == 0) - return -ENOENT; - } while (sm->sae_type != CRYPTO_SAE_LOOPING && - !sm->handshake->ecc_sae_pts[sm->group_retry]); + sm->group = group; if (reset) sae_reset_state(sm); - sm->group = ecc_groups[sm->group_retry]; - -get_curve: sae_debug("Using group %u", sm->group); sm->curve = l_ecc_curve_from_ike_group(sm->group);