| Message ID | 1388701922-5785-1-git-send-email-greearb@candelatech.com |
|---|---|
| State | Rejected |
| Headers | show |
On Thu, Jan 02, 2014 at 02:32:02PM -0800, greearb@candelatech.com wrote: > This error is caused by a crypto library that does not have > proper support for the requested feature, so warn loudly > about it. That is not the only reason for this failure and anyway, all the eap_sim_prf() callers are already using MSG_ERROR level message displaying the failure. > diff --git a/src/eap_common/eap_sim_common.c b/src/eap_common/eap_sim_common.c > static int eap_sim_prf(const u8 *key, u8 *x, size_t xlen) > { > - return fips186_2_prf(key, EAP_SIM_MK_LEN, x, xlen); > + int rv = fips186_2_prf(key, EAP_SIM_MK_LEN, x, xlen); > + if (rv < 0) { > + wpa_printf(MSG_ERROR, "EAP-SIM: Failed to derive keys: %i," > + " Compiled with INVALID CRYPTO LIBRARY?", rv); I don't think this would that helpful way of addressing this. Better fixes would be to add support for it in GnuTLS case or prevent that build.
On 01/07/2014 05:39 AM, Jouni Malinen wrote: > On Thu, Jan 02, 2014 at 02:32:02PM -0800, greearb@candelatech.com wrote: >> This error is caused by a crypto library that does not have >> proper support for the requested feature, so warn loudly >> about it. > > That is not the only reason for this failure and anyway, all the > eap_sim_prf() callers are already using MSG_ERROR level message > displaying the failure. I replaced two identical messages with a single message with some extra information about why the error might happen. It took quite a while for me to find that one small line of text and figure out it was indicating the problem, and more searching around in the code before I could figure out why. At least for the encryption libraries I looked at, that method cannot fail except when it is not implemented at all. >> diff --git a/src/eap_common/eap_sim_common.c b/src/eap_common/eap_sim_common.c >> static int eap_sim_prf(const u8 *key, u8 *x, size_t xlen) >> { >> - return fips186_2_prf(key, EAP_SIM_MK_LEN, x, xlen); >> + int rv = fips186_2_prf(key, EAP_SIM_MK_LEN, x, xlen); >> + if (rv < 0) { >> + wpa_printf(MSG_ERROR, "EAP-SIM: Failed to derive keys: %i," >> + " Compiled with INVALID CRYPTO LIBRARY?", rv); > > I don't think this would that helpful way of addressing this. Better > fixes would be to add support for it in GnuTLS case or prevent that > build. A build-time failure would be better, but I haven't had time to work on that yet. Thanks, Ben
2014/1/8 Ben Greear <greearb@candelatech.com>: > A build-time failure would be better, but I haven't had time to work > on that yet. I made an attached patch for this. [PATCH] EAP-SIM: Prevent build for invalid TLS implementation Currently EAP-SIM could not be used with GnuTLS, Microsoft CryptoAPI and NSS. So prevent build by #error. Signed-hostap: Masashi Honma <masashi.honma@gmail.com> Regards, Masashi Honma. 2014/1/8 Ben Greear <greearb@candelatech.com>: > On 01/07/2014 05:39 AM, Jouni Malinen wrote: >> On Thu, Jan 02, 2014 at 02:32:02PM -0800, greearb@candelatech.com wrote: >>> This error is caused by a crypto library that does not have >>> proper support for the requested feature, so warn loudly >>> about it. >> >> That is not the only reason for this failure and anyway, all the >> eap_sim_prf() callers are already using MSG_ERROR level message >> displaying the failure. > > I replaced two identical messages with a single message > with some extra information about why the error might > happen. It took quite a while for me to find that one > small line of text and figure out it was indicating the problem, > and more searching around in the code before I could figure out why. > > At least for the encryption libraries I looked at, that > method cannot fail except when it is not implemented at > all. > >>> diff --git a/src/eap_common/eap_sim_common.c b/src/eap_common/eap_sim_common.c >>> static int eap_sim_prf(const u8 *key, u8 *x, size_t xlen) >>> { >>> - return fips186_2_prf(key, EAP_SIM_MK_LEN, x, xlen); >>> + int rv = fips186_2_prf(key, EAP_SIM_MK_LEN, x, xlen); >>> + if (rv < 0) { >>> + wpa_printf(MSG_ERROR, "EAP-SIM: Failed to derive keys: %i," >>> + " Compiled with INVALID CRYPTO LIBRARY?", rv); >> >> I don't think this would that helpful way of addressing this. Better >> fixes would be to add support for it in GnuTLS case or prevent that >> build. > > A build-time failure would be better, but I haven't had time to work > on that yet. > > Thanks, > Ben > > -- > Ben Greear <greearb@candelatech.com> > Candela Technologies Inc http://www.candelatech.com > > _______________________________________________ > HostAP mailing list > HostAP@lists.shmoo.com > http://lists.shmoo.com/mailman/listinfo/hostap
On 01/07/2014 11:01 PM, Masashi Honma wrote: > 2014/1/8 Ben Greear <greearb@candelatech.com>: >> A build-time failure would be better, but I haven't had time to work >> on that yet. > > I made an attached patch for this. > > [PATCH] EAP-SIM: Prevent build for invalid TLS implementation > > Currently EAP-SIM could not be used with GnuTLS, Microsoft CryptoAPI and NSS. > So prevent build by #error. I'm not sure that is the proper way to do this. I was thinking that in the USIM code we would need to check for the crypto type and then add an #error in the USIM code if using wrong crypto type. Possibly other places in the code that might call that fips186_2_prf can handle the failure more gracefully, but I did not check. Thanks, Ben > > Signed-hostap: Masashi Honma <masashi.honma@gmail.com> > > Regards, > Masashi Honma. > > 2014/1/8 Ben Greear <greearb@candelatech.com>: >> On 01/07/2014 05:39 AM, Jouni Malinen wrote: >>> On Thu, Jan 02, 2014 at 02:32:02PM -0800, greearb@candelatech.com wrote: >>>> This error is caused by a crypto library that does not have >>>> proper support for the requested feature, so warn loudly >>>> about it. >>> >>> That is not the only reason for this failure and anyway, all the >>> eap_sim_prf() callers are already using MSG_ERROR level message >>> displaying the failure. >> >> I replaced two identical messages with a single message >> with some extra information about why the error might >> happen. It took quite a while for me to find that one >> small line of text and figure out it was indicating the problem, >> and more searching around in the code before I could figure out why. >> >> At least for the encryption libraries I looked at, that >> method cannot fail except when it is not implemented at >> all. >> >>>> diff --git a/src/eap_common/eap_sim_common.c b/src/eap_common/eap_sim_common.c >>>> static int eap_sim_prf(const u8 *key, u8 *x, size_t xlen) >>>> { >>>> - return fips186_2_prf(key, EAP_SIM_MK_LEN, x, xlen); >>>> + int rv = fips186_2_prf(key, EAP_SIM_MK_LEN, x, xlen); >>>> + if (rv < 0) { >>>> + wpa_printf(MSG_ERROR, "EAP-SIM: Failed to derive keys: %i," >>>> + " Compiled with INVALID CRYPTO LIBRARY?", rv); >>> >>> I don't think this would that helpful way of addressing this. Better >>> fixes would be to add support for it in GnuTLS case or prevent that >>> build. >> >> A build-time failure would be better, but I haven't had time to work >> on that yet. >> >> Thanks, >> Ben >> >> -- >> Ben Greear <greearb@candelatech.com> >> Candela Technologies Inc http://www.candelatech.com >> >> _______________________________________________ >> HostAP mailing list >> HostAP@lists.shmoo.com >> http://lists.shmoo.com/mailman/listinfo/hostap
diff --git a/src/eap_common/eap_sim_common.c b/src/eap_common/eap_sim_common.c index e1773bf..f7d7ae0 100644 --- a/src/eap_common/eap_sim_common.c +++ b/src/eap_common/eap_sim_common.c @@ -21,7 +21,12 @@ static int eap_sim_prf(const u8 *key, u8 *x, size_t xlen) { - return fips186_2_prf(key, EAP_SIM_MK_LEN, x, xlen); + int rv = fips186_2_prf(key, EAP_SIM_MK_LEN, x, xlen); + if (rv < 0) { + wpa_printf(MSG_ERROR, "EAP-SIM: Failed to derive keys: %i," + " Compiled with INVALID CRYPTO LIBRARY?", rv); + } + return rv; } @@ -79,7 +84,6 @@ int eap_sim_derive_keys(const u8 *mk, u8 *k_encr, u8 *k_aut, u8 *msk, u8 *emsk) u8 buf[EAP_SIM_K_ENCR_LEN + EAP_SIM_K_AUT_LEN + EAP_SIM_KEYING_DATA_LEN + EAP_EMSK_LEN], *pos; if (eap_sim_prf(mk, buf, sizeof(buf)) < 0) { - wpa_printf(MSG_ERROR, "EAP-SIM: Failed to derive keys"); return -1; } pos = buf; @@ -144,7 +148,6 @@ int eap_sim_derive_keys_reauth(u16 _counter, wpa_hexdump(MSG_DEBUG, "EAP-SIM: XKEY'", xkey, SHA1_MAC_LEN); if (eap_sim_prf(xkey, buf, sizeof(buf)) < 0) { - wpa_printf(MSG_ERROR, "EAP-SIM: Failed to derive keys"); return -1; } if (msk) {
From: Ben Greear <greearb@candelatech.com> This error is caused by a crypto library that does not have proper support for the requested feature, so warn loudly about it. Signed-hostap: Ben Greear <greearb@candelatech.com> --- src/eap_common/eap_sim_common.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)