[Buildroot] [PATCH 1/3] package/mbedtls: add BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION

[Buildroot] [PATCH 1/3] package/mbedtls: add BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION

[Fabrice Fontaine]

Add an option to enable
MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 package/mbedtls/Config.in  | 10 ++++++++++
 package/mbedtls/mbedtls.mk |  8 ++++++++
 2 files changed, 18 insertions(+)

diff --git a/package/mbedtls/Config.in b/package/mbedtls/Config.in
index a39ba65d98..e48f0473b0 100644
--- a/package/mbedtls/Config.in
+++ b/package/mbedtls/Config.in
@@ -29,4 +29,14 @@ config BR2_PACKAGE_MBEDTLS_COMPRESSION
 	  sure CRIME and similar attacks are not applicable to your
 	  particular situation.
 
+config BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION
+	bool "allow X509 unsupported critical extension"
+	help
+	  If set, the X509 parser will not break-off when parsing an
+	  X509 certificate and encountering an unknown critical
+	  extension.
+
+	  Warning: Depending on your PKI use, enabling this can be a
+	  security risk!
+
 endif
diff --git a/package/mbedtls/mbedtls.mk b/package/mbedtls/mbedtls.mk
index 50121fa6c7..155cb8db53 100644
--- a/package/mbedtls/mbedtls.mk
+++ b/package/mbedtls/mbedtls.mk
@@ -51,6 +51,14 @@ else
 MBEDTLS_CONF_OPTS += -DENABLE_ZLIB_SUPPORT=OFF
 endif
 
+ifeq ($(BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION),y)
+define MBEDTLS_ENABLE_X509_UNSUPPORTED_CRITICAL_EXTENSION
+	$(SED) "s://#define MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION:#define MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION:" \
+		$(@D)/include/mbedtls/config.h
+endef
+MBEDTLS_POST_PATCH_HOOKS += MBEDTLS_ENABLE_X509_UNSUPPORTED_CRITICAL_EXTENSION
+endif
+
 define MBEDTLS_DISABLE_ASM
 	$(SED) '/^#define MBEDTLS_AESNI_C/d' \
 		$(@D)/include/mbedtls/config.h
-- 
2.25.1

[Buildroot] [PATCH 2/3] package/uacme: allow selection of crypto backend

[Fabrice Fontaine]

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 package/uacme/Config.in | 19 +++++++++++++++++++
 package/uacme/uacme.mk  |  6 +++---
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/package/uacme/Config.in b/package/uacme/Config.in
index 58b7c534e7..c27727d8c7 100644
--- a/package/uacme/Config.in
+++ b/package/uacme/Config.in
@@ -16,6 +16,25 @@ config BR2_PACKAGE_UACME
 
 if BR2_PACKAGE_UACME
 
+choice
+	prompt "Crypto Backend"
+	help
+	  Select crypto library to be used in uacme.
+
+config BR2_PACKAGE_UACME_GNUTLS
+	bool "gnutls"
+	depends on BR2_PACKAGE_GNUTLS
+
+config BR2_PACKAGE_UACME_MBEDTLS
+	bool "mbedtls"
+	depends on BR2_PACKAGE_MBEDTLS
+
+config BR2_PACKAGE_UACME_OPENSSL
+	bool "openssl"
+	depends on BR2_PACKAGE_OPENSSL
+
+endchoice
+
 config BR2_PACKAGE_UACME_UALPN
 	bool "enable ualpn"
 	depends on BR2_TOOLCHAIN_HAS_THREADS
diff --git a/package/uacme/uacme.mk b/package/uacme/uacme.mk
index 61d3f11ca1..470c608bb1 100644
--- a/package/uacme/uacme.mk
+++ b/package/uacme/uacme.mk
@@ -15,13 +15,13 @@ UACME_DEPENDENCIES = libcurl
 
 UACME_CONF_ENV = ac_cv_prog_cc_c99='-std=gnu99'
 
-ifeq ($(BR2_PACKAGE_GNUTLS),y)
+ifeq ($(BR2_PACKAGE_UACME_GNUTLS),y)
 UACME_CONF_OPTS += --with-gnutls
 UACME_DEPENDENCIES += gnutls
-else ifeq ($(BR2_PACKAGE_MBEDTLS),y)
+else ifeq ($(BR2_PACKAGE_UACME_MBEDTLS),y)
 UACME_CONF_OPTS += --with-mbedtls
 UACME_DEPENDENCIES += mbedtls
