Crash when private key is formatted incorrectly

[leonardpunt]

Hi,

The library (v0.9.0) crashes when a private key is formatted incorrectly. Examples are if the private key is a literal with \n characters, or if the newlines are replaced with spaces.

The following error is thrown:

140014095067008:error:0906D06C:PEM routines:PEM_read_bio:no start line:../deps/openssl/openssl/crypto/pem/pem_lib.c
:703:Expecting: ANY PRIVATE KEY
crypto.js:429
  var ret = this._binding.sign(toBuf(key));
                          ^
Error: SignFinal error
    at Sign.sign (crypto.js:429:27)
    at createRS256Signature (/app/node_modules/gcloud/node_modules/gapitoken/node_modules/jws/index.js:75:58)
    at jwsRS256Sign (/app/node_modules/gcloud/node_modules/gapitoken/node_modules/jws/index.js:68:21)
    at Object.jwsSign [as sign] (/app/node_modules/gcloud/node_modules/gapitoken/node_modules/jws/index.js:32:12)
    at GAPI.getAccessToken (/app/node_modules/gcloud/node_modules/gapitoken/gapitoken.js:56:25)
    at GAPI.getToken (/app/node_modules/gcloud/node_modules/gapitoken/gapitoken.js:35:14)
    at /app/node_modules/gcloud/lib/common/connection.js:207:10
    at process._tickCallback (node.js:419:13)

Also, it would be nice if the library could handle some alternative formats of a private key, like spaces instead of newlines. We're retrieving our private key from a environment variable and environment variables can't be multiline. So right now we have to denote newlines with some character and replace them with newlines in our code.

Kind regards,
Leonard

[stephenplusplus]

Hi Leonard - it seems like a better solution might be storing the path to a private key as an environment variable, rather than the key itself. You can pass that path to gcloud as keyFilename, and it will read and parse the file.

[leonardpunt]

Hi Stephen,

The reason we're storing the private key as an environment variable is that we're using Managed VMs. This means the key should be retrieved somehow when booting a new VM. Since adding the key file to our repository is not on option (security wise), we choose to add the private key to Compute Engine's MetaData. On startup we request the private key from the MetaData and store it as an environment variable.
We were thinking to store the key in Cloud Storage. This would enable us to download the key on startup, however adding the key to the MetaData felt like a better/easier option, at that time. What is your opinion on this? And do you think there is a better alternative?

[stephenplusplus]

Just speaking generally, it's best to keep private data out of public access and global context. But in this case, I'm not very familiar with Managed VMs. Talking with @silvolu, I think he has an idea -- expect to hear from him soon :)

[silvolu]

@leonardpunt The service account associated with a Managed VM is pre-authorized to provide access to your project’s default storage bucket and default datastore dataset.
If that suits your need, you don't need to pass in any key, and we'll handle the token request to the GCE metadata server for you.

[silvolu]

@leonardpunt as an example, gcloud-node-todos "should just work"™ on Managed VMs with no need for keys.

[leonardpunt]

@silvolu I need to connect to the datastore of a different project. But I could store the key file in the project's default storage bucket and download it from there on startup. Would that be a better solution?

[silvolu]

@leonardpunt You could also give the Managed VM's service account access to the other project: go to the Developer's Console, choose the other project, and under Permission add the Managed VM's service account email as a member. That should work.

[leonardpunt]

@silvolu I gave the MVM's service account access to the other project and removed the credentials information from the configuration details. I'm only passing the project id along. However I get this error:

Unauthorized.
Error: Unauthorized.
    at Object.handleResp (/app/node_modules/gcloud/lib/common/util.js:168:14)
    at IncomingMessage.<anonymous> (/app/node_modules/gcloud/lib/datastore/request.js:461:14)
    at IncomingMessage.emit (events.js:117:20)
    at _stream_readable.js:943:16
    at process._tickCallback (node.js:419:13)

[silvolu]

@leonardpunt please use the storage based workaround for the moment. I'll investigate as soon as I can and update this bug.

[Guuz]

@silvolu In theory the pre-authorization should work. However there is an open issue that this does not. See https://code.google.com/p/googleappengine/issues/detail?id=11380
I hope it is only broken for the datastore and works for cloud storage. That should solve our problem :)

Thanks for the help!

[ryanseys]

I realise this is rather untimely but if you would rather use an environment variable to store your key, you can download the key in JSON form, then store it in an environment variable, it will work.

.bashrc or .zshrc file:

export GCLOUD_KEY='{"private_key_id":"XXX", "private_key":"YYY", "client_email":"ZZZ@ZZZ.COM", "client_id":"ABC123", "type":"service_account"}'

yourcode.js file:

var projectId = process.env.GAE_LONG_APP_ID || process.env.DATASET_ID;
var keyContents = process.env.GCLOUD_KEY;

var gcloud = require('gcloud')({
  projectId: projectId,
  credentials: JSON.parse(keyContents) // parse the environment variable as JSON
});

[ryanseys]

Additionally, the private key is stored in this json blob with newlines just written as \n and = encoded as \u003d so the correct format in this json blob is something like:

"private_key": "-----BEGIN PRIVATE KEY-----\nXXX\nYYY\nZZZ\u003d\n-----END PRIVATE KEY-----\n"

Perhaps the = was encoded improperly when you stored it in the environment variable?

[Guuz]

Thanks @ryanseys , that does sound a bit simpler than what we are doing now. I'll have a look after the holidays ;-)
Have a nice one!

[ryanseys]

Great @Guuz! Have a great holiday! :)

[stephenplusplus]

@silvolu was there anything left to check on for this issue?

[ryanseys]

I'm going to close this because I got it to work with the environment variable with some changes to the encoding in #272 (comment) and we now have safe guards in place to prevent cryptic errors from arising and confusing the developers. I don't think @silvolu has any reason to investigate or raise this issue further.