The signatureValue field of a X.509 certificate is encoded as a BIT STRING.
For RSA signatures this BIT STRING is of so-called primitive subtype, which
contains a u8 prefix indicating a count of unused bits in the encoding.
We have to strip this prefix from signature data, just as we already do for
key data in x509_extract_key_data() function.
This wasn't noticed earlier because this prefix byte is zero for RSA key
sizes divisible by 8. Since BIT STRING is a big-endian encoding adding zero
prefixes has no bearing on its value.
The signature length, however was incorrect, which is a problem for RSA
implementations that need it to be exactly correct (like AMD CCP).
Signed-off-by: Maciej S. Szmigiero <[email protected]>
Fixes: c26fd69fa009 ("X.509: Add a crypto key parser for binary (DER) X.509 certificates")
Cc: [email protected]
---
This is a resend of a patch that was previously submitted in one series
with CCP driver changes since this particular patch should go through
the security (rather than crypto) tree.
Changes from v1: Change '!' to '== 0'.
crypto/asymmetric_keys/x509_cert_parser.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/crypto/asymmetric_keys/x509_cert_parser.c b/crypto/asymmetric_keys/x509_cert_parser.c
index 7d81e6bb461a..b6cabac4b62b 100644
--- a/crypto/asymmetric_keys/x509_cert_parser.c
+++ b/crypto/asymmetric_keys/x509_cert_parser.c
@@ -249,6 +249,15 @@ int x509_note_signature(void *context, size_t hdrlen,
return -EINVAL;
}
+ if (strcmp(ctx->cert->sig->pkey_algo, "rsa") == 0) {
+ /* Discard the BIT STRING metadata */
+ if (vlen < 1 || *(const u8 *)value != 0)
+ return -EBADMSG;
+
+ value++;
+ vlen--;
+ }
+
ctx->cert->raw_sig = value;
ctx->cert->raw_sig_size = vlen;
return 0;
On 19.05.2018 14:23, Maciej S. Szmigiero wrote:
> The signatureValue field of a X.509 certificate is encoded as a BIT STRING.
> For RSA signatures this BIT STRING is of so-called primitive subtype, which
> contains a u8 prefix indicating a count of unused bits in the encoding.
>
> We have to strip this prefix from signature data, just as we already do for
> key data in x509_extract_key_data() function.
>
> This wasn't noticed earlier because this prefix byte is zero for RSA key
> sizes divisible by 8. Since BIT STRING is a big-endian encoding adding zero
> prefixes has no bearing on its value.
>
> The signature length, however was incorrect, which is a problem for RSA
> implementations that need it to be exactly correct (like AMD CCP).
Any progress here?
This simple patch has already been submitted 3 times in last 3+ months...
On 02.06.2018 21:12, Maciej S. Szmigiero wrote:
> On 19.05.2018 14:23, Maciej S. Szmigiero wrote:
>> The signatureValue field of a X.509 certificate is encoded as a BIT STRING.
>> For RSA signatures this BIT STRING is of so-called primitive subtype, which
>> contains a u8 prefix indicating a count of unused bits in the encoding.
>>
>> We have to strip this prefix from signature data, just as we already do for
>> key data in x509_extract_key_data() function.
>>
>> This wasn't noticed earlier because this prefix byte is zero for RSA key
>> sizes divisible by 8. Since BIT STRING is a big-endian encoding adding zero
>> prefixes has no bearing on its value.
>>
>> The signature length, however was incorrect, which is a problem for RSA
>> implementations that need it to be exactly correct (like AMD CCP).
>
> Any progress here?
> This simple patch has already been submitted 3 times in last 3+ months...
>
A friendly ping here.
@AMD people:
Without this patch, in-kernel X.509 certificate verification is broken
on AMD CCP RSA implementation.
For example, loading wireless regulatory database gives the following
errors:
> [ 21.310361] cfg80211: Problem loading in-kernel X.509 certificate (-22)
> [ 21.351717] cfg80211: loaded regulatory.db is malformed or signature is missing/invalid
Kernel modules signature verification probably has similar problem, too.
That's why it would be nice if you could ack this patch, please.
Maciej
On Wed, Jun 20, 2018 at 02:24:54PM +0200, Maciej S. Szmigiero wrote:
>
> A friendly ping here.
>
> @AMD people:
> Without this patch, in-kernel X.509 certificate verification is broken
> on AMD CCP RSA implementation.
>
> For example, loading wireless regulatory database gives the following
> errors:
> > [ 21.310361] cfg80211: Problem loading in-kernel X.509 certificate (-22)
> > [ 21.351717] cfg80211: loaded regulatory.db is malformed or signature is missing/invalid
>
> Kernel modules signature verification probably has similar problem, too.
>
> That's why it would be nice if you could ack this patch, please.
David/James, is there an issue with the patch?
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Wed, 20 Jun 2018, Herbert Xu wrote:
> On Wed, Jun 20, 2018 at 02:24:54PM +0200, Maciej S. Szmigiero wrote:
> >
> > A friendly ping here.
> >
> > @AMD people:
> > Without this patch, in-kernel X.509 certificate verification is broken
> > on AMD CCP RSA implementation.
> >
> > For example, loading wireless regulatory database gives the following
> > errors:
> > > [ 21.310361] cfg80211: Problem loading in-kernel X.509 certificate (-22)
> > > [ 21.351717] cfg80211: loaded regulatory.db is malformed or signature is missing/invalid
> >
> > Kernel modules signature verification probably has similar problem, too.
> >
> > That's why it would be nice if you could ack this patch, please.
>
> David/James, is there an issue with the patch?
Not from my POV.
--
James Morris
<[email protected]>
On Thu, Jun 21, 2018 at 11:44:29AM +1000, James Morris wrote:
> On Wed, 20 Jun 2018, Herbert Xu wrote:
>
> > On Wed, Jun 20, 2018 at 02:24:54PM +0200, Maciej S. Szmigiero wrote:
> > >
> > > A friendly ping here.
> > >
> > > @AMD people:
> > > Without this patch, in-kernel X.509 certificate verification is broken
> > > on AMD CCP RSA implementation.
> > >
> > > For example, loading wireless regulatory database gives the following
> > > errors:
> > > > [ 21.310361] cfg80211: Problem loading in-kernel X.509 certificate (-22)
> > > > [ 21.351717] cfg80211: loaded regulatory.db is malformed or signature is missing/invalid
> > >
> > > Kernel modules signature verification probably has similar problem, too.
> > >
> > > That's why it would be nice if you could ack this patch, please.
> >
> > David/James, is there an issue with the patch?
>
> Not from my POV.
Hi James:
I presume you will pick this up then?
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Thu, 21 Jun 2018, Herbert Xu wrote:
> Hi James:
>
> I presume you will pick this up then?
I will -- not sure why David hasn't merged it into his tree.
Can I add your acked or reviewed by?
--
James Morris
<[email protected]>