'hostapd_cli sta <mac-addr>' returned 'dot1xAuthSessionUsername=(null)', when the integrated eap_server was used. This fixes that so the actual username is returned.

Message ID 1505361607-32045-1-git-send-email-Michael.Baird@ecs.vuw.ac.nz
State New
Headers show
Series
  • 'hostapd_cli sta <mac-addr>' returned 'dot1xAuthSessionUsername=(null)', when the integrated eap_server was used. This fixes that so the actual username is returned.
Related show

Commit Message

Michael Baird Sept. 14, 2017, 4 a.m.
See also http://lists.infradead.org/pipermail/hostap/2017-September/037933.html

Signed-off-by: Michael Baird <Michael.Baird@ecs.vuw.ac.nz>
---
 src/ap/ieee802_1x.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Michael Baird Sept. 18, 2017, 8:19 p.m. | #1
Hi,

In relation to this patch (and still occurred at the previous commit 
https://w1.fi/cgit/hostap/commit/?id=3c7863f812d23fefa9987b29308006a29f1d6e9d).

Often the username (dot1XAuthSessionUsername) is <username> plus random 
bytes e.g. 'dot1XAuthSessionUsername=hostuser3ifier 115', where the 
username is 'hostuser3'.


I was thinking that the issue is with not allocating space for a null 
terminator and when used in conjunction with printf-like functions 
running past the end of the char*. As specified in 
https://tools.ietf.org/html/rfc3748#section-5.1 eap identity does not 
send the null terminator,

5.1

"The Identity Response field MUST NOT be null terminated.  In all cases, 
the length of the Type-Data field is derived from the Length field of 
the Request/Response packet."


 From what i can tell when it is used it is expected to be null 
terminated, as a normal char *, or is this wrong? If it is not supposed 
to be null terminated I will amend the above patch to take the 
identity_len field into account when used in the printf function.


Thanks,

Michael
On 14/09/17 16:00, Michael Baird wrote:
> See also http://lists.infradead.org/pipermail/hostap/2017-September/037933.html
>
> Signed-off-by: Michael Baird <Michael.Baird@ecs.vuw.ac.nz>
> ---
>   src/ap/ieee802_1x.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/src/ap/ieee802_1x.c b/src/ap/ieee802_1x.c
> index 6ea1ebe..3517f7d 100644
> --- a/src/ap/ieee802_1x.c
> +++ b/src/ap/ieee802_1x.c
> @@ -17,6 +17,7 @@
>   #include "radius/radius.h"
>   #include "radius/radius_client.h"
>   #include "eap_server/eap.h"
> +#include "eap_server/eap_i.h"
>   #include "eap_common/eap_wsc_common.h"
>   #include "eapol_auth/eapol_auth_sm.h"
>   #include "eapol_auth/eapol_auth_sm_i.h"
> @@ -2638,7 +2639,7 @@ int ieee802_1x_get_mib_sta(struct hostapd_data *hapd, struct sta_info *sta,
>   				   wpa_auth_sta_key_mgmt(sta->wpa_sm))) ?
>   			  1 : 2,
>   			  (unsigned int) diff.sec,
> -			  sm->identity);
> +			  sm->eap->identity);
>   	if (os_snprintf_error(buflen - len, ret))
>   		return len;
>   	len += ret;
Michael Baird Sept. 28, 2017, 7:19 p.m. | #2
Below is the patch that takes into account the identity_len field when sprintf the non null terminated identity.




Only prints the length of the identity & doesn't expect a null terminator, when returning via control interface.

See alsohttp://lists.infradead.org/pipermail/hostap/2017-September/037933.html

Signed-off-by: Michael Baird <Michael.Baird at ecs.vuw.ac.nz>
---
  src/ap/ieee802_1x.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/ap/ieee802_1x.c b/src/ap/ieee802_1x.c
index 793d381..d35773d 100644
--- a/src/ap/ieee802_1x.c
+++ b/src/ap/ieee802_1x.c
@@ -17,6 +17,7 @@
  #include "radius/radius.h"
  #include "radius/radius_client.h"
  #include "eap_server/eap.h"
+#include "eap_server/eap_i.h"
  #include "eap_common/eap_wsc_common.h"
  #include "eapol_auth/eapol_auth_sm.h"
  #include "eapol_auth/eapol_auth_sm_i.h"
