Shigeyuki Fukushima wrote:
>
> Hello.
>
> I'm a OpenSSL user.
> I found openssl's bug.
> I send a bug-fix patch.
> It is a fix for an infinte length SEQUENCE.
>
> infinite length SEQUENCE: 0x30 0x80 ...Contents... 0x00 0x00
>
> And also, I send a pkcs#7 data: cert.p7.
> Using openssl-0.9.6 on FreeBSD system,
>
> $ /somewhere/openssl pkcs7 -inform DER -in <this pkcs#7 data>
> 56451:error:0D091008:asn1 encoding routines:d2i_PKCS7:asn1 length
>mismatch:p7_lib.c:321:address=135442432 offset=19
>
> I think this pkcs#7 data is a valid data.
> But cannot parse it.
>
It is valid data but its more than a little unusual. The normal reason
for indefinite length encoding is that the length of the encoded data is
not known or single pass processing is necessary. In this case however
some of the inner SEQUENCEs are indefinite but the outer on is definite
length.
Its still a valid encoding though and OpenSSL should be able to parse
it.
The new ASN1 parser in OpenSSL 0.9.7 doesn't have any problems with that
structure.
> If this fix is right, would you like to merge it?
>
I haven't used the fix you suggested but something else instead. The
reason being that we curently have:
int asn1_GetSequence(ASN1_CTX *c, long *length)
{
unsigned char *q;
q=c->p;
c->inf=ASN1_get_object(&(c->p),&(c->slen),&(c->tag),&(c->xclass),
*length);
if (c->inf & 0x80)
{
c->error=ERR_R_BAD_GET_ASN1_OBJECT_CALL;
return(0);
}
if (c->tag != V_ASN1_SEQUENCE)
{
c->error=ERR_R_EXPECTING_AN_ASN1_SEQUENCE;
return(0);
}
----> (*length)-=(c->p-q);
if (c->max && (*length < 0))
{
c->error=ERR_R_ASN1_LENGTH_MISMATCH;
return(0);
}
if (c->inf == (1|V_ASN1_CONSTRUCTED))
------> c->slen= *length+ *(c->pp)-c->p;
c->eos=0;
return(1);
}
'*length' is the total amount of data available and c->slen should be
set to the amount of data available in the SEQUENCE.
At the first point indicated *length is reduced by the size of the
SEQUENCE tag+length bytes just read.
If indefinite length constructed encoding is used then we don't know the
precise length but can say that it cannot be larger than the updated
value of '*length'. Therefore I think the correct fix at the second
point is:
c->slen = *length;
Steve.
--
Dr Stephen N. Henson. http://www.drh-consultancy.demon.co.uk/
Personal Email: [EMAIL PROTECTED]
Senior crypto engineer, Celo Communications: http://www.celocom.com/
Core developer of the OpenSSL project: http://www.openssl.org/
Business Email: [EMAIL PROTECTED] PGP key: via homepage.
______________________________________________________________________
OpenSSL Project http://www.openssl.org
Development Mailing List [EMAIL PROTECTED]
Automated List Manager [EMAIL PROTECTED]