[1/3] apply logging settings for OpenSSH correctly

Message ID 6d69e16d-e93b-6c91-a7c1-7731f821e537@link38.eu
State Superseded
Headers
Series [1/3] apply logging settings for OpenSSH correctly |

Commit Message

Peter Müller May 1, 2018, 10:40 p.m. UTC
  The logging settings for OpenSSH (log to syslog with "AUTH"
facility at "INFO" level) were not applied correctly. This
patch fixes that for both installed systems and the LFS file.

Partially addresses #11538.

Signed-off-by: Peter Müller <peter.mueller@link38.eu>
---
 config/rootfiles/core/121/update.sh | 6 ++++++
 lfs/openssh                         | 4 ++--
 2 files changed, 8 insertions(+), 2 deletions(-)
  

Comments

Michael Tremer May 30, 2018, 9:29 p.m. UTC | #1
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

I guess this looks good.

The problem here certainly was that editing a file that comes from upstream with
sed is not a good idea. One line changed can cause the sed to do nothing and we
won't even notice it. Therefore, in the future, I will only accept patches for
changes like this. Those won't apply and then we can investigate why.

Best,
- -Michael

On Tue, 2018-05-01 at 14:40 +0200, Peter Müller wrote:
> The logging settings for OpenSSH (log to syslog with "AUTH"
> facility at "INFO" level) were not applied correctly. This
> patch fixes that for both installed systems and the LFS file.
> 
> Partially addresses #11538.
> 
> Signed-off-by: Peter Müller <peter.mueller@link38.eu>
> ---
>  config/rootfiles/core/121/update.sh | 6 ++++++
>  lfs/openssh                         | 4 ++--
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/config/rootfiles/core/121/update.sh
> b/config/rootfiles/core/121/update.sh
> index 87d5f6ebd..5b8f2c86e 100644
> --- a/config/rootfiles/core/121/update.sh
> +++ b/config/rootfiles/core/121/update.sh
> @@ -56,7 +56,13 @@ rm -rvf \
>  	/usr/share/nagios/ \
>  	/var/nagios/
>  
> +# Update SSH configuration
> +sed -i /etc/ssh/sshd_config \
> +	-e 's/^#SyslogFacility AUTH$/SyslogFacility AUTH/' \
> +	-e 's/^#LogLevel INFO$/LogLevel INFO/'
> +
>  # Start services
> +/etc/init.d/sshd restart
>  /etc/init.d/apache restart
>  
>  # This update needs a reboot...
> diff --git a/lfs/openssh b/lfs/openssh
> index 203446370..46561953d 100644
> --- a/lfs/openssh
> +++ b/lfs/openssh
> @@ -91,8 +91,8 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
>  		-e 's/^#\?IgnoreUserKnownHosts .*$$/IgnoreUserKnownHosts
> yes/' \
>  		-e 's/^#\?UsePAM .*$$//' \
>  		-e 's/^#\?X11Forwarding .*$$/X11Forwarding no/' \
> -		-e 's/^#\?SyslogFacility AUTH .*$$/SyslogFacility AUTH/' \
> -		-e 's/^#\?LogLevel INFO .*$$/LogLevel INFO/' \
> +		-e 's/^#SyslogFacility AUTH$/SyslogFacility AUTH/' \
> +		-e 's/^#LogLevel INFO$/LogLevel INFO/' \
>  		-e 's/^#\?AllowTcpForwarding .*$$/AllowTcpForwarding no/' \
>  		-e 's/^#\?PermitRootLogin .*$$/PermitRootLogin yes/' \
>  		-e 's|^#\?HostKey /etc/ssh/ssh_host_dsa_key$$||' \
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEE5/rW5l3GGe2ypktxgHnw/2+QCQcFAlsOiw4ACgkQgHnw/2+Q
CQdTEw//YlhxZ+tWo9FosukgobdG6nh2bSc9dNm2VDu7e3mXiYp4jjKipW06cBzT
53X0CNDyLLlCxMoU+KX4UzMVsGLi0MIQDXc6cFYxnnjM32r4/yvVEuKN1QcdHXXG
aZcdDgQvpoN2Ao0wIWBAmyY6GkKmXBhdM0PMy7367xpKnGfyr/9uucW25j8vo8Lp
qmGbai52/Eg0lCBgWHNh3P9EqMS8ddjop90a/MKVY+CCQY0DsxN/z4Ijqgx1GApn
39C/mosCE+Wt0rOdJiomVLEvV7bR0SBe3S3j6J2/0er4RVnTb3X74JvBsIsn1RNl
rU5UY35eaBSNGDLrGrpYsJ/0L5NVzqMFFxZnKAy150Ge3Gc/fjZ94q4gV+j2R8SH
FNbsEXCMkX06SnLK1WaMpvbCu0SivS7DCphE1SWcX3rGHrcPrQh2bqTEw62sgGQM
IeSTKlO+1ZSODQbP8byYCgqnjRmsP2xLQLbkcgkMPExkXaLqG/sK+mWaJPGr/Rjf
y9rOlWgWliv6jDbfDjQjHI+VINuPJNm0qn7ZVTQC9EB+/Xt/D33Z7zbmfATTmHHg
wgPbIyTULlRjo9aulpPCx2hEp7lVWH5OMkFtBI5u9PwDnlmxHCQLdb/kV+gLOX2+
C42G09Ils+8rvkiQZUFN2pHNake3URdRu0SYwuPJROO8pvjGHjs=
=9jb+
-----END PGP SIGNATURE-----
  
