Note: There are a few cases of duplicates in user autocompletion which are being worked on.

CERT_AsciiToName() does not handle OIDs in dotted decimal form

RESOLVED FIXED in 3.31

Status

NSS
Libraries
RESOLVED FIXED
5 months ago
2 months ago

People

(Reporter: Miklos Vajna, Assigned: jcj)

Tracking

(Blocks: 2 bugs)

3.28.1
3.31
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 months ago
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:51.0) Gecko/20100101 Firefox/51.0
Build ID: 20170126000000

Steps to reproduce:

Compile this source code:

----
#include <iostream>

#include <cert.h>

void test(const char* string)
{
    CERTName* name = CERT_AsciiToName(string);
    if (name)
    {
        std::cerr << "CERT_AsciiToName() succeeded for '" << string << "'" << std::endl;
        CERT_DestroyName(name);
    }
    else
        std::cerr << "CERT_AsciiToName() failed for '" << string << "'" << std::endl;
}

int main()
{
    test("C=HU,L=Budapest,O=NISZ Nemzeti Infokommunikációs Szolgáltató Zrt.,CN=Állampolgári Tanúsítványkiadó - Qualified Citizen CA,2.5.4.97=VATHU-10585560");

    return 0;
}
----

and run it.


Actual results:

"CERT_AsciiToName() failed for" gets printed.


Expected results:

"CERT_AsciiToName() succeeded for" gets printed.

The problem seems to be the ,2.5.4.97=... part.

See the mailing list archive that confirms this is a bug:

http://www.mail-archive.com/dev-tech-crypto@lists.mozilla.org/msg12789.html
(Reporter)

Comment 1

5 months ago
Created attachment 8841104 [details] [diff] [review]
Proposed patch

The attached patch solves the problem for me. Although this is my first patch to NSS, so it's possible this is not the correct fix.

Could someone please review it? I'm happy to look into writing a testcase for this in case the patch itself is OK.

Thanks!
(Reporter)

Comment 2

5 months ago
Hmm, the patch in itself is not correct:

        SECItem* derName = SEC_ASN1EncodeItem(arena, nullptr, name, 
                                              SEC_ASN1_GET(CERT_NameTemplate));

Results in an ASN1 blob that's not actually valid.

Comment 3

4 months ago
Issue confirmed on https://groups.google.com/forum/?_escaped_fragment_=topic/mozilla.dev.tech.crypto/sBJ7R_V9ldE#!topic/mozilla.dev.tech.crypto/sBJ7R_V9ldE

Background: ECDSA support in xmlsec-nss, bundled by LibreOffice: https://vmiklos.hu/blog/xmlsec-nss-ecdsa.html
Thanks for the patch. Could you provide a test for the fix to make sure this is actually working?
Flags: needinfo?(vmiklos)
(Reporter)

Comment 5

4 months ago
The initial description above has sample code gives "succeeded for" with my patch and "failed for" without the patch. However, as mentioned in comment 2, probably this is only part of a larger problem: someone with more NSS knowledge would have to fix up other parts of NSS, so that when certutils gets a string where one of the attribute types are specified in dotted-decimal form it doesn't fail.

So in short probably just ignore my patch, but the problem is still there.
Flags: needinfo?(vmiklos)
(Assignee)

Updated

2 months ago
Blocks: 1363416
(Assignee)

Updated

2 months ago
Blocks: 1363928
(Assignee)

Updated

2 months ago
Assignee: nobody → jjones
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
https://hg.mozilla.org/projects/nss/rev/47eb8450e8ea

Updated

2 months ago
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → 3.31
You need to log in before you can comment on or make changes to this bug.