-else ifeq ($(BR2_PACKAGE_OPENSSL),y)
+else ifeq ($(BR2_PACKAGE_UACME_OPENSSL),y)
 UACME_CONF_OPTS += --with-openssl
 UACME_DEPENDENCIES += openssl
 endif
-- 
2.25.1

[Buildroot] [PATCH 3/3] package/uacme: ualpn needs X509 unsupported critical extension support

[Fabrice Fontaine]

Since version 1.2 and
https://github.com/ndilieto/uacme/commit/ae483af8953fa478de1e224c0c556ba3a77e3e01,
ualpn on mbedtls needs X509 unsupported critical extension support

Fixes:
 - http://autobuild.buildroot.org/results/5d42189299549cd655218e9e7cfcfa63e79f74ec

Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
---
 package/uacme/Config.in | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/package/uacme/Config.in b/package/uacme/Config.in
index c27727d8c7..0d7cea6d6b 100644
--- a/package/uacme/Config.in
+++ b/package/uacme/Config.in
@@ -38,6 +38,7 @@ endchoice
 config BR2_PACKAGE_UACME_UALPN
 	bool "enable ualpn"
 	depends on BR2_TOOLCHAIN_HAS_THREADS
+	depends on !BR2_PACKAGE_UACME_MBEDTLS || BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION
 	select BR2_PACKAGE_LIBEV
 	help
 	  Build and install ualpn, the transparent proxying tls-alpn-01
@@ -46,4 +47,8 @@ config BR2_PACKAGE_UACME_UALPN
 comment "ualpn needs a toolchain w/ threads"
 	depends on !BR2_TOOLCHAIN_HAS_THREADS
 
+comment "ualpn needs X509 unsupported critical extension support in mbedtls"
+	depends on BR2_PACKAGE_UACME_MBEDTLS
+	depends on !BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION
+
 endif
-- 
2.25.1

[Buildroot] [PATCH 1/3] package/mbedtls: add BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION

[Thomas Petazzoni]

On Wed, 22 Apr 2020 21:20:57 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> Add an option to enable
> MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION
> 
> Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> ---
>  package/mbedtls/Config.in  | 10 ++++++++++
>  package/mbedtls/mbedtls.mk |  8 ++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/package/mbedtls/Config.in b/package/mbedtls/Config.in
> index a39ba65d98..e48f0473b0 100644
> --- a/package/mbedtls/Config.in
> +++ b/package/mbedtls/Config.in
> @@ -29,4 +29,14 @@ config BR2_PACKAGE_MBEDTLS_COMPRESSION
>  	  sure CRIME and similar attacks are not applicable to your
>  	  particular situation.
>  
> +config BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION
> +	bool "allow X509 unsupported critical extension"
> +	help
> +	  If set, the X509 parser will not break-off when parsing an
> +	  X509 certificate and encountering an unknown critical
> +	  extension.
> +
> +	  Warning: Depending on your PKI use, enabling this can be a
> +	  security risk!
> +
>  endif

This whole series is pretty awkward. Shouldn't we instead simply not
allow the use of uacme mbedtls crypto backend ?

What is this X509_UNSUPPORTED_CRITICAL_EXTENSION functionality that is
so weird that it requires patching the mbedtls config.h file ? Why is
uacme absolutely requiring this functionality that no other user of
mbedtls requires ?

Until these questions are answered, I'd prefer to drop support for
mbedtls as a crypto backend for uacme.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/3] package/mbedtls: add BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION

[Yann E. MORIN]

Thomas, All,