Peter Müller May 31, 2018, 5:47 a.m. UTC | #2
Hello Michael,

since we edit a lot of settings in the sshd_config file (and perhaps in
the ssh_config file, too, when it comes to cipher selection), should we
introduce a completely own config file? If so, how do I do so?

We still need to manipulate it via sed for existing installations (via
the update.sh script), but we could omit the procedure during building
the package.

As most of the config file is commented out by default, it could
also be made much smaller and easier to read, only containing settings
different than the defaults.

Best regards,
Peter Müller

> I guess this looks good.
> 
> The problem here certainly was that editing a file that comes from upstream with
> sed is not a good idea. One line changed can cause the sed to do nothing and we
> won't even notice it. Therefore, in the future, I will only accept patches for
> changes like this. Those won't apply and then we can investigate why.
> 
> Best,
> -Michael
> 
> On Tue, 2018-05-01 at 14:40 +0200, Peter Müller wrote:
>> The logging settings for OpenSSH (log to syslog with "AUTH"
>> facility at "INFO" level) were not applied correctly. This
>> patch fixes that for both installed systems and the LFS file.
> 
>> Partially addresses #11538.
> 
>> Signed-off-by: Peter Müller <peter.mueller@link38.eu>
>> ---
>>  config/rootfiles/core/121/update.sh | 6 ++++++
>>  lfs/openssh                         | 4 ++--
>>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
>> diff --git a/config/rootfiles/core/121/update.sh
>> b/config/rootfiles/core/121/update.sh
>> index 87d5f6ebd..5b8f2c86e 100644
>> --- a/config/rootfiles/core/121/update.sh
>> +++ b/config/rootfiles/core/121/update.sh
>> @@ -56,7 +56,13 @@ rm -rvf \
>>  	/usr/share/nagios/ \
>>  	/var/nagios/
> 
>> +# Update SSH configuration
>> +sed -i /etc/ssh/sshd_config \
>> +	-e 's/^#SyslogFacility AUTH$/SyslogFacility AUTH/' \
>> +	-e 's/^#LogLevel INFO$/LogLevel INFO/'
>> +
>>  # Start services
>> +/etc/init.d/sshd restart
>>  /etc/init.d/apache restart
> 
>>  # This update needs a reboot...
>> diff --git a/lfs/openssh b/lfs/openssh
>> index 203446370..46561953d 100644
>> --- a/lfs/openssh
>> +++ b/lfs/openssh
>> @@ -91,8 +91,8 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
>>  		-e 's/^#\?IgnoreUserKnownHosts .*$$/IgnoreUserKnownHosts
>> yes/' \
>>  		-e 's/^#\?UsePAM .*$$//' \
>>  		-e 's/^#\?X11Forwarding .*$$/X11Forwarding no/' \
>> -		-e 's/^#\?SyslogFacility AUTH .*$$/SyslogFacility AUTH/' \
>> -		-e 's/^#\?LogLevel INFO .*$$/LogLevel INFO/' \
>> +		-e 's/^#SyslogFacility AUTH$/SyslogFacility AUTH/' \
>> +		-e 's/^#LogLevel INFO$/LogLevel INFO/' \
>>  		-e 's/^#\?AllowTcpForwarding .*$$/AllowTcpForwarding no/' \
>>  		-e 's/^#\?PermitRootLogin .*$$/PermitRootLogin yes/' \
>>  		-e 's|^#\?HostKey /etc/ssh/ssh_host_dsa_key$$||' \
>
  
