Error in synchronizing time

angeloc commented 2 years ago

# date
Thu Jan  1 00:00:11 UTC 1970
# htpdate -t -s -d https://google.com
google.com                443, 16 Jan 2022 18:04:01 GMT (89 ms) => 0
google.com                443, 16 Jan 2022 18:04:01 GMT (101 ms) => 0
google.com                443, 16 Jan 2022 18:04:01 GMT (67 ms) => 0
google.com                443, 16 Jan 2022 18:04:02 GMT (82 ms) => -1
when: 812500000, nap: -62500000
offset: 0.187500
Setting 0.188 seconds
Set time: Sat Jan  1 18:04:02 2022
# date
Sat Jan  1 18:04:17 UTC 2022

As you can see, the datetime is 15 days less than the real time.

Any idea?

twekkel commented 2 years ago

gmtmktime() was too much code and crappy code too, so I removed it. timegm() does exactly what I wanted, but is not defined by any standard... advised not to be used... that is why I wrote the "custom" epoch().

mktime() is not reliable as it assumes an accurate TZ database... which is not always the case. htpdate is and should always be timezone agnostic.

The epoch calculation is correct and should be identical to what timegm() does... https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html section 4.16

However I believe the initialization is incorrect, ... sometimes I can reproduce, sometimes not... tm_yday is sometimes set to current day, sometimes to 0 (when on 1 Jan 1970).

Please try out this branch: https://github.com/twekkel/htpdate/tree/Epoch As an alternative you could also time timegm() in that same branch... is in comment.

Let me know if it works for you. This definitely needs to be fixed!

angeloc commented 2 years ago

I tested https://github.com/twekkel/htpdate/tree/Epoch

# htpdate -s -t -dd http://www.google.com
bisect: 0, when: 000000000
www.google.com            80, 17 Jan 2022 21:31:44 GMT (76 ms) => -1641072616
bisect: 1, when: 500000000
www.google.com            80, 17 Jan 2022 21:31:44 GMT (65 ms) => -1641072616
bisect: 2, when: 750000000
www.google.com            80, 17 Jan 2022 21:31:44 GMT (96 ms) => -1641072616
bisect: 3, when: 875000000
www.google.com            80, 17 Jan 2022 21:31:44 GMT (79 ms) => -1641072616
when: 937500000, nap: 62500000
offset: 0.000000
#: 1, mean: 0.000, average: 0.000
No time correction needed
# date
Thu Jan  1 00:01:34 UTC 1970

I don't think this works!

twekkel commented 2 years ago

updated the branch, as you hit another (unrelated) bug

works for me now

date -s "@50000"
do  1 jan 1970 14:53:20 CET
# ./htpdate  -dd -t -s -p1 google.com
bisect: 0, when: 000000000
google.com                80, 17 Jan 2022 21:55:15 GMT (14 ms) => -1642406510
when: 500000000, nap: 500000000
offset: 1642406511.000000
#: 1, mean: 1642406511.000, average: 1642406511.000
Setting 1642406511.000 seconds
Set time: Mon Jan 17 22:55:16 2022

please try again

angeloc commented 2 years ago

Commit dc814d3

# htpdate -s -t -dd google.com
bisect: 0, when: 000000000
google.com                80, 17 Jan 2022 22:01:08 GMT (85 ms) => -1641074398
bisect: 1, when: 500000000
google.com                80, 17 Jan 2022 22:01:09 GMT (104 ms) => -1641074399
bisect: 2, when: 250000000
google.com                80, 17 Jan 2022 22:01:09 GMT (116 ms) => -1641074398
bisect: 3, when: 375000000
google.com                80, 17 Jan 2022 22:01:09 GMT (77 ms) => -1641074398
when: 437500000, nap: 62500000
offset: 1641074398.625000
#: 1, mean: 1641074398.625, average: 1641074398.625
Setting 1641074398.625 seconds
Set time: Sat Jan  1 22:01:10 2022
# date
Sat Jan  1 22:02:05 UTC 2022

still not working

twekkel commented 2 years ago

updated branch again, now with timegm()

if this doesn't work, I'm lost...

twekkel commented 2 years ago

sure you are using the newly build htpdate... not the one in PATH?

angeloc commented 2 years ago

On commit 3f61740

