Re: [PATCH 18/18] signed push: final protocol update
Shawn Pearce spea...@spearce.org writes: As you know, the stateless HTTP thing doesn't allow the nonce on the server to be carried from the initial ref advertisement into the final receive-pack. We would either need to write the nonce to disk and load it back up later (ick), or use some sort of stateless nonce. A stateless nonce could look like: nonce = HMAC_SHA1( SHA1(site+path) + '.' + now, site_key ) where site_key is a private key known to the server. It doesn't have to be per-repo. Doing the above naively will force you to check 600 HMAC if your slack is for 10 minutes. You could just instead use nonce = now '-' HMAC_SHA1(path + '.' + now, site_key) and the validation side can make sure the same site_key was used, and also now readable from the plaintext part is fresh enough, with a single HMAC. I may be missing something, but with this, we can always validate that nonce is what the repository issued (whether stateless is used or not). The hook script can decide if now is recent enough or not without bothering receive-pack at all. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/18] signed push: final protocol update
On Thu, Sep 4, 2014 at 4:57 PM, Junio C Hamano gits...@pobox.com wrote: Shawn Pearce spea...@spearce.org writes: As you know, the stateless HTTP thing doesn't allow the nonce on the server to be carried from the initial ref advertisement into the final receive-pack. We would either need to write the nonce to disk and load it back up later (ick), or use some sort of stateless nonce. A stateless nonce could look like: nonce = HMAC_SHA1( SHA1(site+path) + '.' + now, site_key ) where site_key is a private key known to the server. It doesn't have to be per-repo. Doing the above naively will force you to check 600 HMAC if your slack is for 10 minutes. You could just instead use nonce = now '-' HMAC_SHA1(path + '.' + now, site_key) and the validation side can make sure the same site_key was used, and also now readable from the plaintext part is fresh enough, with a single HMAC. Argh. Yes, thank you. This is what I meant but did not write. :( I may be missing something, but with this, we can always validate that nonce is what the repository issued (whether stateless is used or not). The hook script can decide if now is recent enough or not without bothering receive-pack at all. Correct. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/18] signed push: final protocol update
On Mon, Aug 25, 2014 at 10:59 AM, Junio C Hamano gits...@pobox.com wrote: Shawn Pearce spea...@spearce.org writes: A stateless nonce could look like: nonce = HMAC_SHA1( SHA1(site+path) + '.' + now, site_key ) where site_key is a private key known to the server. It doesn't have to be per-repo. receive-pack would then be willing to accept any nonce whose timestamp is within a window, e.g. 10 minutes of the current time, and whose signature verifies in the HMAC. The 10 minute window is important to allow clients time to generate the object list, perform delta compression, and begin transmitting to the server. Hmph, don't you send the finally tell the other end the sequence of update this ref from old to new and the packdata separately? No. The command list (triples of old, new, ref) is sent in the same HTTP request as the pack data, ahead of the pack data. So its one request. Push on smart HTTP is 3 HTTP requests: 1) get advertisement 2) POST empty flush packet to tickle auth (literally just ). 3) POST command list + pack The nonce can be sent server-client in 1, and client-server in 3. I think we have a FLUSH in between, and the push certificate is given before the FLUSH, which you do not have to wait for 10 minutes. Nope I think you need to wait for the pack to generate enough to start sending the pack data stream. Nothing forces the smart HTTP client to push its pending buffer out. We wait for the pack data to either finish, or overflow the in-memory buffer, and then start transmitting. If your client needs a lot of time for counting and delta compression, we aren't likely to overflow and transmit for a while. If you send a _lot_ of refs you can overflow, which will cause us to transmit early. But we are talking about megabytes worth of (old, new, ref) triplets to reach that overflow point. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/18] signed push: final protocol update
Shawn Pearce spea...@spearce.org writes: On Mon, Aug 25, 2014 at 10:59 AM, Junio C Hamano gits...@pobox.com wrote: Shawn Pearce spea...@spearce.org writes: A stateless nonce could look like: nonce = HMAC_SHA1( SHA1(site+path) + '.' + now, site_key ) where site_key is a private key known to the server. It doesn't have to be per-repo. receive-pack would then be willing to accept any nonce whose timestamp is within a window, e.g. 10 minutes of the current time, and whose signature verifies in the HMAC. The 10 minute window is important to allow clients time to generate the object list, perform delta compression, and begin transmitting to the server. Hmph, don't you send the finally tell the other end the sequence of update this ref from old to new and the packdata separately? No. The command list (triples of old, new, ref) is sent in the same HTTP request as the pack data, ahead of the pack data. So its one request. That is unfortunate. Would it be a major surgery to update the protocol not to do that, perhaps by moving the command list from 3 to 2 (the latter of which is not currently doing anything useful payload-wise, other than flushing a HTTP request early)? Push on smart HTTP is 3 HTTP requests: 1) get advertisement 2) POST empty flush packet to tickle auth (literally just ). 3) POST command list + pack The nonce can be sent server-client in 1, and client-server in 3. I think we have a FLUSH in between, and the push certificate is given before the FLUSH, which you do not have to wait for 10 minutes. Nope I think you need to wait for the pack to generate enough to start sending the pack data stream. Nothing forces the smart HTTP client to push its pending buffer out. We wait for the pack data to either finish, or overflow the in-memory buffer, and then start transmitting. If your client needs a lot of time for counting and delta compression, we aren't likely to overflow and transmit for a while. If you send a _lot_ of refs you can overflow, which will cause us to transmit early. But we are talking about megabytes worth of (old, new, ref) triplets to reach that overflow point. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/18] signed push: final protocol update
Junio C Hamano gits...@pobox.com writes: That is unfortunate. Would it be a major surgery to update the protocol not to do that, perhaps by moving the command list from 3 to 2 (the latter of which is not currently doing anything useful payload-wise, other than flushing a HTTP request early)? Nah, that was one of the most stupid thing I ever said here X. There is nothing that ties #2 and #3 unless the server side keeps some state, so that would not work very well X-. Push on smart HTTP is 3 HTTP requests: 1) get advertisement 2) POST empty flush packet to tickle auth (literally just ). 3) POST command list + pack -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/18] signed push: final protocol update
Shawn Pearce spea...@spearce.org writes: A stateless nonce could look like: nonce = HMAC_SHA1( SHA1(site+path) + '.' + now, site_key ) where site_key is a private key known to the server. It doesn't have to be per-repo. receive-pack would then be willing to accept any nonce whose timestamp is within a window, e.g. 10 minutes of the current time, and whose signature verifies in the HMAC. The 10 minute window is important to allow clients time to generate the object list, perform delta compression, and begin transmitting to the server. Hmph, don't you send the finally tell the other end the sequence of update this ref from old to new and the packdata separately? I think we have a FLUSH in between, and the push certificate is given before the FLUSH, which you do not have to wait for 10 minutes. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/18] signed push: final protocol update
Junio C Hamano gits...@pobox.com writes: There are a few gotchas I can certainly use help on, especially from a smart-http expert ;-). * pushed-to URL will identify the site and the repository, so you cannot MITM my push to an experimental server and replay it against the authoritative server. However, the receiving end may not even know what name its users call the repository being pushed into. Obviously gethostname() may not be what the pusher called us, and getcwd() may not match the repository name without leading /var/repos/shard3/ path components stripped, for example. I am not sure if we even have the necessary information at send-pack.c::send_pack() level, where it already has an established connection to the server (hence it does not need to know to whom it is talking to). * The receiving end will issue push-cert=nonce in its initial capability advertisement, and this nonce will be given on the PUSH_CERT_NONCE environment to the pre/post-receive hooks, to allow the nonce nonce header in the signed certificate to be checked against it. You cannot capture my an earlier push to the authoritative server and replay it later. That would all work well within a single receive-pack process, but with stateless RPC, it is unclear to me how we should arrange the nonce the initial instance of receive-pack placed on its capability advertisement to be securely passed to the instance of receive-pack that actually receives the push certificate. A good nonce may be something like taking the SHA-1 hash of the concatenation of the sitename, repo-path and the timestamp when the receive-pack generated the nonce. Replaying a push certificate for a push to a repository at a site that gives such a nonce can succeed at the same chance of finding a SHA-1 collision [*1*]. As long as you exercise good hygiene and only push to repositories that give such nonce, we can do without checking pushed-to that says where the push went. So nonce nonce is the only thing that is necessary to make them impossible to replay. For auditing purposes, pushed-to URL that records the repository the pusher intended to push to may help but probably not necessary [*2*]. [Footnote] *1* And the old-sha1s recorded in the certificate has to match what the repository being attacked currently has; otherwise the push will fail with the ref moved while you were trying to push. *2* When auditing the history for a repository at a site, the certificate the auditors examine would be the ones accumulated at that site for the repository, so we would implicitly know the value for URL already. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/18] signed push: final protocol update
On Fri, Aug 22, 2014 at 10:59 AM, Junio C Hamano gits...@pobox.com wrote: Junio C Hamano gits...@pobox.com writes: There are a few gotchas I can certainly use help on, especially from a smart-http expert ;-). * pushed-to URL will identify the site and the repository, so you cannot MITM my push to an experimental server and replay it against the authoritative server. However, the receiving end may not even know what name its users call the repository being pushed into. Obviously gethostname() may not be what the pusher called us, and getcwd() may not match the repository name without leading /var/repos/shard3/ path components stripped, for example. I am not sure if we even have the necessary information at send-pack.c::send_pack() level, where it already has an established connection to the server (hence it does not need to know to whom it is talking to). * The receiving end will issue push-cert=nonce in its initial capability advertisement, and this nonce will be given on the PUSH_CERT_NONCE environment to the pre/post-receive hooks, to allow the nonce nonce header in the signed certificate to be checked against it. You cannot capture my an earlier push to the authoritative server and replay it later. That would all work well within a single receive-pack process, but with stateless RPC, it is unclear to me how we should arrange the nonce the initial instance of receive-pack placed on its capability advertisement to be securely passed to the instance of receive-pack that actually receives the push certificate. A good nonce may be something like taking the SHA-1 hash of the concatenation of the sitename, repo-path and the timestamp when the receive-pack generated the nonce. Replaying a push certificate for a push to a repository at a site that gives such a nonce can succeed at the same chance of finding a SHA-1 collision [*1*]. As long as you exercise good hygiene and only push to repositories that give such nonce, we can do without checking pushed-to that says where the push went. Yes, this is an interesting solution. As you know, the stateless HTTP thing doesn't allow the nonce on the server to be carried from the initial ref advertisement into the final receive-pack. We would either need to write the nonce to disk and load it back up later (ick), or use some sort of stateless nonce. A stateless nonce could look like: nonce = HMAC_SHA1( SHA1(site+path) + '.' + now, site_key ) where site_key is a private key known to the server. It doesn't have to be per-repo. receive-pack would then be willing to accept any nonce whose timestamp is within a window, e.g. 10 minutes of the current time, and whose signature verifies in the HMAC. The 10 minute window is important to allow clients time to generate the object list, perform delta compression, and begin transmitting to the server. So nonce nonce is the only thing that is necessary to make them impossible to replay. For auditing purposes, pushed-to URL that records the repository the pusher intended to push to may help but probably not necessary [*2*]. So pushed-to is still useful as a documentation/historical artifact, but isn't important for verifying the certificate. That fixes a lot of problems with repositories having different paths under e.g. git:// vs. ssh:// vs. https:// on the same host. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/18] signed push: final protocol update
On Tue, Aug 19, 2014 at 3:06 PM, Junio C Hamano gits...@pobox.com wrote: + push-cert = PKT-LINE(push-cert NUL capability-list LF) Haha. NUL. I love our wire protocol. + PKT-LINE(certificate version 0.1 LF) + PKT-LINE(pusher ident LF) + PKT-LINE(LF) + *PKT-LINE(command LF) + *PKT-LINE(GPG signature lines LF) Should we include the URL as part of this certificate? Perhaps the pusher means to sign the master branch of experimental tree, but not their trunk tree? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/18] signed push: final protocol update
Shawn Pearce spea...@spearce.org writes: On Tue, Aug 19, 2014 at 3:06 PM, Junio C Hamano gits...@pobox.com wrote: + push-cert = PKT-LINE(push-cert NUL capability-list LF) Haha. NUL. I love our wire protocol. + PKT-LINE(certificate version 0.1 LF) + PKT-LINE(pusher ident LF) + PKT-LINE(LF) + *PKT-LINE(command LF) + *PKT-LINE(GPG signature lines LF) Should we include the URL as part of this certificate? Perhaps the pusher means to sign the master branch of experimental tree, but not their trunk tree? Yes, in $gmane/255582 I cover this and also mention that we would need some nonce from the receiving end to make it harder to replay. Currently I am leaning toward to add both pushed-to URL and also nonce nonce, the latter of which the receiver can ask with push-cert=nonce in its initial capability advertisement. There are a few gotchas I can certainly use help on, especially from a smart-http expert ;-). * pushed-to URL will identify the site and the repository, so you cannot MITM my push to an experimental server and replay it against the authoritative server. However, the receiving end may not even know what name its users call the repository being pushed into. Obviously gethostname() may not be what the pusher called us, and getcwd() may not match the repository name without leading /var/repos/shard3/ path components stripped, for example. I am not sure if we even have the necessary information at send-pack.c::send_pack() level, where it already has an established connection to the server (hence it does not need to know to whom it is talking to). * The receiving end will issue push-cert=nonce in its initial capability advertisement, and this nonce will be given on the PUSH_CERT_NONCE environment to the pre/post-receive hooks, to allow the nonce nonce header in the signed certificate to be checked against it. You cannot capture my an earlier push to the authoritative server and replay it later. That would all work well within a single receive-pack process, but with stateless RPC, it is unclear to me how we should arrange the nonce the initial instance of receive-pack placed on its capability advertisement to be securely passed to the instance of receive-pack that actually receives the push certificate. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/18] signed push: final protocol update
On Tue, 2014-08-19 at 15:06 -0700, Junio C Hamano wrote: +If the receiving end does not support push-cert, the sending end MUST +NOT send a push-cert command. + +When a push-cert command is sent, command-list MUST NOT be sent; the +commands recorded in the push certificate is used instead. The GPG +signature lines are detached signature for the contents recorded in are a detached signature +the push certificate before the signature block begins and is used which is used (or and are used) +to certify that the commands were given by the pusher which must be , who must be +the signer. + + +The receive-pack server that advertises this capability is willing +to accept a signed push certificate. A send-pack client MUST NOT +send push-cert packet unless the receive-pack server advertises this packets (or a push-cert packet) +static void queue_commands_from_cert(struct command **p, Uninformative parameter name p. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/18] signed push: final protocol update
On Aug 21, 2014, at 16:40, Junio C Hamano wrote: * The receiving end will issue push-cert=nonce in its initial capability advertisement, and this nonce will be given on the PUSH_CERT_NONCE environment to the pre/post-receive hooks, to allow the nonce nonce header in the signed certificate to be checked against it. You cannot capture my an earlier push to the authoritative server and replay it later. That would all work well within a single receive-pack process, but with stateless RPC, it is unclear to me how we should arrange the nonce the initial instance of receive-pack placed on its capability advertisement to be securely passed to the instance of receive-pack that actually receives the push certificate. Have you considered having the advertised nonce only be updated after receipt of a successful signed push? It would eliminate the stateless issue. And since the next nonce to be advertised would be updated at the successful completion of a receive of a signed push no replay would be possible. (I'm assuming that receive hook activity is already pipelined in the case of simultaneous pushes via some lock file or something or this scheme falls apart.) The obvious downside is that only one of two or more simultaneous signed pushers could succeed. But the sender could be modified to automatically retry (a limited number of times) on a nonce mismatch error. A receive hook could also be responsible for generating the next nonce value using this technique. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 18/18] signed push: final protocol update
On Thu, Aug 21, 2014 at 12:28 PM, Shawn Pearce spea...@spearce.org wrote: On Tue, Aug 19, 2014 at 3:06 PM, Junio C Hamano gits...@pobox.com wrote: + push-cert = PKT-LINE(push-cert NUL capability-list LF) Haha. NUL. I love our wire protocol. It is a direct and natural consequence of [PATCH 02/18]. We could use SP here, if we really wanted to, but that would make the push-cert packet a special kind that is different from others, which we would want to avoid. shallow is already special in that it cannot even carry the feature request, and it is not worth introducing and advertising a new capability to fix it, but at least we can avoid making the same mistake here. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 18/18] signed push: final protocol update
With the interim protocol, we used to send the update commands even
though we already send a signed copy of the same information when
push certificate is in use. Update the send-pack/receive-pack pair
not to do so.
The notable thing on the receive-pack side is that it makes sure
that there is no command sent over the traditional protocol packet
outside the push certificate. Otherwise a pusher can claim to be
pushing one set of ref updates in the signed certificate while
issuing commands to update unrelated refs, and such an update will
evade later audits.
Signed-off-by: Junio C Hamano gits...@pobox.com
---
Documentation/technical/pack-protocol.txt | 20 -
Documentation/technical/protocol-capabilities.txt | 12 +--
builtin/receive-pack.c| 26 +++
send-pack.c | 2 +-
4 files changed, 56 insertions(+), 4 deletions(-)
diff --git a/Documentation/technical/pack-protocol.txt
b/Documentation/technical/pack-protocol.txt
index a845d51..f6c3073 100644
--- a/Documentation/technical/pack-protocol.txt
+++ b/Documentation/technical/pack-protocol.txt
@@ -465,7 +465,7 @@ contain all the objects that the server will need to
complete the new
references.
- update-request= *shallow command-list [pack-file]
+ update-request= *shallow ( command-list | push-cert ) [pack-file]
shallow = PKT-LINE(shallow SP obj-id)
@@ -481,12 +481,30 @@ references.
old-id= obj-id
new-id= obj-id
+ push-cert = PKT-LINE(push-cert NUL capability-list LF)
+ PKT-LINE(certificate version 0.1 LF)
+ PKT-LINE(pusher ident LF)
+ PKT-LINE(LF)
+ *PKT-LINE(command LF)
+ *PKT-LINE(GPG signature lines LF)
+ PKT-LINE(push-cert-end LF)
+
pack-file = PACK 28*(OCTET)
If the receiving end does not support delete-refs, the sending end MUST
NOT ask for delete command.
+If the receiving end does not support push-cert, the sending end MUST
+NOT send a push-cert command.
+
+When a push-cert command is sent, command-list MUST NOT be sent; the
+commands recorded in the push certificate is used instead. The GPG
+signature lines are detached signature for the contents recorded in
+the push certificate before the signature block begins and is used
+to certify that the commands were given by the pusher which must be
+the signer.
+
The pack-file MUST NOT be sent if the only command used is 'delete'.
A pack-file MUST be sent if either create or update command is used,
diff --git a/Documentation/technical/protocol-capabilities.txt
b/Documentation/technical/protocol-capabilities.txt
index e174343..5b398e9 100644
--- a/Documentation/technical/protocol-capabilities.txt
+++ b/Documentation/technical/protocol-capabilities.txt
@@ -18,8 +18,8 @@ was sent. Server MUST NOT ignore capabilities that client
requested
and server advertised. As a consequence of these rules, server MUST
NOT advertise capabilities it does not understand.
-The 'report-status', 'delete-refs', and 'quiet' capabilities are sent and
-recognized by the receive-pack (push to server) process.
+The 'report-status', 'delete-refs', 'quiet', and 'push-cert' capabilities
+are sent and recognized by the receive-pack (push to server) process.
The 'ofs-delta' and 'side-band-64k' capabilities are sent and recognized
by both upload-pack and receive-pack protocols. The 'agent' capability
@@ -250,3 +250,11 @@ allow-tip-sha1-in-want
If the upload-pack server advertises this capability, fetch-pack may
send want lines with SHA-1s that exist at the server but are not
advertised by upload-pack.
+
+push-cert
+-
+
+The receive-pack server that advertises this capability is willing
+to accept a signed push certificate. A send-pack client MUST NOT
+send push-cert packet unless the receive-pack server advertises this
+capability.
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index abdc296..96e4c99 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -907,6 +907,28 @@ static struct command **queue_command(struct command **p,
return cmd-next;
}
+static void queue_commands_from_cert(struct command **p,
+struct strbuf *push_cert)
+{
+ const char *boc, *eoc;
+
+ if (*p)
+ die(protocol error: got both push certificate and unsigned
commands);
+
+ boc = strstr(push_cert-buf, \n\n);
+ if (!boc)
+ die(malformed push certificate %.*s, 100, push_cert-buf);
+ else
+ boc += 2;
+ eoc = push_cert-buf + parse_signature(push_cert-buf, push_cert-len);
+
+ while (boc eoc) {
+ const char *eol = memchr(boc, '\n', eoc - boc);
+ p = queue_command(p, boc, eol ? eol - boc : eoc - eol);
+ boc =
