Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PKCS#7 signed data and custom authenticatedAttributes / OIDs #400

Open
NuSkooler opened this issue May 19, 2016 · 13 comments
Open

PKCS#7 signed data and custom authenticatedAttributes / OIDs #400

NuSkooler opened this issue May 19, 2016 · 13 comments

Comments

@NuSkooler
Copy link

Is there a way to add custom (e.g. not known to node-forge) authenticated attributes to a PKCS#7 signed package?

I'm attempting the following, but see "empty" results when I openssl asn1parse ... the output:

const signed = forge.pkcs7.createSignedData();

signed.addSigner({
    key                 : privateKey,
    certificate             : cert,
    digestAlgorithm     : forge.pki.oids.sha1,
    authenticatedAttributes : [
        {
            type    : forge.pki.oids.contentType,
            value   : forge.pki.oids.data
        },
        {
            type: forge.pki.oids.messageDigest              
        },
        {
            type: forge.pki.oids.signingTime,               
        },
        //  :TODO:
        //  "The transactionID SHOULD be the MD5 hash of the public key from the
        //  request, encoded as a PrintableString"
        {
            name    : 'transactionID',
            type    : '2.16.840.1.113733.1.9.7',
            value   : '2dc41c4aa005263394db2c91904fd183',

        },
        {
            name    : 'messageType',
            type    : '2.16.840.1.113733.1.9.2',
            value   : '3',  //  CertRep         
        },
        {
            name    : 'senderNonce',
            type    : '2.16.840.1.113733.1.9.5',
            value   : '2528804744729265',
        },
        {
            name    : 'responderNonce',
            type    : '2.16.840.1.113733.1.9.6',
            value   : '9523024721942548',
        },  
        {
            name    : 'pkiStatus',
            type    : '2.16.840.1.113733.1.9.3',
            value   : '0',  //  SUCCESS
        }
    ] 
});

signed.content = forge.asn1.toDer(enveloped.toAsn1());
signed.sign();  

The OIDs in question are related to SCEP (See e.g. http://www.cisco.com/c/en/us/support/docs/security-vpn/public-key-infrastructure-pki/116167-technote-scep-00.html)

Example fragment dump from openssl asn1parse ...:

1579:d=6  hl=2 l=  14 cons: SEQUENCE          
 1581:d=7  hl=2 l=  10 prim: OBJECT            :2.16.840.1.113733.1.9.7
 1593:d=7  hl=2 l=   0 cons: SET               
@NuSkooler
Copy link
Author

BTW: I"m fully willing to implement and submit a PR if I can be given a little direction in what needs to be done :)

@dlongley
Copy link
Member

@NuSkooler -- saw this issue come in, just swamped at the moment and can't look into it. If there's no way to add custom attributes, we want to make sure there is ... I can't remember off the top of my head if it's possible but just difficult at the moment.

@NuSkooler
Copy link
Author

@dlongley Thanks for the reply! FWIW, I added a (probably considered hack) way to go about this in pkcs7.js _attributeToAsn1: If attr has an rawValue member, it just takes it as is. The caller (e.g. of addSigner) is expected to create rawValue with asn1.create():

function _attributeToAsn1(attr) {
  var value;

  // TODO: generalize to support more attributes
  if(attr.type === forge.pki.oids.contentType) {
    value = asn1.create(asn1.Class.UNIVERSAL, asn1.Type.OID, false,
      asn1.oidToDer(attr.value).getBytes());
  } else if(attr.type === forge.pki.oids.messageDigest) {
    value = asn1.create(asn1.Class.UNIVERSAL, asn1.Type.OCTETSTRING, false,
      attr.value.bytes());
  } else if(attr.type === forge.pki.oids.signingTime) {
    /* Note per RFC 2985: Dates between 1 January 1950 and 31 December 2049
      (inclusive) MUST be encoded as UTCTime. Any dates with year values
      before 1950 or after 2049 MUST be encoded as GeneralizedTime. [Further,]
      UTCTime values MUST be expressed in Greenwich Mean Time (Zulu) and MUST
      include seconds (i.e., times are YYMMDDHHMMSSZ), even where the
      number of seconds is zero.  Midnight (GMT) must be represented as
      "YYMMDD000000Z". */
    // TODO: make these module-level constants
    var jan_1_1950 = new Date('Jan 1, 1950 00:00:00Z');
    var jan_1_2050 = new Date('Jan 1, 2050 00:00:00Z');
    var date = attr.value;
    if(typeof date === 'string') {
      // try to parse date
      var timestamp = Date.parse(date);
      if(!isNaN(timestamp)) {
        date = new Date(timestamp);
      } else if(date.length === 13) {
        // YYMMDDHHMMSSZ (13 chars for UTCTime)
        date = asn1.utcTimeToDate(date);
      } else {
        // assume generalized time
        date = asn1.generalizedTimeToDate(date);
      }
    }

    if(date >= jan_1_1950 && date < jan_1_2050) {
      value = asn1.create(
        asn1.Class.UNIVERSAL, asn1.Type.UTCTIME, false,
        asn1.dateToUtcTime(date));
    } else {
      value = asn1.create(
        asn1.Class.UNIVERSAL, asn1.Type.GENERALIZEDTIME, false,
        asn1.dateToGeneralizedTime(date));
    }
  } else if(attr.rawValue) {
    value = attr.rawValue;
  }

  // TODO: expose as common API call
  // create a RelativeDistinguishedName set
  // each value in the set is an AttributeTypeAndValue first
  // containing the type (an OID) and second the value
  return asn1.create(asn1.Class.UNIVERSAL, asn1.Type.SEQUENCE, true, [
    // AttributeType
    asn1.create(asn1.Class.UNIVERSAL, asn1.Type.OID, false,
      asn1.oidToDer(attr.type).getBytes()),
    asn1.create(asn1.Class.UNIVERSAL, asn1.Type.SET, true, [
      // AttributeValue
      value
    ])
  ]);
}

Usage:

signed.addSigner({
    key                     : privateKey,
    certificate             : cert
    digestAlgorithm         : forge.pki.oids.sha1,
    authenticatedAttributes : [
        {
            type    : forge.pki.oids.contentType,
            value   : forge.pki.oids.data
        },
        {
            type: forge.pki.oids.messageDigest              
        },
        {
            type: forge.pki.oids.signingTime,               
        },
        {
            name    : 'transactionID',
            type    : '2.16.840.1.113733.1.9.7',
            rawValue : forge.asn1.create(
                forge.asn1.Class.UNIVERSAL, forge.asn1.Type.PRINTABLESTRING, false, '2dc41c4aa005263394db2c91904fd183'),

        }
    ] 
});

This seems to work when I look at openssl asn1parse... but that doesn't necessarily mean it's valid -- that, I'm not sure.

@NuSkooler
Copy link
Author

Update: I can validate that the fix mentioned above functions properly!

@NuSkooler
Copy link
Author

If the above is something you are interested in, I can submit a PR. The only change is actually the else if for the rawValue check above:

} else if(attr.rawValue) {
    value = attr.rawValue;
  }