# htpdate -s -t -dd google.com
random: crng init done
bisect: 0, when: 000000000
google.com                80, 17 Jan 2022 22:09:14 GMT (85 ms) => -1641074913
bisect: 1, when: 500000000
google.com                80, 17 Jan 2022 22:09:15 GMT (139 ms) => -1641074914
bisect: 2, when: 250000000
google.com                80, 17 Jan 2022 22:09:15 GMT (69 ms) => -1641074913
bisect: 3, when: 375000000
google.com                80, 17 Jan 2022 22:09:16 GMT (72 ms) => -1641074914
when: 312500000, nap: -62500000
offset: 1641074913.625000
#: 1, mean: 1641074913.625, average: 1641074913.625
Setting 1641074913.625 seconds
Set time: Sat Jan  1 22:09:16 2022
# date
Sat Jan  1 22:09:33 UTC 2022

Still not working. I' on a 32bit arm running on qemu. The image is built again each time I pull from git.

twekkel commented 2 years ago

it has to do with 32 vs 64 bit and/or uclibc vs glibc ... I'm running trying with a 32bit alpine docker on an amd64

main issue has to do with yday.... can you please run this snippet?

#define _GNU_SOURCE
#include <stdio.h>
#include <time.h>
#include <string.h>

static long long epoch(struct tm *tm) {
    return(tm->tm_sec + tm->tm_min*60 + tm->tm_hour*3600 + tm->tm_yday*86400 +
        (tm->tm_year-70)*31536000 + ((tm->tm_year-69)/4)*86400 -
        ((tm->tm_year-1)/100)*86400 + ((tm->tm_year+299)/400)*86400);
}

int main()
{
    struct tm       tm;
    char remote_time[25] = "1 Jan 1970 00:00:11 GMT";

    memset(&tm, 0, sizeof(struct tm));  
    strptime(remote_time, "%d %b %Y %T", &tm);
    printf("sec    : %i\n", tm.tm_sec);
    printf("min    : %i\n", tm.tm_min);
    printf("hour   : %i\n", tm.tm_hour);
    printf("month  : %i\n", tm.tm_mon);
    printf("yday   : %i\n", tm.tm_yday);
    printf("year   : %i\n", tm.tm_year);
    printf("epoch  : %lli\n", epoch(&tm));
}

angeloc commented 2 years ago

# test-time 
sec    : 11
min    : 0
hour   : 0
month  : 0
yday   : 0
year   : 70
epoch  : 11

twekkel commented 2 years ago

Initializing works... no idea what is going on here... tomorrow another day

On Jan 17, 2022, 23:29, at 23:29, Angelo Compagnucci @.***> wrote:

# test-time 
sec    : 11
min    : 0
hour   : 0
month  : 0
yday   : 0
year   : 70
epoch  : 11
-- Reply to this email directly or view it on GitHub: https://github.com/twekkel/htpdate/issues/13#issuecomment-1014926994 You are receiving this because you commented.

Message ID: @.***>

angeloc commented 2 years ago

# test-time 
date   : 17 Jan 2022 23:38:11 GMT
sec    : 11
min    : 38
hour   : 23
month  : 0
yday   : 0
year   : 122
epoch  : 1641080291
# date @1641080291
Sat Jan  1 23:38:11 UTC 2022

the yday is definitely wrong

angeloc commented 2 years ago

time.c in uclibc source says:

 * strptime implements the glibc extension specifiers.  However, it follows
 *     SUSv3 in requiring at least one non-alphanumeric char between
 *     conversion specifiers.  Also, strptime only sets struct tm fields
 *     for which format specifiers appear and does not try to infer other
 *     fields (such as wday) as glibc's version does.

so tm.tm_yday will always be 0.

twekkel commented 2 years ago

tm.tm_yday will not be touched by uclibc, so it can have any value (as I have seen while debugging this and confused me)... depends on the initialized value

How to proceed? 1) drop uclibc support 2) use mktime()... has a dependency on local TZ database to be uptodate... countries tend to change daylight savings dates (or abolish it) "frequently" 3) use the previous gmtmktime() as used in htpdate in the past 4) write custom strptime... already done using sscanf in https://github.com/twekkel/httpdate/blob/main/httpdate.c and https://github.com/twekkel/httpdate/blob/main/utils.c 5) ?

I don't like options 1 trough 4, so I hope you have a better/simpler/elegant solution. (If not, I probably go for option 4)

