digitalbazaar / forge

A native implementation of TLS in Javascript and tools to write crypto-based and network-heavy webapps
https://digitalbazaar.com/
Other
5.05k stars 778 forks source link

Cannot read property 'id' of undefined at Object.pki.certificateExtensionToAsn1 #273

Open uhhhh2 opened 9 years ago

uhhhh2 commented 9 years ago

When I try to add a custom extension to a certificate signing request, my program crashes with the following error:

asn1.oidToDer(ext.id).getBytes()));
                     ^
TypeError: Cannot read property 'id' of undefined
    at Object.pki.certificateExtensionToAsn1 (/home/logan/node_modules/node-forge/js/x509.js:2576:22)
    at _fillMissingFields (/home/logan/node_modules/node-forge/js/x509.js:2055:33)
    at Object.csr.setAttributes (/home/logan/node_modules/node-forge/js/x509.js:1793:5)
    at Object.<anonymous> (/home/logan/Desktop/nodejs_ssl/GenCSR.js:138:5)
    at Module._compile (module.js:428:26)
    at Object.Module._extensions..js (module.js:446:10)
    at Module.load (module.js:353:32)
    at Function.Module._load (module.js:308:12)
    at Function.Module.runMain (module.js:469:10)
    at startup (node.js:124:18)
    at node.js:882:3

I tried calling pki.certificateExtensionToAsn1() in my own code directly, which worked. Then, I added lots of Console.log() statements to my copy of _fillMissingExtensionFields() and _fillMissingFields() to try to find out what my structures were missing as they were passed through these functions. When I change the line in x509.js that says "return;" to "return e;" (line 2095), my code worked.

Here is the diff of the x509.js files:

--- /home/logan/Downloads/x509.js
+++ /home/logan/node_modules/node-forge/js/x509.js
@@ -2047,8 +2047,15 @@
       if(!attr.value && attr.extensions) {
         attr.value = [];
         for(var ei = 0; ei < attr.extensions.length; ++ei) {
-          attr.value.push(pki.certificateExtensionToAsn1(
-            _fillMissingExtensionFields(attr.extensions[ei])));
+          console.log('---------- HERE WE GO --------------------------------');
+          var theext = attr.extensions[ei];
+          console.log(theext);
+          var theext_missingfields = _fillMissingExtensionFields(theext);
+          console.log(theext_missingfields);
+          var theext_asn1 = pki.certificateExtensionToAsn1(theext_missingfields);
+          console.log(theext_asn1);
+          attr.value.push(theext_asn1);
+          console.log('----------- AND WE ARE OFF ---------------------------');
         }
       }
     }
@@ -2090,9 +2097,12 @@
       throw error;
     }
   }
+  console.log('------ here e is ---------------------------------------------');
+  console.log(e);
+  console.log('------ e is done ---------------------------------------------');

   if(typeof e.value !== 'undefined') {
-    return;
+    return e;
   }

   // handle missing value:
@@ -2273,6 +2283,9 @@
     error.extension = e;
     throw error;
   }
+  console.log('------ here e is again ---------------------------------------');
+  console.log(e);
+  console.log('------ e is done again ---------------------------------------');

   return e;
 }

For reference, here is the section of my code that led to this issue:

var csr = forge.pki.createCertificationRequest();
console.log('csr made');
csr.publicKey = keys.publicKey;
console.log('public key set');
csr.setSubject(distinguishedName);
console.log('subject set');
for(var i = 0; i < customExtensions.length; i++){
    console.log(forge.pki.certificateExtensionToAsn1(customExtensions[i]));
    console.log("extension " + i + " ok");
}
csr.setAttributes([{ ////<-------- THIS IS WHERE IT CRASHES without "return e;"
  name: 'extensionRequest',
  extensions: customExtensions
}]);
console.log('attributes set');
console.log(csr);
csr.sign(keys.privateKey);
console.log('signed');
csr.verify();
console.log('verified');
var pem = forge.pki.certificationRequestToPem(csr);
console.log('pem ready for file');
fs.writeFileSync(csrFileName, pem); // <------ It completes this with the "return e"

where customExtensions is an array of ASN.1 structures generated with forge.asn1.create:

[ { name: 'first_custom_extension',
    id: '2.25.10',
    value: 
     { tagClass: 0,
       type: 16,
       constructed: true,
       composed: true,
       value: [Object] } },
  { name: 'second_custom_extension',
    id: '2.25.30',
    value: 
     { tagClass: 0,
       type: 16,
       constructed: true,
       composed: true,
       value: [Object] } } ]

