diff mbox series

[meta-arago,master/kirkstone,2/2] shadow-securetty: Do not allow root login over telnet

Message ID 20231016211840.23815-2-afd@ti.com
State New
Headers show
Series [meta-arago,master/kirkstone,1/2] initscript-telnetd: Remove this package | expand

Commit Message

Andrew Davis Oct. 16, 2023, 9:18 p.m. UTC
I'm sure I don't have to explain why this was a bad idea..

Signed-off-by: Andrew Davis <afd@ti.com>
---
 .../shadow/shadow-securetty_%.bbappend            | 15 ---------------
 1 file changed, 15 deletions(-)
 delete mode 100644 meta-arago-distro/recipes-extended/shadow/shadow-securetty_%.bbappend

Comments

Chirag Shilwant Oct. 17, 2023, 9:22 a.m. UTC | #1
On 17/10/23 02:48, Andrew Davis via lists.yoctoproject.org wrote:
> I'm sure I don't have to explain why this was a bad idea..

Still, It will be good to have a commit message explaining it :)

>
> Signed-off-by: Andrew Davis <afd@ti.com>
> ---
>   .../shadow/shadow-securetty_%.bbappend            | 15 ---------------
>   1 file changed, 15 deletions(-)
>   delete mode 100644 meta-arago-distro/recipes-extended/shadow/shadow-securetty_%.bbappend
>
> diff --git a/meta-arago-distro/recipes-extended/shadow/shadow-securetty_%.bbappend b/meta-arago-distro/recipes-extended/shadow/shadow-securetty_%.bbappend
> deleted file mode 100644
> index 62999d2a..00000000
> --- a/meta-arago-distro/recipes-extended/shadow/shadow-securetty_%.bbappend
> +++ /dev/null
> @@ -1,15 +0,0 @@
> -PR:append = ".arago0"
> -
> -do_install:append () {
> -    # Allow telnet sessions to login as root
> -    securetty_file=${D}${sysconfdir}/securetty
> -
> -    echo '' >> $securetty_file
> -    echo '# Allow 5 telnet login' >> $securetty_file
> -    echo 'pts/0' >> $securetty_file
> -    echo 'pts/1' >> $securetty_file
> -    echo 'pts/2' >> $securetty_file
> -    echo 'pts/3' >> $securetty_file
> -    echo 'pts/4' >> $securetty_file
> -
> -}
Denys Dmytriyenko Oct. 17, 2023, 5:28 p.m. UTC | #2
On Tue, Oct 17, 2023 at 02:52:43PM +0530, Chirag Shilwant wrote:
> 
> On 17/10/23 02:48, Andrew Davis via lists.yoctoproject.org wrote:
> >I'm sure I don't have to explain why this was a bad idea..
> 
> Still, It will be good to have a commit message explaining it :)

It is a very obvious major security weakness and is definitely a very bad 
idea for an end product!

But, there was never a clear definition of what meta-arago is - is it an 
end product distribution or simply a test environment for the BSP/SDK.

This was added over 10 years ago as part of AM-SDK for ease of testing. 
Even though the commit does not explain it [1], we had a discussion and 
the security implications of sending telnet passwords in clear text were 
questioned.

The counter-argument here is that we build "debug" images w/o root password 
anyway by default, so allowing password-less root logins over telnet is 
rather a moot point, as we already allow the same for ssh.

Mayve instead of completely removing this, it should be conditional and 
only enabled when "debug-tweaks" is enabled in EXTRA_IMAGE_FEATURES, 
similar to allowing ssh root logins w/o a password.

[1] https://git.yoctoproject.org/meta-arago/commit/?id=98b6209a3010e32da963a0f6f53fceebbc37f8f9


