[RFC/RFT] use monotonic clock for relative time where available

Johannes Berg johannes
Wed Nov 13 11:51:28 PST 2013


On Wed, 2013-11-13 at 17:50 +0100, Johannes Berg wrote:

> TODO: carefully check all replacements to see if any should
>       actually be using os_get_time() instead!

> @@ -448,11 +448,11 @@ static void accounting_report_state(struct hostapd_data *hapd, int on)
>   */
>  int accounting_init(struct hostapd_data *hapd)
>  {
> -	struct os_time now;
> +	struct os_reltime now;
>  
>  	/* Acct-Session-Id should be unique over reboots. If reliable clock is
>  	 * not available, this could be replaced with reboot counter, etc. */
> -	os_get_time(&now);
> +	os_get_reltime(&now);

That should stay os_get_time().


> @@ -207,13 +207,13 @@ static u16 auth_shared_key(struct hostapd_data *hapd, struct sta_info *sta,
>  		if (!sta->challenge) {
>  			/* Generate a pseudo-random challenge */
>  			u8 key[8];
> -			struct os_time now;
> +			struct os_reltime now;
>  			int r;
>  			sta->challenge = os_zalloc(WLAN_AUTH_CHALLENGE_LEN);
>  			if (sta->challenge == NULL)
>  				return WLAN_STATUS_UNSPECIFIED_FAILURE;
>  
> -			os_get_time(&now);
> +			os_get_reltime(&now);
>  			r = os_random();
>  			os_memcpy(key, &now.sec, 4);
>  			os_memcpy(key + 4, &r, 4);

This doesn't really matter much, but might as well stay.


> +++ b/src/ap/wpa_auth_ft.c
> @@ -1406,7 +1406,7 @@ static int wpa_ft_rrb_rx_push(struct wpa_authenticator *wpa_auth,
>  {
>  	struct ft_r0kh_r1kh_push_frame *frame, f;
>  	struct ft_remote_r0kh *r0kh;
> -	struct os_time now;
> +	struct os_reltime now;
>  	os_time_t tsend;
>  	int pairwise;
>  
> @@ -1438,7 +1438,7 @@ static int wpa_ft_rrb_rx_push(struct wpa_authenticator *wpa_auth,
>  		return -1;
>  	}
>  
> -	os_get_time(&now);
> +	os_get_reltime(&now);
>  	tsend = WPA_GET_LE32(f.timestamp);
>  	if ((now.sec > tsend && now.sec - tsend > 60) ||
>  	    (now.sec < tsend && tsend - now.sec > 60)) {

I'm guessing this must stay.


> @@ -1600,7 +1600,7 @@ static void wpa_ft_generate_pmk_r1(struct wpa_authenticator *wpa_auth,
>  				   const u8 *s1kh_id, int pairwise)
>  {
>  	struct ft_r0kh_r1kh_push_frame frame, f;
> -	struct os_time now;
> +	struct os_reltime now;
>  
>  	os_memset(&frame, 0, sizeof(frame));
>  	frame.frame_type = RSN_REMOTE_FRAME_TYPE_FT_RRB;
> @@ -1619,7 +1619,7 @@ static void wpa_ft_generate_pmk_r1(struct wpa_authenticator *wpa_auth,
>  	wpa_hexdump_key(MSG_DEBUG, "FT: PMK-R1", f.pmk_r1, PMK_LEN);
>  	wpa_hexdump(MSG_DEBUG, "FT: PMKR1Name", f.pmk_r1_name,
>  		    WPA_PMK_NAME_LEN);
> -	os_get_time(&now);
> +	os_get_reltime(&now);
>  	WPA_PUT_LE32(f.timestamp, now.sec);
>  	f.pairwise = host_to_le16(pairwise);
>  	os_memset(f.pad, 0, sizeof(f.pad));

ditto.


> --- a/src/ap/wps_hostapd.c
> +++ b/src/ap/wps_hostapd.c
> @@ -189,11 +189,11 @@ static void hostapd_wps_pin_needed_cb(void *ctx, const u8 *uuid_e,
>  
>  	if (hapd->conf->wps_pin_requests) {
>  		FILE *f;
> -		struct os_time t;
> +		struct os_reltime t;
>  		f = fopen(hapd->conf->wps_pin_requests, "a");
>  		if (f == NULL)
>  			return;
> -		os_get_time(&t);
> +		os_get_reltime(&t);

This is some API so should stay.

>  void random_add_randomness(const void *buf, size_t len)
>  {
> -	struct os_time t;
> +	struct os_reltime t;
>  	static unsigned int count = 0;
>  
>  	count++;
> @@ -139,7 +139,7 @@ void random_add_randomness(const void *buf, size_t len)
>  	wpa_printf(MSG_EXCESSIVE, "Add randomness: count=%u entropy=%u",
>  		   count, entropy);
>  
> -	os_get_time(&t);
> +	os_get_reltime(&t);

that's ... hard to say. doing both would be good? wall time is really
predictable, but boot time has a small range usually ... let's leave it
for now.


> +++ b/src/eap_peer/eap_fast.c
> @@ -889,7 +889,7 @@ static int eap_fast_parse_pac_info(struct eap_fast_pac *entry, int type,
>  {
>  	u16 pac_type;
>  	u32 lifetime;
> -	struct os_time now;
> +	struct os_reltime now;
>  
>  	switch (type & 0x7fff) {
>  	case PAC_TYPE_CRED_LIFETIME:
> @@ -907,7 +907,7 @@ static int eap_fast_parse_pac_info(struct eap_fast_pac *entry, int type,
>  		 * dump if it is needed for something in the future.
>  		 */
>  		lifetime = WPA_GET_BE32(pos);
> -		os_get_time(&now);
> +		os_get_reltime(&now);

should stay wall time


> +++ b/src/eap_server/eap_server_fast.c
> @@ -127,7 +127,7 @@ static int eap_fast_session_ticket_cb(void *ctx, const u8 *ticket, size_t len,
>  	size_t pac_opaque_len;
>  	u8 *buf, *pos, *end, *pac_key = NULL;
>  	os_time_t lifetime = 0;
> -	struct os_time now;
> +	struct os_reltime now;
>  	u8 *identity = NULL;
>  	size_t identity_len = 0;
>  
> @@ -237,7 +237,7 @@ static int eap_fast_session_ticket_cb(void *ctx, const u8 *ticket, size_t len,
>  		}
>  	}
>  
> -	if (os_get_time(&now) < 0 || lifetime <= 0 || now.sec > lifetime) {
> +	if (os_get_reltime(&now) < 0 || lifetime <= 0 || now.sec > lifetime) {
>  		wpa_printf(MSG_DEBUG, "EAP-FAST: PAC-Key not valid anymore "
>  			   "(lifetime=%ld now=%ld)", lifetime, now.sec);
>  		data->send_new_pac = 2;
> @@ -685,10 +685,10 @@ static struct wpabuf * eap_fast_build_pac(struct eap_sm *sm,
>  	struct eap_tlv_hdr *pac_tlv;
>  	struct pac_tlv_hdr *pac_info;
>  	struct eap_tlv_result_tlv *result;
> -	struct os_time now;
> +	struct os_reltime now;
>  
>  	if (random_get_bytes(pac_key, EAP_FAST_PAC_KEY_LEN) < 0 ||
> -	    os_get_time(&now) < 0)
> +	    os_get_reltime(&now) < 0)
>  		return NULL;
>  	wpa_hexdump_key(MSG_DEBUG, "EAP-FAST: Generated PAC-Key",
>  			pac_key, EAP_FAST_PAC_KEY_LEN);

I think the same applies here, though maybe the data is just
round-tripped? I'll remove this part for now.


> +++ b/src/eap_server/eap_sim_db.c
> @@ -38,7 +38,7 @@ struct eap_sim_db_pending {
>  	char imsi[20];
>  	enum { PENDING, SUCCESS, FAILURE } state;
>  	void *cb_session_ctx;
> -	struct os_time timestamp;
> +	struct os_reltime timestamp;
>  	int aka;
>  	union {
>  		struct {
> @@ -935,7 +935,7 @@ int eap_sim_db_get_gsm_triplets(struct eap_sim_db_data *data,
>  	if (entry == NULL)
>  		return EAP_SIM_DB_FAILURE;
>  
> -	os_get_time(&entry->timestamp);
> +	os_get_reltime(&entry->timestamp);
>  	os_strlcpy(entry->imsi, imsi, sizeof(entry->imsi));
>  	entry->cb_session_ctx = cb_session_ctx;
>  	entry->state = PENDING;
> @@ -1395,7 +1395,7 @@ int eap_sim_db_get_aka_auth(struct eap_sim_db_data *data, const char *username,
>  	if (entry == NULL)
>  		return EAP_SIM_DB_FAILURE;
>  
> -	os_get_time(&entry->timestamp);
> +	os_get_reltime(&entry->timestamp);
>  	entry->aka = 1;
>  	os_strlcpy(entry->imsi, imsi, sizeof(entry->imsi));
>  	entry->cb_session_ctx = cb_session_ctx;

I'm confused here - the timestamp isn't used anywhere?


>  void radius_msg_make_authenticator(struct radius_msg *msg,
>  				   const u8 *data, size_t len)
>  {
> -	struct os_time tv;
> +	struct os_reltime tv;
>  	long int l;
>  	const u8 *addr[3];
>  	size_t elen[3];
>  
> -	os_get_time(&tv);
> +	os_get_reltime(&tv);
>  	l = os_random();
>  	addr[0] = (u8 *) &tv;
>  	elen[0] = sizeof(tv);

doesn't really matter - let's keep using wall time


> +++ b/src/radius/radius_das.c
> @@ -158,7 +158,7 @@ static void radius_das_receive(int sock, void *eloop_ctx, void *sock_ctx)
>  	struct wpabuf *rbuf;
>  	u32 val;
>  	int res;
> -	struct os_time now;
> +	struct os_reltime now;
>  
>  	fromlen = sizeof(from);
>  	len = recvfrom(sock, buf, sizeof(buf), 0,
> @@ -195,7 +195,7 @@ static void radius_das_receive(int sock, void *eloop_ctx, void *sock_ctx)
>  		goto fail;
>  	}
>  
> -	os_get_time(&now);
> +	os_get_reltime(&now);

must be wall time


> +++ b/src/tls/tlsv1_client_write.c
> @@ -45,13 +45,13 @@ static size_t tls_client_cert_chain_der_len(struct tlsv1_client *conn)
>  u8 * tls_send_client_hello(struct tlsv1_client *conn, size_t *out_len)
>  {
>  	u8 *hello, *end, *pos, *hs_length, *hs_start, *rhdr;
> -	struct os_time now;
> +	struct os_reltime now;
>  	size_t len, i;
>  
>  	wpa_printf(MSG_DEBUG, "TLSv1: Send ClientHello");
>  	*out_len = 0;
>  
> -	os_get_time(&now);
> +	os_get_reltime(&now);
>  	WPA_PUT_BE32(conn->client_random, now.sec);
>  	if (random_get_bytes(conn->client_random + 4, TLS_RANDOM_LEN - 4)) {
>  		wpa_printf(MSG_ERROR, "TLSv1: Could not generate "

used as random value - let's not change


> +++ b/src/tls/tlsv1_server_write.c
> @@ -43,7 +43,7 @@ static int tls_write_server_hello(struct tlsv1_server *conn,
>  				  u8 **msgpos, u8 *end)
>  {
>  	u8 *pos, *rhdr, *hs_start, *hs_length;
> -	struct os_time now;
> +	struct os_reltime now;
>  	size_t rlen;
>  
>  	pos = *msgpos;
> @@ -52,7 +52,7 @@ static int tls_write_server_hello(struct tlsv1_server *conn,
>  	rhdr = pos;
>  	pos += TLS_RECORD_HEADER_LEN;
>  
> -	os_get_time(&now);
> +	os_get_reltime(&now);
>  	WPA_PUT_BE32(conn->server_random, now.sec);

ditto


> +++ b/tests/hwsim/stop.sh
> @@ -68,9 +68,9 @@ for i in /tmp/wpas-wlan0 /tmp/wpas-wlan1 /tmp/wpas-wlan2 /var/run/hostapd-global
>  done
>  
>  if grep -q mac80211_hwsim /proc/modules 2>/dev/null ; then
> -    sudo rmmod mac80211_hwsim
> -    sudo rmmod mac80211
> -    sudo rmmod cfg80211
> +#    sudo rmmod mac80211_hwsim
> +#    sudo rmmod mac80211
> +#    sudo rmmod cfg80211
>      # wait at the end to avoid issues starting something new immediately after
>      # this script returns
>      sleep 1

ouch.


> +++ b/wpa_supplicant/notify.c
> @@ -377,7 +377,7 @@ void wpas_notify_suspend(struct wpa_global *global)
>  {
>  	struct wpa_supplicant *wpa_s;
>  
> -	os_get_time(&global->suspend_time);
> +	os_get_reltime(&global->suspend_time);
>  	wpa_printf(MSG_DEBUG, "System suspend notification");
>  	for (wpa_s = global->ifaces; wpa_s; wpa_s = wpa_s->next)
>  		wpa_drv_suspend(wpa_s);
> @@ -386,14 +386,14 @@ void wpas_notify_suspend(struct wpa_global *global)
>  
>  void wpas_notify_resume(struct wpa_global *global)
>  {
> -	struct os_time now;
> +	struct os_reltime now;
>  	int slept;
>  	struct wpa_supplicant *wpa_s;
>  
>  	if (global->suspend_time.sec == 0)
>  		slept = -1;
>  	else {
> -		os_get_time(&now);
> +		os_get_reltime(&now);
>  		slept = now.sec - global->suspend_time.sec;
>  	}
>  	wpa_printf(MSG_DEBUG, "System resume notification (slept %d seconds)",

This part is tricky. With CLOCK_BOOTTIME, this works, but with
CLOCK_MONOTONIC this will return 0. Nothing but the debug messages cares
though, so let's keep using wall time here even if that might not be
accurate either.


> +++ b/src/utils/wpa_debug.c
> @@ -65,12 +65,12 @@ static FILE *out_file = NULL;
>  void wpa_debug_print_timestamp(void)
>  {
>  #ifndef CONFIG_ANDROID_LOG
> -	struct os_time tv;
> +	struct os_reltime tv;
>  
>  	if (!wpa_debug_timestamp)
>  		return;
>  
> -	os_get_time(&tv);
> +	os_get_reltime(&tv);
>  #ifdef CONFIG_DEBUG_FILE
>  	if (out_file) {
>  		fprintf(out_file, "%ld.%06u: ", (long) tv.sec,

That's probably better left alone (although I personally usually only do
relative time analysis with it)

New version here: http://p.sipsolutions.net/b02ac4b02793dfcf.txt

johannes




More information about the Hostap mailing list