[libcamera-devel] libcamera: pub_key: Support GNUTLS < v3
diff mbox series

Message ID 20201109105159.981412-1-kieran.bingham@ideasonboard.com
State Rejected
Delegated to: Kieran Bingham
Headers show
Series
  • [libcamera-devel] libcamera: pub_key: Support GNUTLS < v3
Related show

Commit Message

Kieran Bingham Nov. 9, 2020, 10:51 a.m. UTC 
It has been reported that SailfishOS is packaged with an older GnuTLS
library. Supporting GnuTLS < 3 appears to be trivial, but comes at the
cost of using a #define to switch.

Use a #define block to support older GnuTLS installations.

Reported-by: Simon Schmeisser <mail_to_wrt@gmx.de>
Suggested-by: Matti Lehtimaki <matti.lehtimaki@gmail.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
 src/libcamera/pub_key.cpp | 7 +++++++
 1 file changed, 7 insertions(+)

This was reported at [0], with a fix proposed at [1]

[0] https://github.com/sailfish-on-dontbeevil/droid-config-pinephone/issues/55
[1] https://git.sailfishos.org/mal/libcamera/blob/master/rpm/gnutls2.patch

Alternatively we could just /require/ GnuTLS >= 3... but this seems
fairly cheap.

This patch is a simplified version of [1] (No need to check if we're
__cplusplus, in a cpp file, but I haven't seen that this is needed
either).

I'd like to see Tested-by: tags on this before integration, as I have no
way to verify it.

--
Kieran

Comments

Laurent Pinchart Nov. 9, 2020, 11:36 a.m. UTC | #1 
Hi Kieran,

Thank you for the patch.

On Mon, Nov 09, 2020 at 10:51:59AM +0000, Kieran Bingham wrote:
> It has been reported that SailfishOS is packaged with an older GnuTLS
> library. Supporting GnuTLS < 3 appears to be trivial, but comes at the
> cost of using a #define to switch.
> 
> Use a #define block to support older GnuTLS installations.
> 
> Reported-by: Simon Schmeisser <mail_to_wrt@gmx.de>
> Suggested-by: Matti Lehtimaki <matti.lehtimaki@gmail.com>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> ---
>  src/libcamera/pub_key.cpp | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> This was reported at [0], with a fix proposed at [1]
> 
> [0] https://github.com/sailfish-on-dontbeevil/droid-config-pinephone/issues/55
> [1] https://git.sailfishos.org/mal/libcamera/blob/master/rpm/gnutls2.patch
> 
> Alternatively we could just /require/ GnuTLS >= 3... but this seems
> fairly cheap.

It is. However, the latest version of gnutls 2.x appears to be 2.12.24,
released 4 years ago. For a crypto library, this seems a very big
security risk. Do we want to enable that ?

On a side note, I've been considering moving from gnutls to libnettle,
which is the library that gnutls uses as its crypto backend. -ENOTIME so
far.