On 2022-01-18 00:02, Angelo Compagnucci wrote:

time.c in uclibc source says:

strptime implements the glibc extension specifiers. However, it follows

SUSv3 in requiring at least one non-alphanumeric char between

conversion specifiers. Also, strptime only sets struct tm fields

for which format specifiers appear and does not try to infer other

fields (such as wday) as glibc's version does.

so tm.tm_yday will always be 0.

-- Reply to this email directly, view it on GitHub [1], or unsubscribe [2]. Triage notifications on the go with GitHub Mobile for iOS [3] or Android [4]. You are receiving this because you commented.Message ID: @.***>

Links:

[1] https://github.com/twekkel/htpdate/issues/13#issuecomment-1014940080 [2] https://github.com/notifications/unsubscribe-auth/ANVLPJWQKSO4QCOIQAKNZRLUWSNYFANCNFSM5MDBCNBQ [3] https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 [4] https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub

angeloc commented 2 years ago

this https://stackoverflow.com/a/11324281?

twekkel commented 2 years ago

that is more or less what I did in the past with gmtmktime() and don't like so much https://github.com/twekkel/htpdate/blob/143364b71549f4b4c4af87795aa97af4174c5730/htpdate.c#L79

just weird that it needs to be done like this, IMHO

On 2022-01-18 09:29, Angelo Compagnucci wrote:

this https://stackoverflow.com/a/11324281?

-- Reply to this email directly, view it on GitHub [1], or unsubscribe [2]. Triage notifications on the go with GitHub Mobile for iOS [3] or Android [4]. You are receiving this because you commented.Message ID: @.***>

Links:

[1] https://github.com/twekkel/htpdate/issues/13#issuecomment-1015177059 [2] https://github.com/notifications/unsubscribe-auth/ANVLPJQMFZ5UXUIAGNNR2WLUWUQHNANCNFSM5MDBCNBQ [3] https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 [4] https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub

angeloc commented 2 years ago

I was referring to the implementation of my_timegm, it doesn't use the yday from the tm struct.

twekkel commented 2 years ago

the my_timegm() is more or less the same as gmtmktime() what was in htpdate in the past (and also in your repo), but the snippet you provided it cleaner. Saving and later restoring TZ seems so inefficient, but will do for now.

Please give branch https://github.com/twekkel/htpdate/tree/strptime_epoch a test and I will merge it.

angeloc commented 2 years ago

I cleaned the environment and rebuilt from scratch, this is the minimum set of chages required to make the date sync correctly on uclibc

diff --git a/htpdate.c b/htpdate.c
index f87a8b0..a3476a4 100644
--- a/htpdate.c
+++ b/htpdate.c
@@ -87,14 +87,6 @@ static int debug   = 0;
 static int logmode = 0;
 static int verifycert = 0;

-/* timegm() replacement */
-static long epoch(struct tm tm) {
-    return(tm.tm_sec + tm.tm_min*60 + tm.tm_hour*3600 + tm.tm_yday*86400 +
-        (tm.tm_year-70)*31536000 + ((tm.tm_year-69)/4)*86400 -
-        ((tm.tm_year-1)/100)*86400 + ((tm.tm_year+299)/400)*86400);
-}
-
-
 /* Insertion sort is more efficient (and smaller) than qsort for small lists */
 static void insertsort(double a[], long length) {
     long i, j;
@@ -208,7 +200,7 @@ static long getoffset(char remote_time[25]) {
     clock_gettime(CLOCK_REALTIME, &now);
     memset(&tm, 0, sizeof(struct tm));
     if (strptime(remote_time, "%d %b %Y %T", &tm) != NULL) {
-        timevalue.tv_sec = epoch(tm);
+        timevalue.tv_sec = timegm(&tm);
     } else {
         printlog(1, "unknown time format");
     }

The "my_timegm" works too.

twekkel commented 2 years ago

That means reverting https://github.com/twekkel/htpdate/pull/1

Since OpenBSD is not going to work for many other reasons, it makes sense to reintroduce timegm()

twekkel commented 2 years ago

done reverting... timegm() is in again

angeloc commented 2 years ago

Perfect! it works wonderfully!

Could you tag another release?

Thanks!

twekkel commented 2 years ago

dump new version done, https://github.com/twekkel/htpdate/releases/tag/v1.3.3

twekkel / htpdate

Error in synchronizing time #13

Links:

Links: