[Buildroot] [PATCH] polarssl: deprecate on security grounds

[Buildroot] [PATCH] polarssl: deprecate on security grounds

[Gustavo Zacarias]

The 1.2.x branch is no longer maintained and the latest release from the
maintained branches (2.3, 2.1, 1.3) were security releases, so more
likely than not 1.2 is affected.
In consequence switch shairport-sync to the openssl backend.

Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
---
 Config.in.legacy                         | 10 ++++++++++
 package/polarssl/Config.in               |  1 +
 package/shairport-sync/Config.in         |  2 +-
 package/shairport-sync/shairport-sync.mk | 12 ++----------
 4 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/Config.in.legacy b/Config.in.legacy
index 2fae812..90a9cca 100644
--- a/Config.in.legacy
+++ b/Config.in.legacy
@@ -145,6 +145,16 @@ endif
 ###############################################################################
 comment "Legacy options removed in 2016.11"
 
+config BR2_PACKAGE_POLARSSL
+	bool "polarssl is no longer supported"
+	select BR2_LEGACY
+	help
+	  Upstream isn't maintaining the 1.2 branch any more and there
+	  has been a release in the maintained branches (2.3, 2.1, 1.3)
+	  that contains security fixes, so 1.2 is likely affected to a
+	  certain degree, and even if it isn't any future security fix
+	  will likely go unnoticed.
+
 config BR2_PACKAGE_WESTON_RPI
 	bool "Weston propietary RPI support is gone"
 	select BR2_LEGACY
diff --git a/package/polarssl/Config.in b/package/polarssl/Config.in
index 17125a3..3025e06 100644
--- a/package/polarssl/Config.in
+++ b/package/polarssl/Config.in
@@ -1,5 +1,6 @@
 config BR2_PACKAGE_POLARSSL
 	bool "polarssl"
+	depends on BR2_DEPRECATED_SINCE_2016_11
 	help
 	  PolarSSL is an SSL library written in ANSI C. PolarSSL makes
 	  it easy for developers to include cryptographic and SSL/TLS
diff --git a/package/shairport-sync/Config.in b/package/shairport-sync/Config.in
index c558a64..6c377f5 100644
--- a/package/shairport-sync/Config.in
+++ b/package/shairport-sync/Config.in
@@ -6,7 +6,7 @@ config BR2_PACKAGE_SHAIRPORT_SYNC
 	select BR2_PACKAGE_ALSA_LIB_MIXER
 	select BR2_PACKAGE_LIBCONFIG
 	select BR2_PACKAGE_LIBDAEMON
-	select BR2_PACKAGE_POLARSSL if !BR2_PACKAGE_OPENSSL
+	select BR2_PACKAGE_OPENSSL
 	select BR2_PACKAGE_POPT
 	help
 	  Shairport Sync emulates an AirPort Express for the purpose of