@dlongley
Copy link
Member

@NuSkooler -- I have to look into it, I thought we had something similar somewhere else and I'd like to keep the naming consistent.

nelsonkram pushed a commit to nelsonkram/forge that referenced this issue Dec 11, 2017
@troyfactor4
Copy link
Contributor

Hey @dlongley can I submit a PR for this? I really think it would add a lot of raw value to the node-forge ecosystem if we could add custom attributes :-D

Also, I'm running into the same issue as @NuSkooler

@troyfactor4
Copy link
Contributor

@dlongley is rawCapture what you were thinking of? It seems to be used elsewhere in pkcs7.js in just a slightly different context.

@petitout
Copy link

petitout commented Jan 19, 2021

Hi @dlongley , any news on this issue ? I need to have custom authenticatedAttributes but they are currently set as empty ...

I can create a PR, let me know

Thanks !

@ovk
Copy link

ovk commented May 11, 2021

Unfortunately, the suggested solutions won't work, as they will likely lead to generating invalid signature. The problem is that when attributes are put into SET OF for the signature calculation they must be in sorted order, and node-forge doesn't do anything like that.

@lulin1994
Copy link

Confirmed that @ovk is leading us on to the right track here. In case anyone else is bothered by this issue

@bhaase-cs
Copy link

Is there still no proper workaround to this issue that's been open since 2016? I'm currently copying the entire PKCS7 file just so I can inject a single line into attrbutesToAsn1 to support adding SCEP OIDs to authenticatedAttributes. Can we either get support for custom variables or atleast some kind of overridable prototype so we can specify custom behaviour?

@RedShift1
Copy link

I needed serialNumber and role attributes in the subject, was able to make it work by doing this:

import node_forge from 'node-forge';
const { asn1, pki, md } = node_forge;

pki.oids['2.5.4.5'] = 'serialNumber';
pki.oids['serialNumber'] = '2.5.4.5';
pki.oids['2.5.4.72'] = 'role';
pki.oids['role'] = '2.5.4.72';
...
cert.setSubject([{name: 'role', value: 'myrole'}, {name: 'serialNumber', value: 'A1234'}]);
...

Maybe this works for authenticatedAttributes too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants