Opened 5 years ago

Closed 5 years ago

#14742 closed enhancement (fixed)

Document or remove the --digests option to tor

Reported by: nickm Owned by:
Priority: Low Milestone: Tor: 0.2.7.x-final
Component: Core Tor/Tor Version:
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

If nobody's using it, we should throw it out. If it's useful, we should document it.

Child Tickets

Change History (18)

comment:1 Changed 5 years ago by atagar

Little more context (hate it when issues lack much description), 'tor --digests' provides the sha1 of the source files...

% tor --digests
Tor version 0.2.6.2-alpha-dev (git-518b0b3c5fefba24).
388282ad2a9be7209d64f18d05b1ba65fd254dc8  src/common/aes.c
14f1090665483d0cab4c0ba789d574e71e58d0e2  src/common/crypto.c
0b74196174dd659e06d2acc2a9eeb3894d199b37  src/common/crypto_pwbox.c
2c2f6be4d102cfa5809e0feccc965de328455e5f  src/common/crypto_s2k.c

This presently isn't mentioned in our manual so there's no reason anyone would even know it exists. I only spotted it because we have a cli test for it...

https://gitweb.torproject.org/tor.git/tree/src/test/test_cmdline_args.py#n151

comment:2 Changed 5 years ago by cypherpunks

It's useful information; I say keep it.

comment:3 Changed 5 years ago by nickm

Useful for what? What's the use-case exactly?

comment:4 in reply to:  3 Changed 5 years ago by cypherpunks

Replying to nickm:

Useful for what? What's the use-case exactly?

Knowing* what I compiled.

comment:5 Changed 5 years ago by atagar

Mentioned it on irc but might as well note it here so it's not forgotten: the 'tor --dump-config' command is also undocumented.

comment:6 Changed 5 years ago by atagar

Some other things I'm coming across while testing this...

  • 'tor --dump-config' should provide usage information when no arguments are provided. Presently I guess it's just a no-op which is confusing.
atagar@odin:~/Desktop/stem$ tor --dump-config
atagar@odin:~/Desktop/stem$ tor --dump-config short
DataDirectory /home/atagar/.tor
  • 'tor --dump-config [invalid_option]' gives usage information, but doesn't end with a newline.
atagar@odin:~/Desktop/stem$ tor --dump-config foobar
foobar is not a recognized argument to --dump-config. Please select 'short', 'non-builtin', or 'full'atagar@odin:~/Desktop/stem$ 
  • Output of 'tor --hash-password' is unnecessarily noisy. Trivial improvement would be for '--hash-password' to implicitly include '--hush'.
atagar@odin:~/Desktop/stem$ tor --hash-password foo
Feb 06 09:35:46.451 [notice] Tor v0.2.6.2-alpha-dev (git-518b0b3c5fefba24) running on Linux with Libevent 2.0.16-stable, OpenSSL 1.0.1 and Zlib 1.2.3.4.
Feb 06 09:35:46.451 [notice] Tor can't help you if you use it wrong! Learn how to be safe at https://www.torproject.org/download/download#warning
Feb 06 09:35:46.451 [notice] This version is not a stable Tor release. Expect more bugs than usual.
16:59BB327C197C7A4B60C2DABC2ED2A9BACA04E4C48D138EF942D7A1E75C

atagar@odin:~/Desktop/stem$ tor --hash-password foo --hush
16:C51947FB29F63FAB606357CFE214C4EC3927CE7086FC5E9C65A212D657

comment:7 in reply to:  6 Changed 5 years ago by nickm

Replying to atagar:

Some other things I'm coming across while testing this...

  • 'tor --dump-config' should provide usage information when no arguments are provided. Presently I guess it's just a no-op which is confusing.
atagar@odin:~/Desktop/stem$ tor --dump-config
atagar@odin:~/Desktop/stem$ tor --dump-config short
DataDirectory /home/atagar/.tor
  • 'tor --dump-config [invalid_option]' gives usage information, but doesn't end with a newline.

ack and ack. These should both get fixed. (Probably in a separate ticket? This one is about --digests)

  • Output of 'tor --hash-password' is unnecessarily noisy. Trivial improvement would be for '--hash-password' to implicitly include '--hush'.

agreed; that's a good idea.

comment:8 Changed 5 years ago by atagar

Certainly, as mentioned on irc feel free to split this if you'd like. Just about done porting the tests so should be done soon. Another thing is that we have special behaviour when a config option when a config option is passed on the commandline with a '+' or '/'...

https://gitweb.torproject.org/tor.git/tree/src/test/test_cmdline_args.py#n258

This looks to presently be undocumented. Obvious guess is 'adds/removed from existing config rather than replacing' but I'm getting some unexpected behavior when I pass '/PublishServerDescriptor'. Probably only works properly for LineList arguments.

comment:9 Changed 5 years ago by atagar

There's a formatting issue with ExtORPort. Search for that on...

https://www.torproject.org/docs/tor-manual.html.en

... and you'll find bold text rather than a header like the other options.

comment:10 Changed 5 years ago by nickm

So, here's why I think that the --digests option might not be so useful after all:

  • It isn't a security tool. If somebody is surreptitiously modifying your tor source before you compile, they can surreptitiously modify the build system so that the digests seem like whatever they want them to. So let's not consider security use-cses.
  • It's mostly redundant with the version number Tor displays when it starts. If you are building from a tarball, you get the exact version from the tarball. If you are building from git, you get the current git commit (eg Tor v0.2.7.0-alpha-dev (git-f70f1d283e4fb468)).

comment:11 Changed 5 years ago by atagar

Sounds good to me. :)

comment:12 Changed 5 years ago by nickm

Status: newassigned

comment:13 Changed 5 years ago by nickm

Status: assignedneeds_review

Branch "remove_digests" removes --digests. Needs review

comment:14 Changed 5 years ago by atagar

Neat!

As requested filed tickets for the other issues mentioned above: #15541, #15542, #15543.

comment:15 Changed 5 years ago by yawning

Status: needs_reviewneeds_revision

Remove this from configure.ac? As far as I can tell the only reason we needed those was for generating the files.

AC_PATH_PROG([SHA1SUM], [sha1sum], none)
AC_PATH_PROG([OPENSSL], [openssl], none)

comment:16 Changed 5 years ago by nickm

Status: needs_revisionneeds_review

Updated; better now?

comment:17 Changed 5 years ago by yawning

lgtm

comment:18 Changed 5 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merged!

Note: See TracTickets for help on using tickets.