crypt32: Re: exception handling, part 2

[Saulius Krasuckas]

* Log message from [1]:
> Juan Lang
> - fix decoding of long-form data lengths
> - use exception handling in decoding
> - tests for these changes

One of added tests in the patch fails on winME:

| encode.c:313: Test failed: Expected CRYPT_E_ASN1_LARGE, got c0000005

But the bad thing is that after running small code chunk another calls to 
CryptEncodeObjectEx() starts generating exceptions here.  Mentioned code:

+    /* Try to decode some bogus large items */
+    /* The buffer size is smaller than the encoded length, so this should fail
+     * with CRYPT_E_ASN1_EOD if it's being decoded.  It's failing with
+     * CRYPT_E_ASN1_LARGE, meaning there's a limit on the size decoded.
+     * The magic limit under XP seems to be 0x061a8000 bytes--more than this
+     * fails with CRYPT_E_ASN1_LARGE.
+     */
+    ret = CryptDecodeObjectEx(dwEncoding, X509_MULTI_BYTE_INTEGER, tooBig,
+     0x7fffffff, CRYPT_DECODE_ALLOC_FLAG, NULL, (BYTE *)&buf, &bufSize);
+    ok(!ret && GetLastError() == CRYPT_E_ASN1_LARGE,
+     "Expected CRYPT_E_ASN1_LARGE, got %08lx\n", GetLastError());

And example of parameters for the mentioned call (which after throws an 
exception of "Access violation" in another tests):

CryptEncodeObjectEx(
  X509_ASN_ENCODING, 
  X509_ENUMERATED, 
  1, 
  CRYPT_ENCODE_ALLOC_FLAG, 
  NULL, 
  (BYTE *)&buf, 
  &bufSize
);

HTH to fix exceptions.  If that informations is not enough, I will be glad 
to debug this via e-mail, Juan. :-)

[1] http://www.winehq.org/hypermail/wine-cvs/2005/06/0683.html

[Juan Lang]

Hi Saulius,
> But the bad thing is that after running small code chunk another calls
> to CryptEncodeObjectEx() starts generating exceptions here.

Hm, that's what I was afraid of, that the error handling on Win9x wasn't
as good as on XP.  I'll probably just have to remove those tests, as I was
trying to cause exceptions, and was showing that in WinXP at least the
exceptions are caught.

Thanks for pointing that out,
--Juan

____________________________________________________
Start your day with Yahoo! - make it your home page 
http://www.yahoo.com/r/hs

[Saulius Krasuckas]

* On Fri, 22 Jul 2005, Juan Lang wrote:
> > But the bad thing is that after running small code chunk another calls 
> > to CryptEncodeObjectEx() starts generating exceptions here.
>
> the error handling on Win9x wasn't as good as on XP.

Hmm.  I don't understand the code, but if I change (+) BYTE tooBig 
definition, then exceptions are gone, but return codes still differs in 
the output (|):

+ tooBig[] = { 0x00, 0x84, 0xff, 0xff, 0xff, 0xfc };
| encode.c:313: Test failed: Expected CRYPT_E_ASN1_LARGE, got 8009310b

+ tooBig[] = { 0x02, 0x84, 0xf0, 0xff, 0xff, 0xfc };
+ tooBig[] = { 0x02, 0x84, 0xff, 0x0f, 0xff, 0xfc };
| encode.c:313: Test failed: Expected CRYPT_E_ASN1_LARGE, got 80093102

+ tooBig[] = { 0x02, 0x04, 0xff, 0xff, 0xff, 0xfc };
| encode.c:313: Test failed: Expected CRYPT_E_ASN1_LARGE, got 00000000

+ tooBig[] = { 0x02, 0x80, 0xff, 0xff, 0xff, 0xfc };
| encode.c:313: Test failed: Expected CRYPT_E_ASN1_LARGE, got 80093103

+ tooBig[] = { 0x02, 0x83, 0xff, 0xff, 0xff, 0xfc };
| encode.c:313: Test failed: Expected CRYPT_E_ASN1_LARGE, got c0000005

+ tooBig[] = { 0x02, 0x84, 0x0f, 0xff, 0xff, 0xfc };
+ tooBig[] = { 0x02, 0x84, 0xff, 0xff, 0xff, 0xfc };
| encode.c:313: Test failed: Expected CRYPT_E_ASN1_LARGE, got 80093106

+ tooBig[] = { 0x02, 0x84, 0xff, 0xff, 0xff, 0xfd };
| encode.c:313: Test failed: Expected CRYPT_E_ASN1_LARGE, got c0000005 
| (plus further exceptions)

For me it is strange that "{0x02, 0x83, ..." gives same GLE (c0000005) as 
original tooBig value, but exceptions aren't being catched in debugger.  
I thought if there may be some sense in tuning tooBig value, then could it 
be done?  If not, then sorry for the noise.

[Juan Lang]

--- Saulius Krasuckas  wrote:
> Hmm.  I don't understand the code, but if I change (+) BYTE tooBig 
> definition, then exceptions are gone, but return codes still differs in 
> the output (|):

(snip)
> I thought if there may be some sense in tuning tooBig value, then could
> it be done?

You may be right.  I'll explain what some of these errors mean:

In ASN.1 encoding, the first byte is the tag, or type of the value that
follows.  0x02 is an integer.  The second byte, if the high bit is not
set, is the number of bytes of the value.  So for example,

0x02 0x01 0x01

is the integer value 1.

If the high bit of the length byte is set, then the value of the length
byte & 0x7f is the number of length bytes that follow.  So, in long form:

0x02 0x81 0x01 0x01

is also the integer value 1.  0x81 & 0x7f is 0x01, meaning one length byte
follows.  The length value is 0x01, meaning one integer byte follows.

