| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
How does this pull request add signing support? It seems like it's just adding a new convenience Repository.createCommitWithSignature function that wraps around the existing Commit.createWithSignature. |
Sorry, something went wrong.
|
That's exactly right - it wraps the Commit.createWithSignature method. Would you rather see this method extend Repository.createCommit to take the private key and password additional parameters and do the signing right here in this library? Repository.prototype.createCommitWithSignature = function(
updateRef, author, committer, message, tree, parents, privateKey, password, callback)
Or do you have something else in mind? |
Sorry, something went wrong.
|
Make sure to enable support for git_commit_extract_signature for this PR by updating https://github.com/nodegit/nodegit/blob/master/generate/input/descriptor.json#L525 |
Sorry, something went wrong.
|
@implausible awesome! I'll give that a try |
Sorry, something went wrong.
|
@implausible I'm having some trouble calling extractSignature. Do you know a way to handle these buffers to write the signature and signed_data to? Here's the generated .cc /*
* @param Buffer signature
* @param Buffer signed_data
* @param Repository repo
* @param Oid commit_id
* @param String field
*/
NAN_METHOD(GitCommit::ExtractSignature) {
if (info.Length() == 0 || !info[0]->IsObject()) {
return Nan::ThrowError("Buffer signature is required.");
}
if (info.Length() == 1 || !info[1]->IsObject()) {
return Nan::ThrowError("Buffer signed_data is required.");
}
if (info.Length() == 2 || !info[2]->IsObject()) {
return Nan::ThrowError("Repository repo is required.");
}
if (info.Length() == 3
|| (!info[3]->IsObject() && !info[3]->IsString())) {
return Nan::ThrowError("Oid commit_id is required.");
}
if (info.Length() == 4 || !info[4]->IsString()) {
return Nan::ThrowError("String field is required.");
}
if (info.Length() == 5 || !info[5]->IsFunction()) {
return Nan::ThrowError("Callback is required and must be a Function.");
}
ExtractSignatureBaton* baton = new ExtractSignatureBaton;
baton->error_code = GIT_OK;
baton->error = NULL;
// start convert_from_v8 block
git_buf * from_signature = NULL;
from_signature = GitBufConverter::Convert(info[0]);
// end convert_from_v8 block
baton->signature = from_signature;
// start convert_from_v8 block
git_buf * from_signed_data = NULL;
from_signed_data = GitBufConverter::Convert(info[1]);
// end convert_from_v8 block
baton->signed_data = from_signed_data;
// start convert_from_v8 block
git_repository * from_repo = NULL;
from_repo = Nan::ObjectWrap::Unwrap<GitRepository>(info[2]->ToObject())->GetValue();
// end convert_from_v8 block
baton->repo = from_repo;
// start convert_from_v8 block
git_oid * from_commit_id = NULL;
if (info[3]->IsString()) {
// Try and parse in a string to a git_oid
String::Utf8Value oidString(info[3]->ToString());
git_oid *oidOut = (git_oid *)malloc(sizeof(git_oid));
if (git_oid_fromstr(oidOut, (const char *) strdup(*oidString)) != GIT_OK) {
free(oidOut);
if (giterr_last()) {
return Nan::ThrowError(giterr_last()->message);
} else {
return Nan::ThrowError("Unknown Error");
}
}
from_commit_id = oidOut;
}
else {
from_commit_id = Nan::ObjectWrap::Unwrap<GitOid>(info[3]->ToObject())->GetValue();
}
// end convert_from_v8 block
baton->commit_id = from_commit_id;
baton->commit_idNeedsFree = info[3]->IsString();
// start convert_from_v8 block
const char * from_field = NULL;
String::Utf8Value field(info[4]->ToString());
// malloc with one extra byte so we can add the terminating null character C-strings expect:
from_field = (const char *) malloc(field.length() + 1);
// copy the characters from the nodejs string into our C-string (used instead of strdup or strcpy because nulls in
// the middle of strings are valid coming from nodejs):
memcpy((void *)from_field, *field, field.length());
// ensure the final byte of our new string is null, extra casts added to ensure compatibility with various C types
// used in the nodejs binding generation:
memset((void *)(((char *)from_field) + field.length()), 0, 1);
// end convert_from_v8 block
baton->field = from_field;
Nan::Callback *callback = new Nan::Callback(v8::Local<Function>::Cast(info[5]));
ExtractSignatureWorker *worker = new ExtractSignatureWorker(baton, callback);
if (!info[0]->IsUndefined() && !info[0]->IsNull())
worker->SaveToPersistent("signature", info[0]->ToObject());
if (!info[1]->IsUndefined() && !info[1]->IsNull())
worker->SaveToPersistent("signed_data", info[1]->ToObject());
if (!info[2]->IsUndefined() && !info[2]->IsNull())
worker->SaveToPersistent("repo", info[2]->ToObject());
if (!info[3]->IsUndefined() && !info[3]->IsNull())
worker->SaveToPersistent("commit_id", info[3]->ToObject());
if (!info[4]->IsUndefined() && !info[4]->IsNull())
worker->SaveToPersistent("field", info[4]->ToObject());
AsyncLibgit2QueueWorker(worker);
return;
}
|
Sorry, something went wrong.
|
You can circumvent the GitBuf with a new String(). See here for an example |
Sorry, something went wrong.
|
Let me know if you need any other help on this. I would really like to get GPG signing in sooner rather than later. Awesome work so far! |
Sorry, something went wrong.
|
@cjhoward92 thanks. I did give that a try, but I need to read the value back out. var buffer1 = new String(); var buffer2 = new String(); var responseCode = extractSignature(buffer1, buffer2, repo, oid, 'gpgsig'); I can get responseCode to go to 0 (success) with the right parameters, but buffer1 is never populated. I'm not sure what I am missing. |
Sorry, something went wrong.
| signature); | ||
| }) | ||
| .then(function(commitId) { | ||
| assert.equal(expectedCommitId, commitId); |
There was a problem hiding this comment.
I know you are still working, but I would like to see a test that verifies the signature in the gpgsig field of the commit header. That will definitely lend credence to fact that we have GPG signing.
Sorry, something went wrong.
|
Oh, I see. I didn't realize you were talking about the ExtractSignature function. Yeah, strings won't work. Does a Buffer object from node work? We may need to add some code to allocate a buffer for us. Let me look around and see what we have that does that already, if anything. I think the final signature will probably look different since we need to return some of this stuff... |
Sorry, something went wrong.
|
Check out descriptor.json definition for git_filter_list_apply_to_data. If you look at file_list.cc ApplyToData after you generate the source, it allocates a buffer for us. So, you will need to alter your definition a bit. Instead of "out": {
"isReturn": true,
"shouldAlloc": true
}
You will have to specify both signature and signature_data as shouldAlloc: true in the args list. Then you can pass in a new String(your_data) as the starting data, I think. The problem comes with the return value. What do we return? Looks like both buffers are overwritten. This is where we may need extra code to handle this case. We could take the altered buffers from libgit2 and return copies of each in a special object or something... I'm not really sure how to best handle this yet. Let me look into it a bit more. If you have ideas, let me know. Does that all make sense? I think what I told you can get you closer, but not all the way there quite yet. |
Sorry, something went wrong.
|
yes it makes sense. I'll try out generating with the buffer and see if I can get it to work. For what to return. We could return an object that has both, or just return the signature. var result = commit.extractSignature() and result could be {
'signature': 'asiudqw....',
'signed_data': 'tree asdads....'
}
or result could be just the signature asiudqw... the method I was planning to add to lib/commit.js would look something like this, but with the promises Commit.prototype.extractSignature = function() {
var signatureBuffer = new String();
var signedDataBuffer = new String();
NodeGit.Commit.extractSignature(signatureBuffer, signedDataBuffer, this.repo, this.id(), "gpgsig");
return signatureBuffer.toString();
}
|
Sorry, something went wrong.
|
Do you know what the difference between signature and signature_data is? What would we need signature_data for? I haven't used GPG enough or looked at libgit2 in much detail, so I am not sure what we will be using ExtractSignature for. I assume signature verification, but I don't really know. Overall I like your ideas! We just need to find the best path for you to be able to execute. For now, you can try setting just the signature as the return for that function and see if you get it working. If so, then we can move to the next step of determining the best way to return all of the data. |
Sorry, something went wrong.
|
sounds good. I'll try to get something working. I believe the intention of ExtractSignature is so that someone could take the signed_data and the signature and verify it against a public key signature is the actual signature produced from signing the commit. This is the full commit: "tree 6b79e22d69bf46e289df0345a14ca059dfc9bdf6\n\ parent 34734e478d6cf50c27c9d69026d93974d052c454\n\ author Ben Burkert <ben@benburkert.com> 1358451456 -0800\n\ committer Ben Burkert <ben@benburkert.com> 1358451456 -0800\n\ gpgsig -----BEGIN PGP SIGNATURE-----\n\ Version: GnuPG v1.4.12 (Darwin)\n\ \n\ iQIcBAABAgAGBQJQ+FMIAAoJEH+LfPdZDSs1e3EQAJMjhqjWF+WkGLHju7pTw2al\n\ o6IoMAhv0Z/LHlWhzBd9e7JeCnanRt12bAU7yvYp9+Z+z+dbwqLwDoFp8LVuigl8\n\ JGLcnwiUW3rSvhjdCp9irdb4+bhKUnKUzSdsR2CK4/hC0N2i/HOvMYX+BRsvqweq\n\ AsAkA6dAWh+gAfedrBUkCTGhlNYoetjdakWqlGL1TiKAefEZrtA1TpPkGn92vbLq\n\ SphFRUY9hVn1ZBWrT3hEpvAIcZag3rTOiRVT1X1flj8B2vGCEr3RrcwOIZikpdaW\n\ who/X3xh/DGbI2RbuxmmJpxxP/8dsVchRJJzBwG+yhwU/iN3MlV2c5D69tls/Dok\n\ 6VbyU4lm/ae0y3yR83D9dUlkycOnmmlBAHKIZ9qUts9X7mWJf0+yy2QxJVpjaTGG\n\ cmnQKKPeNIhGJk2ENnnnzjEve7L7YJQF6itbx5VCOcsGh3Ocb3YR7DMdWjt7f8pu\n\ c6j+q1rP7EpE2afUN/geSlp5i3x8aXZPDj67jImbVCE/Q1X9voCtyzGJH7MXR0N9\n\ ZpRF8yzveRfMH8bwAJjSOGAFF5XkcR/RNY95o+J+QcgBLdX48h+ZdNmUf6jqlu3J\n\ 7KmTXXQcOVpN6dD3CmRFsbjq+x6RHwa8u1iGn+oIkX908r97ckfB/kHKH7ZdXIJc\n\ cpxtDQQMGYFpXK/71stq\n\ =ozeK\n\ -----END PGP SIGNATURE-----\n\ \n\ a simple commit which works\n" this is the signature: -----BEGIN PGP SIGNATURE-----\n\ " Version: GnuPG v1.4.12 (Darwin)\n\ \n\ iQIcBAABAgAGBQJQ+FMIAAoJEH+LfPdZDSs1e3EQAJMjhqjWF+WkGLHju7pTw2al\n\ o6IoMAhv0Z/LHlWhzBd9e7JeCnanRt12bAU7yvYp9+Z+z+dbwqLwDoFp8LVuigl8\n\ JGLcnwiUW3rSvhjdCp9irdb4+bhKUnKUzSdsR2CK4/hC0N2i/HOvMYX+BRsvqweq\n\ AsAkA6dAWh+gAfedrBUkCTGhlNYoetjdakWqlGL1TiKAefEZrtA1TpPkGn92vbLq\n\ SphFRUY9hVn1ZBWrT3hEpvAIcZag3rTOiRVT1X1flj8B2vGCEr3RrcwOIZikpdaW\n\ who/X3xh/DGbI2RbuxmmJpxxP/8dsVchRJJzBwG+yhwU/iN3MlV2c5D69tls/Dok\n\ 6VbyU4lm/ae0y3yR83D9dUlkycOnmmlBAHKIZ9qUts9X7mWJf0+yy2QxJVpjaTGG\n\ cmnQKKPeNIhGJk2ENnnnzjEve7L7YJQF6itbx5VCOcsGh3Ocb3YR7DMdWjt7f8pu\n\ c6j+q1rP7EpE2afUN/geSlp5i3x8aXZPDj67jImbVCE/Q1X9voCtyzGJH7MXR0N9\n\ ZpRF8yzveRfMH8bwAJjSOGAFF5XkcR/RNY95o+J+QcgBLdX48h+ZdNmUf6jqlu3J\n\ 7KmTXXQcOVpN6dD3CmRFsbjq+x6RHwa8u1iGn+oIkX908r97ckfB/kHKH7ZdXIJc\n\ cpxtDQQMGYFpXK/71stq\n\ =ozeK\n\ -----END PGP SIGNATURE-----" This is signed_data: "tree 6b79e22d69bf46e289df0345a14ca059dfc9bdf6\n\ parent 34734e478d6cf50c27c9d69026d93974d052c454\n\ author Ben Burkert <ben@benburkert.com> 1358451456 -0800\n\ committer Ben Burkert <ben@benburkert.com> 1358451456 -0800\n\ \n\ a simple commit which works\n"; |
Sorry, something went wrong.
|
Cool, that makes sense. Yeah, we should probably get that in. Let me know if you want any extra help or just a second set of eyes! Thanks again for working on this. I know a lot of people want GPG in NodeGit so you will be a hero! |
Sorry, something went wrong.
|
@dabutvin I am opening up a PR shortly that will get git_commit_extract_signature built into nodegit for you. You'll want to rebase on that. I will post here when it's open. |
Sorry, something went wrong.
Sorry, something went wrong.
|
@implausible I updated to verify the signature from the signed commit using the new commit.getSignature method. |
Sorry, something went wrong.
|
Can you work your magic in descriptor.json for git_commit_create_buffer? This is what I am trying to set up in repository.js onSignature would be a function that is up to the caller to implement Repository.prototype.createCommitWithSignature = function(
author, committer, message, tree, parents, signature_field, onSignature, callback) {
var repo = this;
var promises = [];
parents = parents || [];
promises.push(repo.getTree(tree));
parents.forEach(function(parent) {
promises.push(repo.getCommit(parent));
});
return Promise.all(promises).then(function(results) {
tree = results[0];
// Get the normalized values for our input into the function
var parentsLength = parents.length;
parents = [];
for (var i = 0; i < parentsLength; i++) {
parents.push(results[i + 1]);
}
return Commit.createBuffer(
repo,
author,
committer,
null /* use default message encoding */,
message,
tree,
parents.length,
parents
);
}).then(function(commit_content) {
return Commit.createWithSignature(
repo,
commit_content,
onSignature(commit_content),
"gpgsig");
}, callback);
};
|
Sorry, something went wrong.
| it("Can create a signed commit", function() { | ||
| var expectedCommitId = "b78570159824e6fc161418a626a646933a10ea89"; | ||
|
|
||
| // https://github.com/libgit2/libgit2/blob/master/tests/commit/write.c#L362-L384 |
There was a problem hiding this comment.
I don't think we can just copy/paste this since libgit2 is licensed under the GPLv2 and NodeGit is licensed under MIT.
Sorry, something went wrong.
|
One thing to keep in mind with the onSignature callback is that it may be asynchronous. I would change the flow in your code to account for that. |
Sorry, something went wrong.
Sorry, something went wrong.
|
@implausible thanks for exposing the createCommitBuffer |
Sorry, something went wrong.
|
No changes for a while? Still working on it? @dabutvin |
Sorry, something went wrong.
|
I forgot all about this actually. I'll rebase the commit now - it looks finished, but there is a broken build. |
Sorry, something went wrong.
|
it passed! |
Sorry, something went wrong.
There was a problem hiding this comment.
Everything looks good but this one bit here. Thanks again for your work on this 😄
Sorry, something went wrong.
|
@implausible not sure what happened to the build, but the documentation config looks like it hung. |
Sorry, something went wrong.
- the commit that gets created and the signed content have to match
|
@implausible check out newline I appended to the end of the commit buffer and let me know what you think. I was doing some end to end testing of the signing and verification and found that when verifying the signature from the commit content, this newline was required to get an exact match with the commit content. |
Sorry, something went wrong.
|
Thanks for the reminder! |
Sorry, something went wrong.
|
This is great. We're also working on adding: libgit2/libgit2#4913. Hopefully the next release of nodegit will include signing for rebase and commit. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
This wraps the libgit commit signing methods.
I couldn't get to git_commit_extract_signature as it's not available until 0.24
https://libgit2.github.com/libgit2/#HEAD/group/commit/git_commit_extract_signature