> >Signed-off-by: Andrew Davis <afd@ti.com>
> >---
> >  .../shadow/shadow-securetty_%.bbappend            | 15 ---------------
> >  1 file changed, 15 deletions(-)
> >  delete mode 100644 meta-arago-distro/recipes-extended/shadow/shadow-securetty_%.bbappend
> >
> >diff --git a/meta-arago-distro/recipes-extended/shadow/shadow-securetty_%.bbappend b/meta-arago-distro/recipes-extended/shadow/shadow-securetty_%.bbappend
> >deleted file mode 100644
> >index 62999d2a..00000000
> >--- a/meta-arago-distro/recipes-extended/shadow/shadow-securetty_%.bbappend
> >+++ /dev/null
> >@@ -1,15 +0,0 @@
> >-PR:append = ".arago0"
> >-
> >-do_install:append () {
> >-    # Allow telnet sessions to login as root
> >-    securetty_file=${D}${sysconfdir}/securetty
> >-
> >-    echo '' >> $securetty_file
> >-    echo '# Allow 5 telnet login' >> $securetty_file
> >-    echo 'pts/0' >> $securetty_file
> >-    echo 'pts/1' >> $securetty_file
> >-    echo 'pts/2' >> $securetty_file
> >-    echo 'pts/3' >> $securetty_file
> >-    echo 'pts/4' >> $securetty_file
> >-
> >-}
Ryan Eatmon Oct. 19, 2023, 10:02 p.m. UTC | #3
On 10/17/2023 12:28 PM, Denys Dmytriyenko wrote:
> On Tue, Oct 17, 2023 at 02:52:43PM +0530, Chirag Shilwant wrote:
>>
>> On 17/10/23 02:48, Andrew Davis via lists.yoctoproject.org wrote:
>>> I'm sure I don't have to explain why this was a bad idea..
>>
>> Still, It will be good to have a commit message explaining it :)
> 
> It is a very obvious major security weakness and is definitely a very bad
> idea for an end product!
> 
> But, there was never a clear definition of what meta-arago is - is it an
> end product distribution or simply a test environment for the BSP/SDK.
> 
> This was added over 10 years ago as part of AM-SDK for ease of testing.
> Even though the commit does not explain it [1], we had a discussion and
> the security implications of sending telnet passwords in clear text were
> questioned.
> 
> The counter-argument here is that we build "debug" images w/o root password
> anyway by default, so allowing password-less root logins over telnet is
> rather a moot point, as we already allow the same for ssh.
> 
> Mayve instead of completely removing this, it should be conditional and
> only enabled when "debug-tweaks" is enabled in EXTRA_IMAGE_FEATURES,
> similar to allowing ssh root logins w/o a password.
> 
> [1] https://git.yoctoproject.org/meta-arago/commit/?id=98b6209a3010e32da963a0f6f53fceebbc37f8f9
> 

Well, we have to keep this for now.  We will work to disable the telnet 
requirement in our testing flow and move to ssh.  At that point we can 
revisit this patch.


>>> Signed-off-by: Andrew Davis <afd@ti.com>
>>> ---
>>>   .../shadow/shadow-securetty_%.bbappend            | 15 ---------------
>>>   1 file changed, 15 deletions(-)
>>>   delete mode 100644 meta-arago-distro/recipes-extended/shadow/shadow-securetty_%.bbappend
>>>
>>> diff --git a/meta-arago-distro/recipes-extended/shadow/shadow-securetty_%.bbappend b/meta-arago-distro/recipes-extended/shadow/shadow-securetty_%.bbappend
>>> deleted file mode 100644
>>> index 62999d2a..00000000
>>> --- a/meta-arago-distro/recipes-extended/shadow/shadow-securetty_%.bbappend
>>> +++ /dev/null
>>> @@ -1,15 +0,0 @@
>>> -PR:append = ".arago0"
>>> -
>>> -do_install:append () {
>>> -    # Allow telnet sessions to login as root
>>> -    securetty_file=${D}${sysconfdir}/securetty
>>> -
>>> -    echo '' >> $securetty_file
>>> -    echo '# Allow 5 telnet login' >> $securetty_file
>>> -    echo 'pts/0' >> $securetty_file
>>> -    echo 'pts/1' >> $securetty_file
>>> -    echo 'pts/2' >> $securetty_file
>>> -    echo 'pts/3' >> $securetty_file
>>> -    echo 'pts/4' >> $securetty_file
>>> -
>>> -}
diff mbox series

Patch

diff --git a/meta-arago-distro/recipes-extended/shadow/shadow-securetty_%.bbappend b/meta-arago-distro/recipes-extended/shadow/shadow-securetty_%.bbappend
deleted file mode 100644
index 62999d2a..00000000
--- a/meta-arago-distro/recipes-extended/shadow/shadow-securetty_%.bbappend
+++ /dev/null
@@ -1,15 +0,0 @@ 
-PR:append = ".arago0"
-
-do_install:append () {
-    # Allow telnet sessions to login as root
-    securetty_file=${D}${sysconfdir}/securetty
-
-    echo '' >> $securetty_file
-    echo '# Allow 5 telnet login' >> $securetty_file
-    echo 'pts/0' >> $securetty_file
-    echo 'pts/1' >> $securetty_file
-    echo 'pts/2' >> $securetty_file
-    echo 'pts/3' >> $securetty_file
-    echo 'pts/4' >> $securetty_file
-
-}