So the tooBig test does the following:
0x02 0x84 0xff 0xff 0xff 0xff

This means it's an integer, with 4 length bytes, whose value are
0xffffffff.  That means 0xffffffff bytes of integer value should follow
(which obviously don't.)  In WinXP, there's apparently a sanity check on
this to say this is too big a value, without trying to read this many
bytes.  That's what the test is trying to show:  CRYPT_E_ASN1_LARGE is
returned for an "insane" value.
> - tooBig[] = { 0x00, 0x84, 0xff, 0xff, 0xff, 0xfc };
>
> | encode.c:313: Test failed: Expected CRYPT_E_ASN1_LARGE, got 8009310b

This is CRYPT_E_ASN1_BADTAG, which is true because the first byte is not a
valid tag.
> - tooBig[] = { 0x02, 0x84, 0xf0, 0xff, 0xff, 0xfc };
> - tooBig[] = { 0x02, 0x84, 0xff, 0x0f, 0xff, 0xfc };
>
> | encode.c:313: Test failed: Expected CRYPT_E_ASN1_LARGE, got 80093102

This is CRYPT_E_ASN1_EOD, "end of data."  That could be useful.
> - tooBig[] = { 0x02, 0x04, 0xff, 0xff, 0xff, 0xfc };
>
> | encode.c:313: Test failed: Expected CRYPT_E_ASN1_LARGE, got 00000000

This is a valid integer, 0xfffffffc, and there's already a test case for
these.
> - tooBig[] = { 0x02, 0x80, 0xff, 0xff, 0xff, 0xfc };
>
> | encode.c:313: Test failed: Expected CRYPT_E_ASN1_LARGE, got 80093103

This is CRYPT_E_ASN1_CORRUPT, the length (0 bytes) is invalid.
> - tooBig[] = { 0x02, 0x83, 0xff, 0xff, 0xff, 0xfc };
>
> | encode.c:313: Test failed: Expected CRYPT_E_ASN1_LARGE, got c0000005

That's interesting too.  0x00ffffff bytes are supposed to follow, but an
access violation is caught.  So the WinME version catches some exceptions,
but not all?
> - tooBig[] = { 0x02, 0x84, 0x0f, 0xff, 0xff, 0xfc };
> - tooBig[] = { 0x02, 0x84, 0xff, 0xff, 0xff, 0xfc };
>
> | encode.c:313: Test failed: Expected CRYPT_E_ASN1_LARGE, got 80093106

That's CRYPT_E_ASN1_MEMORY, meaning out of memory.  Weird, it must try to
allocate this much?

Anyway, the WinME implementation is kind of strange buggy.  Thanks for
playing with it.  I think accepting both CRYPT_E_ASN1_LARGE and
CRYPT_E_ASN1_EOD, along with a value that results in CRYPT_E_ASN1_EOD,
would be a useful fix.  Do you want to patch it?

--Juan

__________________________________________________
Do You Yahoo!?
Tired of spam?  Yahoo! Mail has the best spam protection around 
http://mail.yahoo.com

[Saulius Krasuckas]

Thank you for taking your time to explain all that changes, Juan.

* On Fri, 22 Jul 2005, Juan Lang wrote:
> That's interesting too.  0x00ffffff bytes are supposed to follow, but an 
> access violation is caught.  So the WinME version catches some 
> exceptions, but not all?

I wouldn't be surprised much.  WinME was rather strange step by M$, IMHO.
> > | encode.c:313: Test failed: Expected CRYPT_E_ASN1_LARGE, got 80093106
>
> That's CRYPT_E_ASN1_MEMORY, meaning out of memory.  Weird, it must try to
> allocate this much?

Sorry, I have no opinion of this case.  May my box be really out of mem?
> I think accepting both CRYPT_E_ASN1_LARGE and CRYPT_E_ASN1_EOD, along 
> with a value that results in CRYPT_E_ASN1_EOD, would be a useful fix.  
> Do you want to patch it?

Hm, nah.  It's not my corner of coding (at least now), plus I just have 
came back from an unurbanized place surrounded by a forest, where I have 
done some physical job.  So now I feel somewhat unITized. =)

If you'll send some patch, I would be glad to see it (and to test it upon 
a request).

[Kuba Ober]

> > > - tooBig[] = { 0x02, 0x84, 0x0f, 0xff, 0xff, 0xfc };
> > > - tooBig[] = { 0x02, 0x84, 0xff, 0xff, 0xff, 0xfc };
> > >
> > > | encode.c:313: Test failed: Expected CRYPT_E_ASN1_LARGE, got 80093106
> >
> > That's CRYPT_E_ASN1_MEMORY, meaning out of memory.  Weird, it must try to
> > allocate this much?
>
> Sorry, I have no opinion of this case.  May my box be really out of mem?

Since the test tried to push it either 0x0ffffffc (268MiB) or 0xfffffffc 
(4.29GiB), no wonder it's out of memory. Note that you stripped out important 
context in your quoting, which I duly restored :)

Cheers, Kuba

[Juan Lang]

Oops, let me take back some of my advice:
> I think accepting both CRYPT_E_ASN1_LARGE and CRYPT_E_ASN1_EOD,
> along with a value that results in CRYPT_E_ASN1_EOD, would be a
> useful fix.

I should read my own comments.  If it returns CRYPT_E_ASN1_EOD on WinME,
that means the test is just wrong on WinME--as the comment says, it's
trying to detect a case where CRYPT_E_ASN1_EOD isn't returned.  So the
test should just be removed.

--Juan

____________________________________________________
Start your day with Yahoo! - make it your home page 
http://www.yahoo.com/r/hs