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.
/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