openssh bugs - May 2015 - [Bug 2398] New: AuthenticationMethods doesn't have default value (inconsistency) and it accept empty value

https://bugzilla.mindrot.org/show_bug.cgi?id=2398

            Bug ID: 2398
           Summary: AuthenticationMethods doesn't have default value
                    (inconsistency) and it accept empty value
           Product: Portable OpenSSH
           Version: 6.8p1
          Hardware: Other
                OS: Linux
            Status: NEW
          Severity: enhancement
          Priority: P5
         Component: sshd
          Assignee: unassigned-bugs at mindrot.org
          Reporter: jjelen at redhat.com

Created attachment 2620
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2620&action=edit
proposed patch

Based on previous bugzilla, we realized that it would be useful to have
default value for this server option, because of:

 * First of all, it is inconsistent with all other options that are
available in openssh.

 * Another issue is usability. You can't reset this option in match
block to it's default value if other match block or default config
changed this option.

Ex: I want to have all users to authenticate using public key and
password, but I want exception for localhost to use any authentication
method available. It would be nice, if we could do something like this:
>Match Address ::1
>    AuthenticationMethods any
>Match Address *
>    AuthenticationMethods publickey,password
There can be used workaround:
>Match Address !::1
>    AuthenticationMethods publickey,password
but it doesn't work, as stated in bz2397. Also it can get quite messy
if you have more blocks like that.

To have this feature working, we need to choose value for ANY (proposed
"any"), use this value as default (enforced by
fill_default_server_options) and make sure that it is handled
everywhere in the code consistently. There are few design
consideration, before posting a patch:
 * We can't use just num_auth_methods == 0, because this is considered
as not-defined and it can't override previously definde authentication
methods
 * We can use enforce num_auth_methods == 1 && strcmp(auth_methods[0],
"any"), but it is not much elegant from my POV, but best I have got.
 * We can use num_auth_methods == -1, but it would require few changes
in more data types in application (currently defined as u_int, so we
can't store here -1).


Also as I can see, there was not properly propagated change to bz2281
from our bugzilla which covered also empty values of
AuthenticationMethods (also covered in attached patch).

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2398

Stanislav Zidek <szidek at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |szidek at redhat.com

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2398

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2451
                 CC|                            |djm at mindrot.org


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2451
[Bug 2451] Bugs intended to be fixed in 7.2
-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2398

--- Comment #1 from Damien Miller <djm at mindrot.org> ---
Retarget to openssh-7.3

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2398

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |2543


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2543
[Bug 2543] Tracking bug for OpenSSH 7.3 release
-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2398

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|2451                        |

--- Comment #2 from Damien Miller <djm at mindrot.org> ---
Retarget to openssh-7.3


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=2451
[Bug 2451] Bugs intended to be fixed in 7.2
-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2398

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2620|0                           |1
        is obsolete|                            |
           Assignee|unassigned-bugs at mindrot.org |djm at mindrot.org
             Status|NEW                         |ASSIGNED

--- Comment #3 from Damien Miller <djm at mindrot.org> ---
Created attachment 2836
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2836&action=edit
clear AuthenticationMethods=any in fill_default_server_options()

I think it would be simpler to handle it similarly to the way we do
CLEAR_ON_NONE options, i.e. clear it in fill_default_server_options()
if it holds a special value.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2398

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2836|0                           |1
        is obsolete|                            |
                 CC|                            |dtucker at zip.com.au
   Attachment #2837|                            |ok?(dtucker at zip.com.au)
              Flags|                            |

--- Comment #4 from Damien Miller <djm at mindrot.org> ---
Created attachment 2837
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2837&action=edit
with manual bits

Revised diff with a mention of AuthenticationMethods=any in
sshd_config.5

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2398

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2837|ok?(dtucker at zip.com.au)     |
              Flags|                            |
   Attachment #2837|0                           |1
        is obsolete|                            |

--- Comment #5 from Damien Miller <djm at mindrot.org> ---
Created attachment 2838
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2838&action=edit
really with manual bits

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2398

Darren Tucker <dtucker at zip.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #2838|                            |ok+
              Flags|                            |

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2398

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #6 from Damien Miller <djm at mindrot.org> ---
patch applied, with an extra test for AuthenticationMethod=""

Will be released in openssh-7.3 - thanks!

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=2398

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #7 from Damien Miller <djm at mindrot.org> ---
Close all resolved bugs after 7.3p1 release

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.