On 2020-04-23 22:09 +0200, Thomas Petazzoni spake thusly:
> On Wed, 22 Apr 2020 21:20:57 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
> 
> > Add an option to enable
> > MBEDTLS_X509_ALLOW_UNSUPPORTED_CRITICAL_EXTENSION
> > 
> > Signed-off-by: Fabrice Fontaine <fontaine.fabrice@gmail.com>
> > ---
> >  package/mbedtls/Config.in  | 10 ++++++++++
> >  package/mbedtls/mbedtls.mk |  8 ++++++++
> >  2 files changed, 18 insertions(+)
> > 
> > diff --git a/package/mbedtls/Config.in b/package/mbedtls/Config.in
> > index a39ba65d98..e48f0473b0 100644
> > --- a/package/mbedtls/Config.in
> > +++ b/package/mbedtls/Config.in
> > @@ -29,4 +29,14 @@ config BR2_PACKAGE_MBEDTLS_COMPRESSION
> >  	  sure CRIME and similar attacks are not applicable to your
> >  	  particular situation.
> >  
> > +config BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION
> > +	bool "allow X509 unsupported critical extension"
> > +	help
> > +	  If set, the X509 parser will not break-off when parsing an
> > +	  X509 certificate and encountering an unknown critical
> > +	  extension.
> > +
> > +	  Warning: Depending on your PKI use, enabling this can be a
> > +	  security risk!
> > +
> >  endif
> 
> This whole series is pretty awkward. Shouldn't we instead simply not
> allow the use of uacme mbedtls crypto backend ?
> 
> What is this X509_UNSUPPORTED_CRITICAL_EXTENSION functionality that is
> so weird that it requires patching the mbedtls config.h file ? Why is
> uacme absolutely requiring this functionality that no other user of
> mbedtls requires ?

Patching mbedtls is the way to configure it; we already do it to enable
threaads or use of zlib, or to disable asm.

That being said, I also have some concerns about this stuff.

For example, uacme is currently broken with mbedtls, so it was not
runtime tested.

Second, patch 2 does not fix the problem, which is only fixed with patch
3. Semantically, the last two patches should be reversed: first fix the
problem (use of X.509 extension), then add a feature (choice of
backend).

Third, uacme should not depend on the X.509 option, but should select
it.

Regards,
Yann E. MORIN.

> Until these questions are answered, I'd prefer to drop support for
> mbedtls as a crypto backend for uacme.
> 
> Best regards,
> 
> Thomas
> -- 
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/3] package/mbedtls: add BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION

[Thomas Petazzoni]

On Thu, 23 Apr 2020 22:27:26 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> Second, patch 2 does not fix the problem, which is only fixed with patch
> 3. Semantically, the last two patches should be reversed: first fix the
> problem (use of X.509 extension), then add a feature (choice of
> backend).

I thought the idea of patch 2 was that it would allow to select the
BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION option in patch
3, but indeed, it does not. This makes patch 2 and the whole series
even more awkward.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/3] package/mbedtls: add BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION

[Nicola Di Lieto]

On Thu, Apr 23, 2020 at 10:09:05PM +0200, Thomas Petazzoni wrote:
>
>What is this X509_UNSUPPORTED_CRITICAL_EXTENSION functionality that is
>so weird that it requires patching the mbedtls config.h file ? Why is
>uacme absolutely requiring this functionality that no other user of
>mbedtls requires ?
>

There is an explanation at
https://github.com/ndilieto/uacme/issues/23

Briefly, tls-alpn-01 validation requires (as per RFC8737 section 6.1) a 
new critical certificate extension. mbedTLS doesn't know about it and 
refuses to parse any certificate with such extension unless that build 
feature is enabled.

[Buildroot] [PATCH 1/3] package/mbedtls: add BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION

[Yann E. MORIN]

Nicola, Fabrice, Thomas, All,

On 2020-04-24 01:27 +0200, Nicola Di Lieto spake thusly:
> On Thu, Apr 23, 2020 at 10:09:05PM +0200, Thomas Petazzoni wrote:
> >What is this X509_UNSUPPORTED_CRITICAL_EXTENSION functionality that is
> >so weird that it requires patching the mbedtls config.h file ? Why is
> >uacme absolutely requiring this functionality that no other user of
> >mbedtls requires ?
> 
> There is an explanation at
> https://github.com/ndilieto/uacme/issues/23
> 
> Briefly, tls-alpn-01 validation requires (as per RFC8737 section 6.1) a new
> critical certificate extension. mbedTLS doesn't know about it and refuses to
> parse any certificate with such extension unless that build feature is
> enabled.

So, I think I now wrapped my head around this issue, and I think I got
it. Here's what I understood from the different resources [0] [1]:

  - in X.509, some extensions can be added to certificates
  - an extension can be marked as 'critical' or 'not critical'
  - an X.509 parser that encounters an extension marked 'critical' when
    parsing a certificate, and that does not recognise that extension,
    *must* reject that certificate.

mbedtls does the right thing here: it rejects such certificates.

However, embedtls has an option to treat thoe 'critical' extensions as
if they were 'not critical'.

I think we should refuse to use mbedtls with uacme.

