SEC_ASN1_BIT_STRING's shift when using nsslowkey_RSAPublicKeyTemplate

[Andrew Cagney]

Hi,

I'm reading this code:

NSSLOWKEYSubjectPublicKeyInfo spki;
NSSLOWKEYPublicKey pubk;
SECItem *publicKeyInfo;

if (SEC_ASN1EncodeItem(arena, &spki.subjectPublicKey,
&pubk, nsslowkey_RSAPublicKeyTemplate) == NULL) {
crv = CKR_HOST_MEMORY;
goto loser;
}

publicKeyInfo = SEC_ASN1EncodeItem(arena, NULL,
&spki,
nsslowkey_SubjectPublicKeyInfoTemplate);
if (!publicKeyInfo) {
crv = CKR_HOST_MEMORY;
goto loser;
}

in nss/lib/softoken/pkcs11c.c:sftk_unwrapPrivateKey()

In my version of this code I'm finding that I need to convert the
returned subjectPublicKey's length to bits, something like:

spki.subjectPublicKey *= 8;

before making the second SEC_ASN1EncodeItem() call. I believe this is
because that field is encoded using:

{ SEC_ASN1_BIT_STRING,
offsetof(NSSLOWKEYSubjectPublicKeyInfo, subjectPublicKey) },

and SEC_ASN1_BIT_STRING expects the SECItem.len to be in bits not bytes.

could the above have the same problem?

Andrew

[Robert Relyea]

On 6/30/22 12:06 PM, Andrew Cagney wrote:
> Hi,
>
> I'm reading this code:
>
> NSSLOWKEYSubjectPublicKeyInfo spki;
> NSSLOWKEYPublicKey pubk;
> SECItem *publicKeyInfo;
>
> if (SEC_ASN1EncodeItem(arena, &spki.subjectPublicKey,
> &pubk, nsslowkey_RSAPublicKeyTemplate) == NULL) {
> crv = CKR_HOST_MEMORY;
> goto loser;
> }
>
> publicKeyInfo = SEC_ASN1EncodeItem(arena, NULL,
> &spki,
> nsslowkey_SubjectPublicKeyInfoTemplate);
> if (!publicKeyInfo) {
> crv = CKR_HOST_MEMORY;
> goto loser;
> }
>
> in nss/lib/softoken/pkcs11c.c:sftk_unwrapPrivateKey()

I little bit more context would have help, but I finally found it.

It looks like it's happening for PSS keys where it's trying to create
CKA_PUBLIC_KEY_INFO. I don't see anything in NSS proper which references
CKA_PUBLIC_KEY_INFO, so it quite likely could be wrong and we haven't
notices.

> In my version of this code I'm finding that I need to convert the
> returned subjectPublicKey's length to bits, something like:
>
> spki.subjectPublicKey *= 8;
>
> before making the second SEC_ASN1EncodeItem() call. I believe this is
> because that field is encoded using:
>
> { SEC_ASN1_BIT_STRING,
> offsetof(NSSLOWKEYSubjectPublicKeyInfo, subjectPublicKey) },
>
> and SEC_ASN1_BIT_STRING expects the SECItem.len to be in bits not bytes.
>
> could the above have the same problem?

I think you are right, it could have the same problem.

bob

>
> Andrew
>

[Andrew Cagney]

On Thu, 30 Jun 2022 at 17:07, Robert Relyea <rre...@redhat.com> wrote:

> > in nss/lib/softoken/pkcs11c.c:sftk_unwrapPrivateKey()
>
>
> I little bit more context would have help, but I finally found it.

> It looks like it's happening for PSS keys where it's trying to create
> CKA_PUBLIC_KEY_INFO. I don't see anything in NSS proper which references
> CKA_PUBLIC_KEY_INFO, so it quite likely could be wrong and we haven't
> notices.

Ah. Unfortunately I had no real context to give.
I came across this code while searching for an example of how to use
the ASN.1 parser to encode an RSA public key. It was essentially what
I wanted except my output was off when fed into any ASN.1 parser.

(this answers my question about PEM encoded public keys)

> > In my version of this code I'm finding that I need to convert the
> > returned subjectPublicKey's length to bits, something like:
> >
> > spki.subjectPublicKey *= 8;
> >
> > before making the second SEC_ASN1EncodeItem() call. I believe this is
> > because that field is encoded using:
> >
> > { SEC_ASN1_BIT_STRING,
> > offsetof(NSSLOWKEYSubjectPublicKeyInfo, subjectPublicKey) },
> >
> > and SEC_ASN1_BIT_STRING expects the SECItem.len to be in bits not bytes.
> >
> > could the above have the same problem?
>
> I think you are right, it could have the same problem.

Thanks.

> bob
>
> >
> > Andrew

[Marcin Cieslak]

I was tracing once the ASN.1 code and I found it extremely confiusing that
SECItem.len is sometimes in bytes, sometime in bits.

If I am not mistaken, functions used to copy such an SECItem will copy 8 times
more junk, since BITSTRING data are stored as 8 bits per byte, anyway.

I have also noticed that both ASN.1 parsers in the code behave the same way,
beefing up SECItem.len times 8.

This must be intentional. (Because we do not have any means to store the exact
number of bits anywhere?) Should/Shouldn't generic functions be aware of this distiction?

Marcin