Aside from the fact that the OIDs are unregistered, what am I doing wrong?

dlongley commented 9 years ago

You found a bug; the x509 module should have been returning e, good catch. With that fix, does everything work as expected now?

dlongley commented 9 years ago

Can you provide a full example of what you're doing (including the creation of the custom extensions) so I can build a test for this? If you place a line with only triple backticks (```) before and after the code you put into a comment, it will be more readable.

uhhhh2 commented 9 years ago

Here is a full example, including the custom extensions. Everything works as expected. EDIT: Changed the way the bitstring is constructed. It may need '\u0000' followed by the contents as a hex string.

"use strict";

//Maybe not all of these are required. 
var fs = require('fs');
var readlineSync = require('readline-sync');
var path = require('path');
var forge = require('node-forge');

/**
 * GenCSR.js
 * This script generates a RSA key and a Certificate Signing Request (CSR). 
 * 
 * Sample Input (you will need to copy in manually, line by line):
 * key.pem
 * key_password
 * [WAIT UNTIL KEY GENERATION IS COMPLETE BEFORE COPYING ANY FURTHER]
 * Example Corp
 * Client Division
 * Anytown
 * Anystate
 * US 
 * John Doe
 * john@test.com
 * extension_1 
 * 2.25.10 
 * file_1 
 * 2.25.11 
 * 0 
 * testfile.txt 
 * exit 
 * exit 
 * csr.csr
 */

/**
 * Prompts the user for child values for a custom extension. 
 * ext_name is the name of the custom extension. 
 * Returns an ASN.1 object representing the new child value. 
 */
function addNewValueToExtension(ext_name){
    var newValueName = readlineSync.question("Enter the name of the new value for " + 
        ext_name + ", or type \'exit\' to quit: ").toLowerCase();
    if(newValueName == "exit"){
        return null;
    }
    else{
        var oidStr = readlineSync.question("Enter the OID for " + 
            newValueName + ": ").toLowerCase();
        var oidASN1 = forge.asn1.create(forge.asn1.Class.UNIVERSAL, 
            forge.asn1.Type.OID, false, oidStr);

        var tagInt = parseInt(readlineSync.question("Enter the tag for " + 
            newValueName + ": "));
        var tagASN1 = forge.asn1.create(forge.asn1.Class.UNIVERSAL, 
            forge.asn1.Type.INTEGER, false, tagInt);

        var contentsFile = readlineSync.question("Enter the file to use for " 
            + newValueName + ": ");
        var contentsASN1 = forge.asn1.create(forge.asn1.Class.UNIVERSAL, 
            forge.asn1.Type.BITSTRING, false, 
            '\u0000' + fs.readFileSync(contentsFile).toString('hex'));

        var tagAndContentsSequenceASN1 = forge.asn1.create(forge.asn1.Class.UNIVERSAL, 
            forge.asn1.Type.SEQUENCE, true, [tagASN1, contentsASN1]);
        var oidAndTagAndContentsSequenceASN1 = forge.asn1.create(
            forge.asn1.Class.UNIVERSAL, forge.asn1.Type.SEQUENCE,
            true, [oidASN1, tagAndContentsSequenceASN1]);
        return oidAndTagAndContentsSequenceASN1;
    }
}

/**
 * Creates a new extension 
 * Returns a JSON structure with the following:
 *     name: the name of the custom extension.
 *     id: the OID of the custom extension.
 *     value: The SEQUENCE of ASN.1 values for the custom extension.  
 */
function addNewExtension(){
    var name = readlineSync.question("Enter the name of the new extension, " + 
        "or type \'exit\' to quit: ").toLowerCase();
    if(name == "exit"){
        return null;
    }
    else{
        var oidStr = readlineSync.question("Enter the OID for " + 
            name + ": ").toLowerCase();
        var oid = forge.asn1.create(forge.asn1.Class.UNIVERSAL, 
            forge.asn1.Type.OID, false, oidStr); 
        var criticalBoolean = forge.asn1.create(forge.asn1.Class.UNIVERSAL, 
            forge.asn1.Type.BOOLEAN, false, false);
        var values = [];
        while(1){
            var newValue = addNewValueToExtension(name);
            if(!newValue){
                break;
            }
            else{
                values.push(newValue);
            }
        }
        var valuesASN1 = forge.asn1.create(
            forge.asn1.Class.UNIVERSAL, 
            forge.asn1.Type.SEQUENCE, true, values);
        var valuesDER = forge.asn1.toDer(valuesASN1);
        var valuesOctetString = forge.asn1.create(
            forge.asn1.Class.UNIVERSAL, 
            forge.asn1.Type.OCTETSTRING, 
            false, valuesDER);
        var extensionASN1 = forge.asn1.create(
            forge.asn1.Class.UNIVERSAL, 
            forge.asn1.Type.SEQUENCE, true, 
            [oid, criticalBoolean, valuesOctetString]);
        return {name: name, id: oidStr, value:valuesASN1};
    }
}