Michael Tremer May 31, 2018, 8:32 p.m. UTC | #3
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On Wed, 2018-05-30 at 21:47 +0200, Peter Müller wrote:
> Hello Michael,
> 
> since we edit a lot of settings in the sshd_config file (and perhaps in
> the ssh_config file, too, when it comes to cipher selection), should we
> introduce a completely own config file? If so, how do I do so?

Well, write a new configuration file and a script that takes the settings from
the previous one and changes it accordingly.

Those settings should also be in /var/ipfire/remote/settings.

> We still need to manipulate it via sed for existing installations (via
> the update.sh script), but we could omit the procedure during building
> the package.

Yes.

> As most of the config file is commented out by default, it could
> also be made much smaller and easier to read, only containing settings
> different than the defaults.

Yes, we can remove lots here.

I think we should keep this as easy as possible because we got loads of other
things to take care of.

Best,
- -Michael

> 
> Best regards,
> Peter Müller
> 
> > I guess this looks good.
> > 
> > The problem here certainly was that editing a file that comes from upstream
> > with
> > sed is not a good idea. One line changed can cause the sed to do nothing and
> > we
> > won't even notice it. Therefore, in the future, I will only accept patches
> > for
> > changes like this. Those won't apply and then we can investigate why.
> > 
> > Best,
> > -Michael
> > 
> > On Tue, 2018-05-01 at 14:40 +0200, Peter Müller wrote:
> > > The logging settings for OpenSSH (log to syslog with "AUTH"
> > > facility at "INFO" level) were not applied correctly. This
> > > patch fixes that for both installed systems and the LFS file.
> > > Partially addresses #11538.
> > > Signed-off-by: Peter Müller <peter.mueller@link38.eu>
> > > ---
> > >  config/rootfiles/core/121/update.sh | 6 ++++++
> > >  lfs/openssh                         | 4 ++--
> > >  2 files changed, 8 insertions(+), 2 deletions(-)
> > > diff --git a/config/rootfiles/core/121/update.sh
> > > b/config/rootfiles/core/121/update.sh
> > > index 87d5f6ebd..5b8f2c86e 100644
> > > --- a/config/rootfiles/core/121/update.sh
> > > +++ b/config/rootfiles/core/121/update.sh
> > > @@ -56,7 +56,13 @@ rm -rvf \
> > >  	/usr/share/nagios/ \
> > >  	/var/nagios/
> > > +# Update SSH configuration
> > > +sed -i /etc/ssh/sshd_config \
> > > +	-e 's/^#SyslogFacility AUTH$/SyslogFacility AUTH/' \
> > > +	-e 's/^#LogLevel INFO$/LogLevel INFO/'
> > > +
> > >  # Start services
> > > +/etc/init.d/sshd restart
> > >  /etc/init.d/apache restart
> > >  # This update needs a reboot...
> > > diff --git a/lfs/openssh b/lfs/openssh
> > > index 203446370..46561953d 100644
> > > --- a/lfs/openssh
> > > +++ b/lfs/openssh
> > > @@ -91,8 +91,8 @@ $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
> > >  		-e 's/^#\?IgnoreUserKnownHosts .*$$/IgnoreUserKnownHosts
> > > yes/' \
> > >  		-e 's/^#\?UsePAM .*$$//' \
> > >  		-e 's/^#\?X11Forwarding .*$$/X11Forwarding no/' \
> > > -		-e 's/^#\?SyslogFacility AUTH .*$$/SyslogFacility AUTH/'
> > > \
> > > -		-e 's/^#\?LogLevel INFO .*$$/LogLevel INFO/' \
> > > +		-e 's/^#SyslogFacility AUTH$/SyslogFacility AUTH/' \
> > > +		-e 's/^#LogLevel INFO$/LogLevel INFO/' \
> > >  		-e 's/^#\?AllowTcpForwarding .*$$/AllowTcpForwarding no/'
> > > \
> > >  		-e 's/^#\?PermitRootLogin .*$$/PermitRootLogin yes/' \
> > >  		-e 's|^#\?HostKey /etc/ssh/ssh_host_dsa_key$$||' \
> 
> 
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEE5/rW5l3GGe2ypktxgHnw/2+QCQcFAlsPz1UACgkQgHnw/2+Q
CQf/bQ//WJsgSR+unMsIFby8diKw6knYkbN2sW/ODN5DLn6bLfAiO59Xdma6FhUP
iLH2T8ftoCDwn/6ScHPhSmmlFl4j7P6Vqaagz7uykI5ul62+VLhqhViqS9FVI6qb
GD4OdZ8VvrXfl/DNHd9fSoNp2eUzYBWoyqajA1vIyNMfldhRMOFe49VOprOm3HBB
nVFKbScjeW6m+FCAVSnTfB6F57gMqVHHtPINbkFIHaa86KAbLrJYP9xkPUd9yT7O
kCoA0QUKRk78Nmho108na9ife0HPgTHDUmh9qR1GRe9pXGrJvr8g+VXjdBfLgl6U
0gwmv8bMPYTxKnM8gmwXQZwRjeVrdiOttnnbeTI24zzgivXoECRtl0ijn4nobs9t
moJ9ilk0SpmI/+Fmgy3jejExKelI5pcOxEW4yjSaBb4oeTRLHy5vO8bT0t1XHVxB
fRLVbyhhdBa0duTSVsky/JmtjhuFdcCo+aX6oebsySOHW+QcH36AkCqSxF1odw+/
9kKwrXQNFYOUiSTIQfix6WeOKbWzIe3T4s72//T7LXVGBqNdw/DEii5Zvu8BMsTR
VUq/e8gxsfmCEEzAUdlGXkA9lNROcbntRLQ8Zcp1ogZnKVkmgUlyrr+ad7TJoAz3
/kUvS5Lg1pZKDxnf0r34cAReQSx7RgnJHZiq9WnUJxRSFITsvwo=
=AtA3
-----END PGP SIGNATURE-----
  

