SEC_ASN1EncodeInteger cannot correctly encode integers in these ranges: 0x00000080-0x000000ff 0x00008000-0x0000ffff 0x00800000-0x00ffffff It incorrectly concludes that those numbers are negative numbers. It does not ensure that a zero byte is placed in front of them in the encoding to keep them positive.
Created attachment 451476 [details] [diff] [review] Patch v1 for NSS trunk This seems to fix the problem, but I want to make sure it causes no regressions before requesting review.
Created attachment 451498 [details] [diff] [review] Patch v3 for NSS trunk (with .def file)
Another (minor?) problem with SEC_ASN1EncodeInteger is that it doesn't encode negative values in the minimum number of octets. For example, -129 should be encoded as the following value octets: FF 7F But SEC_ASN1EncodeInteger encodes -129 as the following value octects on a 32-bit system: FF FF FF 7F Similarly, -128 should be 80 but is encoded as FF FF FF 80
Created attachment 503244 [details] [diff] [review] Patch v4 Fix the incorrect encoding of a nonnegative value. Fix a confusing comment that seems to imply there is an *unsigned* DER encoding of INTEGER. The 'make_unsigned' parameter is renamed 'is_unsigned' to suggest that it means the 'value' argument is truly an unsigned type. (Perhaps 'value_is_unsigned' or even 'value_is_signed', with the opposite sense, is a better name.) I also added a test program.
Comment on attachment 503244 [details] [diff] [review] Patch v4 I like this MUCH better than my previous patch! Thanks.
Patch checked in on the NSS trunk (NSS 3.13). Checking in lib/util/secasn1e.c; /cvsroot/mozilla/security/nss/lib/util/secasn1e.c,v <-- secasn1e.c new revision: 1.22; previous revision: 1.21 done RCS file: /cvsroot/mozilla/security/nss/cmd/tests/encodeinttest.c,v done Checking in cmd/tests/encodeinttest.c; /cvsroot/mozilla/security/nss/cmd/tests/encodeinttest.c,v <-- encodeinttest.c initial revision: 1.1 done Checking in cmd/tests/manifest.mn; /cvsroot/mozilla/security/nss/cmd/tests/manifest.mn,v <-- manifest.mn new revision: 1.10; previous revision: 1.9 done The bug fix (but not the test program) checked in on the NSS_3_12_BRANCH (3.12.10). Checking in secasn1e.c; /cvsroot/mozilla/security/nss/lib/util/secasn1e.c,v <-- secasn1e.c new revision: 184.108.40.206; previous revision: 1.21 done Filed bug 625239 for the problem I described in comment 5.