[0] https://en.wikipedia.org/wiki/X.509
[1] https://github.com/ndilieto/uacme/issues/23

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/3] package/mbedtls: add BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION

[Nicola Di Lieto]

On Fri, Apr 24, 2020 at 11:07:10AM +0200, Yann E. MORIN wrote:
>  - an X.509 parser that encounters an extension marked 'critical' when
>    parsing a certificate, and that does not recognise that extension,
>    *must* reject that certificate.
>

I wouldn't be so sure that it must be done at parsing stage though.  
OpenSSL and GnuTLS parse the certificate without problems and then fail 
at validation stage.  OpenSSL even has a "-ignore_critical" command line 
switch to ignore critical extensions:

https://man.openbsd.org/openssl.1#ignore_critical

I honestly think the approach of mbedTLS to critical extensions is blunt 
to say the least.  And just as a side note, mbedTLS *will* still happily 
generate, sign and export a certificate with an unsupported critical 
extension, even when it's built without that damn feature.  Don't ask me 
how I know...  if you don't believe me, look at 
mbedtls_x509write_crt_set_extension, called on line 1167 in ualpn.c

>
>I think we should refuse to use mbedtls with uacme.

I agree, at least until mbedTLS changes its approach.

Regards
Nicola

[Buildroot] [PATCH 1/3] package/mbedtls: add BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION

[Nicola Di Lieto]

On Fri, Apr 24, 2020 at 01:26:39PM +0200, Nicola Di Lieto wrote:
>>
>>I think we should refuse to use mbedtls with uacme.
>
>I agree, at least until mbedTLS changes its approach.

Just to clarify. It's sufficient to disable building ualpn at configure 
stage with --without-ualpn. The main binary uacme is fine, even without 
unsupported critical extensions in mbedTLS.

[Buildroot] [PATCH 1/3] package/mbedtls: add BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION

[Yann E. MORIN]

Nicola, All,

On 2020-04-24 13:32 +0200, Nicola Di Lieto spake thusly:
> On Fri, Apr 24, 2020 at 01:26:39PM +0200, Nicola Di Lieto wrote:
> >>
> >>I think we should refuse to use mbedtls with uacme.
> >
> >I agree, at least until mbedTLS changes its approach.
> 
> Just to clarify. It's sufficient to disable building ualpn at configure
> stage with --without-ualpn. The main binary uacme is fine, even without
> unsupported critical extensions in mbedTLS.

Still, I think we should just entirely drop support for embedtls.

Users would be surprised if they have a different behaviour between
using openssl v.s gnutls v.s embedtls.

Then, when embedtls is updated to undersdand that critical extension, we
can re-instate support for embedtls in uacme.

Until then, my opinion is that we should just stop building uacme with
embedtls.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 1/3] package/mbedtls: add BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION

[Nicola Di Lieto]

On Fri, Apr 24, 2020 at 01:48:13PM +0200, Yann E. MORIN wrote:
>Until then, my opinion is that we should just stop building uacme with
>embedtls.

Please do not confuse the main binary uacme (which is perfecty OK and 
*does not* require the new TLS extension) with ualpn, a separate and 
optional binary recently added to the uacme distribution to answer 
tls-alpn-01 challenges (which requires the new TLS extension).

Building ualpn should actually be disabled by default in buildroot 
(check BR2_PACKAGE_UACME_UALPN in package/uacme/Config.in)

I propose making BR2_PACKAGE_UACME_UALPN depend on !BR2_PACKAGE_MBEDTLS.  
Would that be acceptable?

[Buildroot] [PATCH 1/3] package/mbedtls: add BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION

[Fabrice Fontaine]

Hi all,

Le ven. 24 avr. 2020 ? 15:11, Nicola Di Lieto
<nicola.dilieto@gmail.com> a ?crit :
>
> On Fri, Apr 24, 2020 at 01:48:13PM +0200, Yann E. MORIN wrote:
> >Until then, my opinion is that we should just stop building uacme with
> >embedtls.
>
> Please do not confuse the main binary uacme (which is perfecty OK and
> *does not* require the new TLS extension) with ualpn, a separate and
> optional binary recently added to the uacme distribution to answer
> tls-alpn-01 challenges (which requires the new TLS extension).
>
> Building ualpn should actually be disabled by default in buildroot
> (check BR2_PACKAGE_UACME_UALPN in package/uacme/Config.in)
>
> I propose making BR2_PACKAGE_UACME_UALPN depend on !BR2_PACKAGE_MBEDTLS.
> Would that be acceptable?
I think it should depend on !BR2_PACKAGE_UACME_MBEDTLS (added by the
second patch of this serie).
Otherwise, a user won't be able to select ualpn with a gnutls-enabled
uacme if he has also enabled mbedlts for an other purpose.
>
Best Regards,