> This patch is a simplified version of [1] (No need to check if we're
> __cplusplus, in a cpp file, but I haven't seen that this is needed
> either).
> 
> I'd like to see Tested-by: tags on this before integration, as I have no
> way to verify it.
> 
> diff --git a/src/libcamera/pub_key.cpp b/src/libcamera/pub_key.cpp
> index 9bb08fda34af..857c395373ea 100644
> --- a/src/libcamera/pub_key.cpp
> +++ b/src/libcamera/pub_key.cpp
> @@ -8,7 +8,9 @@
>  #include "libcamera/internal/pub_key.h"
>  
>  #if HAVE_GNUTLS
> +extern "C" {
>  #include <gnutls/abstract.h>
> +}
>  #endif
>  
>  /**
> @@ -87,8 +89,13 @@ bool PubKey::verify([[maybe_unused]] Span<const uint8_t> data,
>  		static_cast<unsigned int>(sig.size())
>  	};
>  
> +#if GNUTLS_VERSION_MAJOR >= 3
>  	int ret = gnutls_pubkey_verify_data2(pubkey_, GNUTLS_SIGN_RSA_SHA256, 0,
>  					     &gnuTlsData, &gnuTlsSig);
> +#else
> +	int ret = gnutls_pubkey_verify_data(pubkey_, 0, &gnuTlsData, &gnuTlsSig);
> +#endif
> +
>  	return ret >= 0;
>  #else
>  	return false;
Kieran Bingham Nov. 9, 2020, 11:46 a.m. UTC | #2 
Hi Laurent,

On 09/11/2020 11:36, Laurent Pinchart wrote:
> Hi Kieran,
> 
> Thank you for the patch.
> 
> On Mon, Nov 09, 2020 at 10:51:59AM +0000, Kieran Bingham wrote:
>> It has been reported that SailfishOS is packaged with an older GnuTLS
>> library. Supporting GnuTLS < 3 appears to be trivial, but comes at the
>> cost of using a #define to switch.
>>
>> Use a #define block to support older GnuTLS installations.
>>
>> Reported-by: Simon Schmeisser <mail_to_wrt@gmx.de>
>> Suggested-by: Matti Lehtimaki <matti.lehtimaki@gmail.com>
>> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>> ---
>>  src/libcamera/pub_key.cpp | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> This was reported at [0], with a fix proposed at [1]
>>
>> [0] https://github.com/sailfish-on-dontbeevil/droid-config-pinephone/issues/55
>> [1] https://git.sailfishos.org/mal/libcamera/blob/master/rpm/gnutls2.patch
>>
>> Alternatively we could just /require/ GnuTLS >= 3... but this seems
>> fairly cheap.
> 
> It is. However, the latest version of gnutls 2.x appears to be 2.12.24,
> released 4 years ago. For a crypto library, this seems a very big
> security risk. Do we want to enable that ?

Indeed, and overall for the project reporting the issue packaging such
an older version might be a concern.

I'd be tempted to /require/ newer version ... and ignore this patch.

> On a side note, I've been considering moving from gnutls to libnettle,
> which is the library that gnutls uses as its crypto backend. -ENOTIME so
> far.

That might also clean things up anyway.

Lets add that (libnettle support) to the list of potential newcomer
tasks and not worry about it.
--
Kieran



>> This patch is a simplified version of [1] (No need to check if we're
>> __cplusplus, in a cpp file, but I haven't seen that this is needed
>> either).
>>
>> I'd like to see Tested-by: tags on this before integration, as I have no
>> way to verify it.
>>
>> diff --git a/src/libcamera/pub_key.cpp b/src/libcamera/pub_key.cpp
>> index 9bb08fda34af..857c395373ea 100644
>> --- a/src/libcamera/pub_key.cpp
>> +++ b/src/libcamera/pub_key.cpp
>> @@ -8,7 +8,9 @@
>>  #include "libcamera/internal/pub_key.h"
>>  
>>  #if HAVE_GNUTLS
>> +extern "C" {
>>  #include <gnutls/abstract.h>
>> +}
>>  #endif
>>  
>>  /**
>> @@ -87,8 +89,13 @@ bool PubKey::verify([[maybe_unused]] Span<const uint8_t> data,
>>  		static_cast<unsigned int>(sig.size())
>>  	};
>>  
>> +#if GNUTLS_VERSION_MAJOR >= 3
>>  	int ret = gnutls_pubkey_verify_data2(pubkey_, GNUTLS_SIGN_RSA_SHA256, 0,
>>  					     &gnuTlsData, &gnuTlsSig);
>> +#else
>> +	int ret = gnutls_pubkey_verify_data(pubkey_, 0, &gnuTlsData, &gnuTlsSig);
>> +#endif
>> +
>>  	return ret >= 0;
>>  #else
>>  	return false;
>

Patch
diff mbox series 

diff --git a/src/libcamera/pub_key.cpp b/src/libcamera/pub_key.cpp
index 9bb08fda34af..857c395373ea 100644
--- a/src/libcamera/pub_key.cpp
+++ b/src/libcamera/pub_key.cpp
@@ -8,7 +8,9 @@ 
 #include "libcamera/internal/pub_key.h"
 
 #if HAVE_GNUTLS
+extern "C" {
 #include <gnutls/abstract.h>
+}
 #endif
 
 /**
@@ -87,8 +89,13 @@  bool PubKey::verify([[maybe_unused]] Span<const uint8_t> data,
 		static_cast<unsigned int>(sig.size())
 	};
 
+#if GNUTLS_VERSION_MAJOR >= 3
 	int ret = gnutls_pubkey_verify_data2(pubkey_, GNUTLS_SIGN_RSA_SHA256, 0,
 					     &gnuTlsData, &gnuTlsSig);
+#else
+	int ret = gnutls_pubkey_verify_data(pubkey_, 0, &gnuTlsData, &gnuTlsSig);
+#endif
+
 	return ret >= 0;
 #else
 	return false;