forge icon indicating copy to clipboard operation
forge copied to clipboard

Support for UTF-8 chars in subject

Open kamil7x opened this issue 5 years ago • 10 comments

Fixes https://github.com/digitalbazaar/forge/issues/397 Based on https://github.com/digitalbazaar/forge/issues/397#issuecomment-218420565

kamil7x avatar Nov 05 '19 14:11 kamil7x

Why not merge?

LuanMaik avatar Feb 04 '21 17:02 LuanMaik

@LuanMaik This fell off the radar. It's a challenge to determine if this is the correct fix. Ideally this would refer to the proper part of a spec that defines the behavior, and there would be some tests added. Data in the wild may or may not follow those specs, so that adds more complexity.

Does anyone have time to add test cases for this?

davidlehn avatar Feb 04 '21 18:02 davidlehn

This is a relevant case to be observed https://github.com/vbuch/node-signpdf/issues/111

LuanMaik avatar Feb 04 '21 18:02 LuanMaik

Hello,

I did a test with two V3 certifcates in the context of pdf signature

  • first one with special chars CN = AC Representación
  • second one with CN = AC Representacion (without special chars)

Without the fix the first one fail The code called in node-signpdf is https://github.com/vbuch/node-signpdf/blob/develop/src/signpdf.js#L80

    const forgeCert = _nodeForge.default.util.createBuffer(p12Buffer.toString('binary'));
    const p12Asn1 = _nodeForge.default.asn1.fromDer(forgeCert);
    const p12 = _nodeForge.default.pkcs12.pkcs12FromAsn1(p12Asn1, options.asn1StrictParsing, options.passphrase);

For me this PR fix my problem

bopos avatar Feb 05 '21 09:02 bopos

Hi @davidlehn, could this PR be merged? Or its missing something? The related issue #397 its marked as resolved, but the PR its not merged, there is something we can do to merge it?

At my company we are having the same problem as vbuch/node-signpdf#111 and we ended here, for now we are working with a local fork, but we would like to know if we can do anything to help moving this forward, thanks.

SrZorro avatar May 06 '21 08:05 SrZorro

What do you think to change this:

if(obj.valueTagClass === asn1.Type.UTF8) {
     obj.value = forge.util.decodeUtf8(obj.value);
} 

to this:

if(obj.valueTagClass === asn1.Type.UTF8) {
   try {
       obj.value = forge.util.decodeUtf8(obj.value);
   } catch(e) { 
      /* This exception means obj.value is utf-8 */ 
   }
} 

to avoid unhandled exceptions, when the obj.value is utf-8 already.

Check: https://stackoverflow.com/questions/36314943/check-if-javascript-string-is-valid-utf-8

LuanMaik avatar May 07 '21 03:05 LuanMaik

I need this fix, why not merge?

ArturoVicencioParada avatar Jun 30 '21 16:06 ArturoVicencioParada

@Arturokin It's necessary to add test cases for this to determine if this is the correct fix.

LuanMaik avatar Jun 30 '21 17:06 LuanMaik

I am not using forge anymore. Maybe someone could jump in and fix this?

kamil7x avatar Jan 14 '22 11:01 kamil7x