On Sun, Jul 21, 2013 at 09:15:00AM +0200, Alexander Hall wrote: > On 07/21/13 08:11, patrick keshishian wrote: > > Hi, > > > > Couple of comments inline. > > > > On Sun, Jul 21, 2013 at 03:16:28AM +0200, Alexander Hall wrote: > >> This is an attempt to make the ssh-* man pages more exact regarding > >> SSH_ASKPASS, when used for ssh-agent key confirmation. > >> > >> The point I'm making is that the relevant SSH_ASKPASS environment > >> variable is not that of ssh-add(1) (apart from when ssh-add is actually > >> asking for a passphrase). > >> > >> On a sidenote, I think I'd prefer a 'SSH_CONFIRM' variable or somesuch > >> (falling back to SSH_ASKPASS), but maybe we don't want to pollute the > >> environment any further. > >> > >> /Alexander > >> > >> > >> Index: ssh-add.1 > >> =================================================================== > >> RCS file: /cvs/src/usr.bin/ssh/ssh-add.1,v > >> retrieving revision 1.58 > >> diff -u -p -r1.58 ssh-add.1 > >> --- ssh-add.1 3 Dec 2012 08:33:02 -0000 1.58 > >> +++ ssh-add.1 21 Jul 2013 01:09:49 -0000 > >> @@ -84,14 +84,10 @@ to work. > >> The options are as follows: > >> .Bl -tag -width Ds > >> .It Fl c > >> -Indicates that added identities should be subject to confirmation before > >> +Indicates that > >> +.Xr ssh-agent 1 > >> +should ask for confirmation before added identities are > >> being used for authentication. > > ^^^^^ > > Zap "being" from above. > > > > > >> -Confirmation is performed by the > >> -.Ev SSH_ASKPASS > >> -program mentioned below. > >> -Successful confirmation is signaled by a zero exit status from the > >> -.Ev SSH_ASKPASS > >> -program, rather than text entered into the requester. > >> .It Fl D > >> Deletes all identities from the agent. > >> .It Fl d > >> Index: ssh-agent.1 > >> =================================================================== > >> RCS file: /cvs/src/usr.bin/ssh/ssh-agent.1,v > >> retrieving revision 1.53 > >> diff -u -p -r1.53 ssh-agent.1 > >> --- ssh-agent.1 21 Nov 2010 01:01:13 -0000 1.53 > >> +++ ssh-agent.1 21 Jul 2013 01:09:49 -0000 > >> @@ -161,6 +161,18 @@ Later > >> .Xr ssh 1 > >> looks at these variables and uses them to establish a connection to the > >> agent. > >> .Pp > >> +If confirmation before using a key is requested by > >> +.Xr ssh-add 1 , > >> +it is performed by the program specified in the > >> +.Ev SSH_ASKPASS > >> +environment variable, or > >> +.Xr ssh-askpass 1 > >> +if > >> +.Ev SSH_ASKPASS > >> +is not set. > >> +Successful confirmation is signaled by a zero exit status, and that the > > ^^^^ > > Maybe drop the "that" from above. > > > >> +first line of the program's output is empty or the string "yes". > >> +.Pp > > > > However, the sentence still reads awkwardly. Are you trying to > > say the requirement is: > > > > if (an_exit_status == 0 && > > (output_string == "" || output_string == "yes")) > > Yup. > > > > > If so, maybe a better wording would be: > > > > Successful confirmation is signaled by a zero exit status, > > and the first line of the program's output SHOULD be either > > empty or the string "yes." > > > > Suggesting the following: > > Successful confirmation is signaled by a zero exit status, > and the first line of the program's output being either > empty or the string "yes". > > Must really the full stop should be within the quotes, even though it's > not part of the quote? If so, and we may not ignore this for the > percieved accuracy of the man pages, I'd suggest we rephrase it so that > it is not the last word.
I believe so, see: http://owl.english.purdue.edu/owl/resource/577/03/ But, my suggestion mainly was to introduce the word "should" in order to make the statement less passive, when stating what is expected of the program's first output line. --patrick > > /Alexander > > > Index: ssh-add.1 > =================================================================== > RCS file: /cvs/src/usr.bin/ssh/ssh-add.1,v > retrieving revision 1.58 > diff -u -p -r1.58 ssh-add.1 > --- ssh-add.1 3 Dec 2012 08:33:02 -0000 1.58 > +++ ssh-add.1 21 Jul 2013 07:14:13 -0000 > @@ -84,14 +84,10 @@ to work. > The options are as follows: > .Bl -tag -width Ds > .It Fl c > -Indicates that added identities should be subject to confirmation before > -being used for authentication. > -Confirmation is performed by the > -.Ev SSH_ASKPASS > -program mentioned below. > -Successful confirmation is signaled by a zero exit status from the > -.Ev SSH_ASKPASS > -program, rather than text entered into the requester. > +Indicates that > +.Xr ssh-agent 1 > +should ask for confirmation before added identities are used for > +authentication. > .It Fl D > Deletes all identities from the agent. > .It Fl d > Index: ssh-agent.1 > =================================================================== > RCS file: /cvs/src/usr.bin/ssh/ssh-agent.1,v > retrieving revision 1.53 > diff -u -p -r1.53 ssh-agent.1 > --- ssh-agent.1 21 Nov 2010 01:01:13 -0000 1.53 > +++ ssh-agent.1 21 Jul 2013 07:14:13 -0000 > @@ -161,6 +161,18 @@ Later > .Xr ssh 1 > looks at these variables and uses them to establish a connection to the > agent. > .Pp > +If confirmation before using a key is requested by > +.Xr ssh-add 1 , > +it is performed by the program specified in the > +.Ev SSH_ASKPASS > +environment variable, or > +.Xr ssh-askpass 1 > +if > +.Ev SSH_ASKPASS > +is not set. > +Successful confirmation is signaled by a zero exit status, and the first line > +of the program's output being either empty or the string "yes". > +.Pp > The agent will never send a private key over its request channel. > Instead, operations that require a private key will be performed > by the agent, and the result will be returned to the requester. > Index: ssh_config.5 > =================================================================== > RCS file: /cvs/src/usr.bin/ssh/ssh_config.5,v > retrieving revision 1.166 > diff -u -p -r1.166 ssh_config.5 > --- ssh_config.5 27 Jun 2013 14:05:37 -0000 1.166 > +++ ssh_config.5 21 Jul 2013 07:14:13 -0000 > @@ -286,7 +286,7 @@ will cause ssh > to listen for control connections, but require confirmation using the > .Ev SSH_ASKPASS > program before they are accepted (see > -.Xr ssh-add 1 > +.Xr ssh-agent 1 > for details). > If the > .Cm ControlPath >