Fabrice

[Buildroot] [PATCH 1/3] package/mbedtls: add BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION

[Thomas Petazzoni]

On Fri, 24 Apr 2020 15:20:19 +0200
Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:

> > I propose making BR2_PACKAGE_UACME_UALPN depend on !BR2_PACKAGE_MBEDTLS.
> > Would that be acceptable?  
> I think it should depend on !BR2_PACKAGE_UACME_MBEDTLS (added by the
> second patch of this serie).
> Otherwise, a user won't be able to select ualpn with a gnutls-enabled
> uacme if he has also enabled mbedlts for an other purpose.

Or instead make sure ualpn can be enabled only if either gnutls or
openssl are available, and tweak the .mk file to use gnutls or openssl
instead of mbedtls when ualpn is enabled.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/3] package/mbedtls: add BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION

[Fabrice Fontaine]

Hi all,

Le ven. 24 avr. 2020 ? 15:21, Thomas Petazzoni
<thomas.petazzoni@bootlin.com> a ?crit :
>
> On Fri, 24 Apr 2020 15:20:19 +0200
> Fabrice Fontaine <fontaine.fabrice@gmail.com> wrote:
>
> > > I propose making BR2_PACKAGE_UACME_UALPN depend on !BR2_PACKAGE_MBEDTLS.
> > > Would that be acceptable?
> > I think it should depend on !BR2_PACKAGE_UACME_MBEDTLS (added by the
> > second patch of this serie).
> > Otherwise, a user won't be able to select ualpn with a gnutls-enabled
> > uacme if he has also enabled mbedlts for an other purpose.
>
> Or instead make sure ualpn can be enabled only if either gnutls or
> openssl are available, and tweak the .mk file to use gnutls or openssl
> instead of mbedtls when ualpn is enabled.
OK, v2 sent.
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Best Regards,

Fabrice

[Buildroot] [PATCH 1/3] package/mbedtls: add BR2_PACKAGE_MBEDTLS_X509_UNSUPPORTED_CRITICAL_EXTENSION

[Yann E. MORIN]

On 2020-04-24 13:26 +0200, Nicola Di Lieto spake thusly:
> On Fri, Apr 24, 2020 at 11:07:10AM +0200, Yann E. MORIN wrote:
> > - an X.509 parser that encounters an extension marked 'critical' when
> >   parsing a certificate, and that does not recognise that extension,
> >   *must* reject that certificate.
> >
> 
> I wouldn't be so sure that it must be done at parsing stage though.

Well, "parsing" is maybe not the correct word, but as far as I
understand wikipedia on X.209 [0]:

    A certificate-using system must reject the certificate if it
    encounters a critical extension that it does not recognize,
    or a critical extension that contains information that it cannot
    process.

[0] https://en.wikipedia.org/wiki/X.509#Structure_of_a_certificate

So, as mbedtls does not know what to do with such a critical extension,
allowing it to just ignore it is a violation of the X.509 spec.

>  OpenSSL
> and GnuTLS parse the certificate without problems and then fail at
> validation stage.  OpenSSL even has a "-ignore_critical" command line switch
> to ignore critical extensions:
> 
> https://man.openbsd.org/openssl.1#ignore_critical

But that is different: this is a command line argument, which is
obviously not the default. Enabling X509_UNSUPPORTED_CRITICAL_EXTENSION
will make that the default behaviour, which is unsound.

Regards,
Yann E. MORIN.

> I honestly think the approach of mbedTLS to critical extensions is blunt to
> say the least.  And just as a side note, mbedTLS *will* still happily
> generate, sign and export a certificate with an unsupported critical
> extension, even when it's built without that damn feature.  Don't ask me how
> I know...  if you don't believe me, look at
> mbedtls_x509write_crt_set_extension, called on line 1167 in ualpn.c
> 
> >
> >I think we should refuse to use mbedtls with uacme.
> 
> I agree, at least until mbedTLS changes its approach.
> 
> Regards
> Nicola

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'