@@ -2633,13 +2634,13 @@ int ieee802_1x_get_mib_sta(struct hostapd_data *hapd, struct sta_info *sta,
  			  "dot1xAuthSessionAuthenticMethod=%d\n"
  			  "dot1xAuthSessionTime=%u\n"
  			  "dot1xAuthSessionTerminateCause=999\n"
-			  "dot1xAuthSessionUserName=%s\n",
+			  "dot1xAuthSessionUserName=%.*s\n",
  			  (unsigned long long) sta->acct_session_id,
  			  (wpa_key_mgmt_wpa_ieee8021x(
  				   wpa_auth_sta_key_mgmt(sta->wpa_sm))) ?
  			  1 : 2,
  			  (unsigned int) diff.sec,
-			  sm->identity);
+			  (int) sm->eap->identity_len, sm->eap->identity);
  	if (os_snprintf_error(buflen - len, ret))
  		return len;
  	len += ret;
-- 2.7.4


On 19/09/17 08:19, Michael Baird wrote:
> Hi,
>
> In relation to this patch (and still occurred at the previous commit 
> https://w1.fi/cgit/hostap/commit/?id=3c7863f812d23fefa9987b29308006a29f1d6e9d).
>
> Often the username (dot1XAuthSessionUsername) is <username> plus 
> random bytes e.g. 'dot1XAuthSessionUsername=hostuser3ifier 115', where 
> the username is 'hostuser3'.
>
>
> I was thinking that the issue is with not allocating space for a null 
> terminator and when used in conjunction with printf-like functions 
> running past the end of the char*. As specified in 
> https://tools.ietf.org/html/rfc3748#section-5.1 eap identity does not 
> send the null terminator,
>
> 5.1
>
> "The Identity Response field MUST NOT be null terminated.  In all 
> cases, the length of the Type-Data field is derived from the Length 
> field of the Request/Response packet."
>
>
> From what i can tell when it is used it is expected to be null 
> terminated, as a normal char *, or is this wrong? If it is not 
> supposed to be null terminated I will amend the above patch to take 
> the identity_len field into account when used in the printf function.
>
>
> Thanks,
>
> Michael
> On 14/09/17 16:00, Michael Baird wrote:
>> See also 
>> http://lists.infradead.org/pipermail/hostap/2017-September/037933.html
>>
>> Signed-off-by: Michael Baird <Michael.Baird@ecs.vuw.ac.nz>
>> ---
>>   src/ap/ieee802_1x.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/ap/ieee802_1x.c b/src/ap/ieee802_1x.c
>> index 6ea1ebe..3517f7d 100644
>> --- a/src/ap/ieee802_1x.c
>> +++ b/src/ap/ieee802_1x.c
>> @@ -17,6 +17,7 @@
>>   #include "radius/radius.h"
>>   #include "radius/radius_client.h"
>>   #include "eap_server/eap.h"
>> +#include "eap_server/eap_i.h"
>>   #include "eap_common/eap_wsc_common.h"
>>   #include "eapol_auth/eapol_auth_sm.h"
>>   #include "eapol_auth/eapol_auth_sm_i.h"
>> @@ -2638,7 +2639,7 @@ int ieee802_1x_get_mib_sta(struct hostapd_data 
>> *hapd, struct sta_info *sta,
>>                      wpa_auth_sta_key_mgmt(sta->wpa_sm))) ?
>>                 1 : 2,
>>                 (unsigned int) diff.sec,
>> -              sm->identity);
>> +              sm->eap->identity);
>>       if (os_snprintf_error(buflen - len, ret))
>>           return len;
>>       len += ret;
>

Patch

diff --git a/src/ap/ieee802_1x.c b/src/ap/ieee802_1x.c
index 6ea1ebe..3517f7d 100644
--- a/src/ap/ieee802_1x.c
+++ b/src/ap/ieee802_1x.c
@@ -17,6 +17,7 @@ 
 #include "radius/radius.h"
 #include "radius/radius_client.h"
 #include "eap_server/eap.h"
+#include "eap_server/eap_i.h"
 #include "eap_common/eap_wsc_common.h"
 #include "eapol_auth/eapol_auth_sm.h"
 #include "eapol_auth/eapol_auth_sm_i.h"
@@ -2638,7 +2639,7 @@  int ieee802_1x_get_mib_sta(struct hostapd_data *hapd, struct sta_info *sta,
 				   wpa_auth_sta_key_mgmt(sta->wpa_sm))) ?
 			  1 : 2,
 			  (unsigned int) diff.sec,
-			  sm->identity);
+			  sm->eap->identity);
 	if (os_snprintf_error(buflen - len, ret))
 		return len;
 	len += ret;