Patch

diff --git a/config/rootfiles/core/121/update.sh b/config/rootfiles/core/121/update.sh
index 87d5f6ebd..5b8f2c86e 100644
--- a/config/rootfiles/core/121/update.sh
+++ b/config/rootfiles/core/121/update.sh
@@ -56,7 +56,13 @@  rm -rvf \
 	/usr/share/nagios/ \
 	/var/nagios/
 
+# Update SSH configuration
+sed -i /etc/ssh/sshd_config \
+	-e 's/^#SyslogFacility AUTH$/SyslogFacility AUTH/' \
+	-e 's/^#LogLevel INFO$/LogLevel INFO/'
+
 # Start services
+/etc/init.d/sshd restart
 /etc/init.d/apache restart
 
 # This update needs a reboot...
diff --git a/lfs/openssh b/lfs/openssh
index 203446370..46561953d 100644
--- a/lfs/openssh
+++ b/lfs/openssh
@@ -91,8 +91,8 @@  $(TARGET) : $(patsubst %,$(DIR_DL)/%,$(objects))
 		-e 's/^#\?IgnoreUserKnownHosts .*$$/IgnoreUserKnownHosts yes/' \
 		-e 's/^#\?UsePAM .*$$//' \
 		-e 's/^#\?X11Forwarding .*$$/X11Forwarding no/' \
-		-e 's/^#\?SyslogFacility AUTH .*$$/SyslogFacility AUTH/' \
-		-e 's/^#\?LogLevel INFO .*$$/LogLevel INFO/' \
+		-e 's/^#SyslogFacility AUTH$/SyslogFacility AUTH/' \
+		-e 's/^#LogLevel INFO$/LogLevel INFO/' \
 		-e 's/^#\?AllowTcpForwarding .*$$/AllowTcpForwarding no/' \
 		-e 's/^#\?PermitRootLogin .*$$/PermitRootLogin yes/' \
 		-e 's|^#\?HostKey /etc/ssh/ssh_host_dsa_key$$||' \