Opened 12 years ago

Closed 12 years ago

Last modified 11 years ago

#1784 closed Enhancement (wontfix)

[PATCH] Ask for password if not supplied

Reported by: turbo Owned by: charles
Priority: Normal Milestone:
Component: Daemon Version: 1.42
Severity: Normal Keywords: request password patch
Cc:

Description

When running 'ps', the username:password pair is viable in the output.

Fix this with the included patch in two ways:

  1. Change the proctitle using 'setproctitle' (availible for linux at the URLhttp://llg.cubic.org/tools/setproctitle-0.3.2.tar.gz, included by default (?) in *BSD)
  2. Allow to not give username and password on the commandline. Instead being asked for credentials if first connection failed, then send request again.

Patch include check for availibility for libsetproctitle - any *BSD needs to add checking in m4/check-setproctitle.m4...

Attachments (4)

transmission-setproctitle.diff (8.8 KB) - added by turbo 12 years ago.
Change proctitle and/or ask for credentials if auth required.
transmission-setproctitle.2.diff (8.9 KB) - added by turbo 12 years ago.
Don't fail if libsetproctitle don't exists, missed a 'ifdef HAVE_SETPROCTITLE'…
transmission-setproctitle.3.diff (8.9 KB) - added by turbo 12 years ago.
Output messages on stderr instead of stdout (so that they're visable even if sending stdout through a pipe - grep'ing for example)
transmission-request_pw.diff (10.7 KB) - added by turbo 12 years ago.
Ask for username/password if not specified. Solved by getpass() and/or readpassphrase() (if not avail in system, use readpassphrase.[ch] stolen from OpenSSH :).

Download all attachments as: .zip

Change History (22)

Changed 12 years ago by turbo

Change proctitle and/or ask for credentials if auth required.

Changed 12 years ago by turbo

Don't fail if libsetproctitle don't exists, missed a 'ifdef HAVE_SETPROCTITLE'...

comment:1 Changed 12 years ago by mezz

The setproctitle() has been available in FreeBSD since 2.2 version. But it is in libc, so that -lsetproctitle and $(SETPROCTITLE_*) are not need for FreeBSD. Should be same for other BSD* but better to check in case if I am wrong. It looks like it needs to add a check on if it is Linux then use -lsetproctitle and $(SETPROCTITLE_*) if the setproctitle is available in Linux?

http://www.freebsd.org/cgi/man.cgi?query=setproctitle

comment:2 Changed 12 years ago by turbo

Sounds kinda over-complicated configure check - 'if not Linux, then just do it, if linux then check'...

It's just simpler to just check if/where it is (and if setproctitle() is availible in said library). But the SETPROCTITLE_MINIMUM could be removed completley I guess. It doesn't do much good in Linux either really...

Changed 12 years ago by turbo

Output messages on stderr instead of stdout (so that they're visable even if sending stdout through a pipe - grep'ing for example)

comment:3 Changed 12 years ago by charles

  • Milestone changed from None Set to 1.50
  • Status changed from new to assigned

I'm not impressed with setproctitle()'s lack of portability, but I agree there needs to be a mechanism to provide authentication without leaking it in process list. Since libcurl supports netrc(5), let's add an option for that in transmission-remote.

comment:4 Changed 12 years ago by charles

  • Resolution set to fixed
  • Status changed from assigned to closed

added to 1.5x branch in r7864

added to trunk in r7865

comment:5 Changed 12 years ago by turbo

  • Keywords request added; visable removed
  • Milestone 1.50 deleted
  • Priority changed from High to Normal
  • Resolution fixed deleted
  • Severity changed from Major to Normal
  • Status changed from closed to reopened
  • Type changed from Bug to Enhancement

The part that asks for a password on the command line is missing in the SVN commits...

comment:6 Changed 12 years ago by turbo

  • Summary changed from [PATCH] Credential information visable in process list (ps) to [PATCH] Ask for password if not supplied

comment:7 Changed 12 years ago by charles

  • Milestone set to 1.50
  • Resolution set to fixed
  • Status changed from reopened to closed

a transmission-remote's session only lasts a second and is often called again and again. It doesn't make sense to keep prompting the user for a password; it's better to store it in a private file.

comment:8 Changed 12 years ago by turbo

  • Resolution fixed deleted
  • Status changed from closed to reopened

It doesn't 'keep prompting'. It asks once. And you have a number of seconds (>10) to enter username and password...

comment:9 Changed 12 years ago by livings124

  • Resolution set to fixed
  • Status changed from reopened to closed

Asks once, per session.

comment:10 Changed 12 years ago by turbo

  • Resolution fixed deleted
  • Status changed from closed to reopened

Don't know what you wanted to say by that, but no matter how many commands I stack on one command line, it only asks for PW _once_!

[celia.pts/3]$ transmission-remote -l -t100 --verify > /dev/null Unauthorized. Please authorize: Username: admin Password: [celia.pts/3]$

ONCE!

comment:11 Changed 12 years ago by turbo

[celia.pts/3]$ transmission-remote -l -t100 --verify > /dev/null
Unauthorized. Please authorize:
Username: admin
Password: 
[celia.pts/3]$ 

comment:12 Changed 12 years ago by turbo

Don't know how many more options to stack, but still only ONCE!

[celia.pts/3]$ /usr/local/bin/transmission-remote localhost:9091 -l --downlimit 80 -t100 --verify -i > /tmp/x
Unauthorized. Please authorize:
Username: admin
Password: 

comment:13 Changed 12 years ago by charles

  1. It prompts the user every time transmission-remote is invoked, which is very tedious. I still think the .netrc route is the best solution, since it don't require re-prompting and it doesn't expose the authentication string in the process lists. Maybe if we were to prompt once ever and save the authentication to a .netrc file, that would be better.
  1. The man page for getpass() says: "This function is obsolete. Do not use it."
  1. Better to call curl_easy_getinfo() with CURLINFO_RESPONSE_CODE and check for 401, than to grep for the "Unauthorized" string.
  1. The patch still uses setproctitle(), which for reasons outlined above I'd rather not use.

comment:14 Changed 12 years ago by turbo

  • Being asked for a password for a network service shouldn't come as a big surprise to anyone/most...
  • Keeping username:password in a file is so ... 70's! It was insecure then, it's even worse now (and no, protecting it and it's leading directories is NOT security)! You didn't want to add the --quit (#1721) because of security concerns, but having a username/password combo in the filesystem!?
  • Having all three possibilities (file, prompt AND commandline) gives the choice for those that don't like one or the other.

Point 2 - Yeah, I was thinking about implementing my own variant, but I found getpass() and it worked so niceley so... It shouldn't be to difficult to do roll your own.

Point 3 - fair enough. Can easily be fixed.

Point 4 - you don't need to apply the WHOLE patch, just the prompt part! Besides, adding/using setproctitle() for those (quite a lot of systems!) that DO have it, should be 'the correct way' - it's not the first time in history a functionality is only available under special circumstances. Besides, it IS nice to have '-n' (I have an alias which uses it while I'm debugging - there will be _NO_ file in my system which have username/password in it!!).

comment:15 Changed 12 years ago by charles

Actually it looks like there's not a good replacement for getpass(), and in the gnu documentation it's *not* deprecated...

comment:16 Changed 12 years ago by charles

  • Milestone 1.50 deleted

Changed 12 years ago by turbo

Ask for username/password if not specified. Solved by getpass() and/or readpassphrase() (if not avail in system, use readpassphrase.[ch] stolen from OpenSSH :).

comment:17 Changed 12 years ago by charles

  • Resolution set to wontfix
  • Status changed from reopened to closed

I appreciate the patch, but this whole topic feels like overkill to me. I'm hesitant to add this to the ever-growing codebase without more users asking for something like this.

I've already fixed the original complaint of this ticket, but it doesn't seem right to mark the ticket as "fixed" since the request was then revised. So for now I'm going to close this as "wontfix" until there's more interest in this feature.

comment:18 Changed 11 years ago by sim

(deleted spam)

Last edited 11 years ago by charles (previous) (diff)
Note: See TracTickets for help on using tickets.