diff --git a/package/shairport-sync/shairport-sync.mk b/package/shairport-sync/shairport-sync.mk
index 49c6604..9f40758 100644
--- a/package/shairport-sync/shairport-sync.mk
+++ b/package/shairport-sync/shairport-sync.mk
@@ -9,7 +9,7 @@ SHAIRPORT_SYNC_SITE = $(call github,mikebrady,shairport-sync,$(SHAIRPORT_SYNC_VE
 
 SHAIRPORT_SYNC_LICENSE = MIT, BSD-3c
 SHAIRPORT_SYNC_LICENSE_FILES = LICENSES
-SHAIRPORT_SYNC_DEPENDENCIES = alsa-lib libconfig libdaemon popt host-pkgconf
+SHAIRPORT_SYNC_DEPENDENCIES = alsa-lib libconfig libdaemon openssl popt host-pkgconf
 
 # git clone, no configure
 SHAIRPORT_SYNC_AUTORECONF = YES
@@ -17,6 +17,7 @@ SHAIRPORT_SYNC_AUTORECONF = YES
 SHAIRPORT_SYNC_CONF_OPTS = --with-alsa \
 	--with-metadata \
 	--with-pipe \
+	--with-ssl=openssl \
 	--with-stdout
 
 # Avahi or tinysvcmdns (shaiport-sync bundles its own version of tinysvcmdns).
@@ -31,15 +32,6 @@ else
 SHAIRPORT_SYNC_CONF_OPTS += --with-tinysvcmdns
 endif
 
-# OpenSSL or PolarSSL
-ifeq ($(BR2_PACKAGE_OPENSSL),y)
-SHAIRPORT_SYNC_DEPENDENCIES += openssl
-SHAIRPORT_SYNC_CONF_OPTS += --with-ssl=openssl
-else
-SHAIRPORT_SYNC_DEPENDENCIES += polarssl
-SHAIRPORT_SYNC_CONF_OPTS += --with-ssl=polarssl
-endif
-
 ifeq ($(BR2_PACKAGE_SHAIRPORT_SYNC_LIBSOXR),y)
 SHAIRPORT_SYNC_DEPENDENCIES += libsoxr
 SHAIRPORT_SYNC_CONF_OPTS += --with-soxr
-- 
2.7.3

[Buildroot] [PATCH] polarssl: deprecate on security grounds

[Thomas Petazzoni]

Hello,

On Mon, 10 Oct 2016 14:26:33 -0300, Gustavo Zacarias wrote:
> The 1.2.x branch is no longer maintained and the latest release from the
> maintained branches (2.3, 2.1, 1.3) were security releases, so more
> likely than not 1.2 is affected.
> In consequence switch shairport-sync to the openssl backend.
> 
> Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>

I'm fine with the principle, but..

>  Config.in.legacy                         | 10 ++++++++++

... with your change, we get two definitions of the
BR2_PACKAGE_POLARSSL variable. The addition to Config.in.legacy should
only take place when the package gets removed. Deprecated packages are
meant to still allow the build to take place. But with the
Config.in.legacy option having the same name, you can't build, because
you're selecting BR2_LEGACY, which prevents the build from starting.

In fact, amusingly, with our process, "deprecating" a package means
that it blindly disappears for users.

But if you remove the package, we are warning the user by aborting the
build, thanks to the Config.in.legacy mechanism.

So I'm wondering if we shouldn't simply drop the polarssl package
altogether, in order to take advantage from the Config.in.legacy
mechanism.

Yann, Peter, Arnout, what do you think?

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

[Buildroot] [PATCH] polarssl: deprecate on security grounds

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:

 > Hello,
 > On Mon, 10 Oct 2016 14:26:33 -0300, Gustavo Zacarias wrote:
 >> The 1.2.x branch is no longer maintained and the latest release from the
 >> maintained branches (2.3, 2.1, 1.3) were security releases, so more
 >> likely than not 1.2 is affected.
 >> In consequence switch shairport-sync to the openssl backend.
 >> 
 >> Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>

 > I'm fine with the principle, but..

 >> Config.in.legacy                         | 10 ++++++++++

 > ... with your change, we get two definitions of the
 > BR2_PACKAGE_POLARSSL variable. The addition to Config.in.legacy should
 > only take place when the package gets removed. Deprecated packages are
 > meant to still allow the build to take place. But with the
 > Config.in.legacy option having the same name, you can't build, because
 > you're selecting BR2_LEGACY, which prevents the build from starting.

 > In fact, amusingly, with our process, "deprecating" a package means
 > that it blindly disappears for users.

Yeah, that isn't really good. Perhaps we need to add a 'select
BR2_NEEDS_DEPRECATED' to everything depending on BR2_DEPRECATED and add
some logic to warn if BR2_NEEDS_DEPRECATED && !BR2_DEPRECATED?


 > But if you remove the package, we are warning the user by aborting the
 > build, thanks to the Config.in.legacy mechanism.

 > So I'm wondering if we shouldn't simply drop the polarssl package
 > altogether, in order to take advantage from the Config.in.legacy
 > mechanism.

 > Yann, Peter, Arnout, what do you think?

Yes, I agree. Lets just remove it and add it to Config.in.legacy.

-- 
Venlig hilsen,
Peter Korsgaard 

[Buildroot] [PATCH] polarssl: deprecate on security grounds

[Arnout Vandecappelle]

On 11-10-16 19:13, Peter Korsgaard wrote:
>>>>>> "Thomas" == Thomas Petazzoni <thomas.petazzoni@free-electrons.com> writes:
> 
>  > Hello,
>  > On Mon, 10 Oct 2016 14:26:33 -0300, Gustavo Zacarias wrote:
>  >> The 1.2.x branch is no longer maintained and the latest release from the
>  >> maintained branches (2.3, 2.1, 1.3) were security releases, so more
>  >> likely than not 1.2 is affected.
>  >> In consequence switch shairport-sync to the openssl backend.
>  >> 
>  >> Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar>
> 
>  > I'm fine with the principle, but..
> 
>  >> Config.in.legacy                         | 10 ++++++++++
> 
>  > ... with your change, we get two definitions of the
>  > BR2_PACKAGE_POLARSSL variable. The addition to Config.in.legacy should
>  > only take place when the package gets removed. Deprecated packages are
>  > meant to still allow the build to take place. But with the
>  > Config.in.legacy option having the same name, you can't build, because
>  > you're selecting BR2_LEGACY, which prevents the build from starting.
> 
>  > In fact, amusingly, with our process, "deprecating" a package means
>  > that it blindly disappears for users.
> 
> Yeah, that isn't really good. Perhaps we need to add a 'select
> BR2_NEEDS_DEPRECATED' to everything depending on BR2_DEPRECATED and add
> some logic to warn if BR2_NEEDS_DEPRECATED && !BR2_DEPRECATED?

 That doesn't work. If you keep the dependency on BR2_DEPRECATED, then the
symbol won't be selectable without BR2_DEPRECATED so it will be silently removed
from .config. If you remove the dependency on BR2_DEPRECATED, then the package
stays user selectable. In that case, the BR2_DEPRECATED option becomes a bit
useless (it just hides the warning comment), and it's easy for users to
accidentally select a deprecated package.


>  > But if you remove the package, we are warning the user by aborting the
>  > build, thanks to the Config.in.legacy mechanism.
> 
>  > So I'm wondering if we shouldn't simply drop the polarssl package
>  > altogether, in order to take advantage from the Config.in.legacy
>  > mechanism.
> 
>  > Yann, Peter, Arnout, what do you think?
> 
> Yes, I agree. Lets just remove it and add it to Config.in.legacy.

 I completely agree, BR2_DEPRECATED should be deprecated :-P. Really, I see no
point in "deprecating" a package or feature. It doesn't remove much of our
maintenance burden, except that it no longer appears in autobuilder results. But
the code is still there. When the deprecated package is eventually really
removed, you have to go through the entire tree again to check if there are some
hidden dependencies or whatnot on the package, so it actively adds to the
maintenance burden. And what does it bring to the user? That he can stumble on
for another 18 months with this deprecated package, but without support and
eventually the package gets removed anyway.

 No, it's as easy for the user to just revert the package removal or revive it
in BR2_EXTERNAL. In addition, the legacy support gives the user a warning,
sometimes a way to solve it, and a help text that can explain why the package
was removed.

 The only case where deprecation would make a little bit of sense is for version
deprecation (old binutils, gcc, kernel-headers versions), because those can't
easily be "fixed" in BR2_EXTERNAL. But they can be fixed with OVERRIDE_SRCDIR so
I'm still not convinced.


 You can task me with deprecating BR2_DEPRECATED during the devel days - mainly
documentation updates.


 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

[Buildroot] [PATCH] polarssl: deprecate on security grounds

[Peter Korsgaard]

>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

Hi,

 >> > In fact, amusingly, with our process, "deprecating" a package means
 >> > that it blindly disappears for users.
 >> 
 >> Yeah, that isn't really good. Perhaps we need to add a 'select
 >> BR2_NEEDS_DEPRECATED' to everything depending on BR2_DEPRECATED and add
 >> some logic to warn if BR2_NEEDS_DEPRECATED && !BR2_DEPRECATED?

 >  That doesn't work. If you keep the dependency on BR2_DEPRECATED, then the
 > symbol won't be selectable without BR2_DEPRECATED so it will be silently removed
 > from .config. If you remove the dependency on BR2_DEPRECATED, then the package
 > stays user selectable. In that case, the BR2_DEPRECATED option becomes a bit
 > useless (it just hides the warning comment), and it's easy for users to
 > accidentally select a deprecated package.

Ahh yes, true.

 >  You can task me with deprecating BR2_DEPRECATED during the devel
 > days - mainly documentation updates.

Ok, sounds good to me!

-- 
Venlig hilsen,
Peter Korsgaard