/**
 * Prompts the user for their Distinguished Name (DN) information
 * and returns the array of name-value pairs.  
 */
function makeCodeForDN(){
    return [{
        name: 'organizationName',
        value: readlineSync.question("\nWhat is your Organization Name? ")
    }, {
        name: 'organizationalUnitName',
        value: readlineSync.question("\nWhat is your Organizational Unit? ")
    }, {
        name: 'localityName',
        value: readlineSync.question("\nWhat is your City/District/Town name? ")
    }, {
        name: 'stateOrProvinceName',
        value: readlineSync.question("\nWhat is your State/Province name? ")
    }, {
        name: 'countryName',
        value: readlineSync.question("\nWhat is your 2-letter Country Code? ")
    }, {
        name: 'commonName',
        value: readlineSync.question(
            "What is your common name (eg, your name or your server's hostname)? ")
    }, {
        name: 'emailAddress',
        value: readlineSync.question("\nWhat is your email address? ")
    }];
}

/**
 * Makes the private key and returns the file and password. 
 */
function makeNewKey() {
    //prompt for file info, password 
    var fle = readlineSync.question("\nWhat filename do you want to store your key in? ");
    fle = path.resolve(fle);
    var pass = readlineSync.question("\nWhat password do you want for the key file? ", 
        {'hideEchoBack': true});

    //make the keys and save to file
    var keys =  forge.pki.rsa.generateKeyPair(4096);
    var pem = forge.pki.encryptRsaPrivateKey(keys.privateKey, pass, {algorithm: '3des', legacy: true});
    fs.writeFileSync(fle, pem);
    console.log("The key file has been created.");
    return keys;
} 

/* ************************************************************************** */
/* ************************ BEGIN MAIN CODE ********************************* */
/* ************************************************************************** */
console.log("Welcome to the GenCSR script. " + 
    "This script will generate a certificate signing request (CSR) and a RSA keypair. ");

var keys = makeNewKey();
var distinguishedName = makeCodeForDN();

//Handle the custom extensions. 
var customExtensions = [];
var newExtension = null;
while((newExtension = addNewExtension()) != null){
    customExtensions.push(newExtension);
}

var csrFileName = readlineSync.question("\nWhat name do you want for the certificate signing request? ");
csrFileName = path.resolve(csrFileName);

//This is the portion of code referenced in the original issue posting. 
var csr = forge.pki.createCertificationRequest();
csr.publicKey = keys.publicKey;
csr.setSubject(distinguishedName);
csr.setAttributes([{
  name: 'extensionRequest',
  extensions: customExtensions
}]);
csr.sign(keys.privateKey);
csr.verify();

/*
 * NOTE: The logging can only work if you actually add a custom extension.   
 */
var extensionLabel = customExtensions[0];
console.log(extensionLabel);
console.log('-- end custom extension label --');
var extensionVal = extensionLabel.value;
console.log(extensionVal);
console.log('-- end custom extension root --');
console.log(extensionVal.value);
console.log('--end custom extension root.value --');
console.log(extensionVal.value[0].value);
console.log('-- end custom extension root.value[0].value --');
console.log(extensionVal.value[0].value[1].value);
console.log('-- end custom extension root.value[0].value[1].value --');

//write to the certificate file. 
var pem = forge.pki.certificationRequestToPem(csr, 64);
fs.writeFileSync(csrFileName, pem);

console.log("The private key and CSR have been created. ");

/* ************************************************************************** */
/* ************************ END MAIN CODE *********************************** */
/* ************************************************************************** */
dlongley commented 9 years ago

@uhhhh2,

Everything works as expected.

Ok, great. Thanks for the example code; I'll turn it into a test when I get some time. I'm going to leave this issue open until a test can be written.