Opened 3 months ago

Closed 7 weeks ago

#31012 closed defect (fixed)

tor-print-ed-signing-cert shows local time, without a timezone

Reported by: teor Owned by: rl1987
Priority: Medium Milestone: Tor: 0.4.2.x-final
Component: Core Tor/Tor Version: Tor: 0.3.5.1-alpha
Severity: Normal Keywords: dgoulet-merge
Cc: rl1987 Actual Points: 0.1
Parent ID: Points: 0.1
Reviewer: nickm Sponsor:

Description

tor-print-ed-signing-cert prints the local time.
But that isn't obvious from the output, and it isn't documented in the man page,

Instead, we should print local time + offset, and document that in the man page.

Child Tickets

Change History (13)

comment:1 Changed 3 months ago by rl1987

Owner: set to rl1987
Status: newaccepted

comment:2 Changed 3 months ago by teor

weasel also asked for rfc 822, but do whatever is easiest and most convenient in C.

comment:3 Changed 3 months ago by rl1987

Status: acceptedneeds_review

comment:4 Changed 3 months ago by teor

Actual Points: 0.1
Milestone: Tor: unspecifiedTor: 0.4.2.x-final
Status: needs_reviewneeds_revision
Version: Tor: 0.3.5.1-alpha

I left some comments on the PR.

Please fix this crash:

$  src/tools/tor-print-ed-signing-cert ~/.tor/keys/ed25519_signing_cert 
Expires at: Tue Jul  2 17:00:00 2019

=================================================================
==11166==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffee8b04630 at pc 0x0001074297de bp 0x7ffee8b04350 sp 0x7ffee8b03ac8
READ of size 23 at 0x7ffee8b04630 thread T0
    #0 0x1074297dd in printf_common(void*, char const*, __va_list_tag*) (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x267dd)
    #1 0x10742a6bc in wrap_printf (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x276bc)
    #2 0x1070fc689 in main tor-print-ed-signing-cert.c:77
    #3 0x7fff586df014 in start (libdyld.dylib:x86_64+0x1014)

Address 0x7ffee8b04630 is located in stack of thread T0 at offset 464 in frame
    #0 0x1070fc21f in main tor-print-ed-signing-cert.c:17

  This frame has 5 object(s):
    [32, 40) 'cert' (line 18)
    [64, 72) 'got_tag' (line 27)
    [96, 352) 'certbuf' (line 29)
    [416, 424) 'expiration' (line 59)
    [448, 464) 'rfc822_str' (line 63) <== Memory access at offset 464 overflows this variable
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x267dd) in printf_common(void*, char const*, __va_list_tag*)
Shadow bytes around the buggy address:
  0x1fffdd160870: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1fffdd160880: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1
  0x1fffdd160890: 00 f2 f2 f2 00 f2 f2 f2 00 00 00 00 00 00 00 00
  0x1fffdd1608a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1fffdd1608b0: 00 00 00 00 00 00 00 00 f2 f2 f2 f2 f2 f2 f2 f2
=>0x1fffdd1608c0: 00 f2 f2 f2 00 00[f3]f3 00 00 00 00 00 00 00 00
  0x1fffdd1608d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1fffdd1608e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1fffdd1608f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1fffdd160900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x1fffdd160910: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==11166==ABORTING
Abort trap: 6
Exit 134

comment:5 Changed 3 months ago by teor

Reviewer: teor
Status: needs_revisionmerge_ready

Looks better now, let's get the merger to run it, and then we're done.

comment:6 Changed 3 months ago by teor

I opened #31023 to write tests that run these tools.

comment:7 Changed 3 months ago by nickm

Is there anywhere else that we use these formats for user-visible data? We already have format_rfc1123_time and format_iso_time; why do we need new formats here?

comment:8 Changed 3 months ago by nickm

Status: merge_readyneeds_revision

comment:9 Changed 3 months ago by rl1987

Status: needs_revisionneeds_review

comment:10 Changed 7 weeks ago by teor

Reviewer: teornickm

comment:11 Changed 7 weeks ago by nickm

Status: needs_reviewmerge_ready

This seems okay to me. My only worry would be that someone is depending on the existing format, but I guess we can stick this behind an option if it causes trouble.

comment:12 Changed 7 weeks ago by dgoulet

Keywords: dgoulet-merge added

comment:13 Changed 7 weeks ago by dgoulet

Resolution: fixed
Status: merge_readyclosed

Merged!

Note: See TracTickets for help on using tickets.