If the server did not advertise the capability to have signed pushes
it should not accept signed pushes as stated in
Documentation/technical/protocol-capabilities.txt:
Client will then send a space separated list of capabilities it wants
to be in effect. The client MUST NOT ask for capabilities the server
did not say it supports.
Server MUST diagnose and abort if capabilities it does not understand
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.
After rereading the second paragraph I think they should also be reworded to
Server MUST diagnose and abort if capabilities it did not advertise
was sent.
Suppose there would be hypothetical flaw in the capability of signed
pushes (or any capability for the current reasoning) which may harm
the server. This would require a bugfix release if it was severe and
would put us on time pressure to get it done.
A change like the one proposed would allow us to tell the community to
simply configure the server to not advertise the feature and if not
advertised the flaw could not be abused.
I am not saying there is a problem now, but I am rather saying patches
similar to this one would buy us time in case of problems arising.
Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
---
Notes:
As I discovered the idea while composing the
atomic push series and the changes line of this
patch is closeby, this applies on top of
origin/sb/atomic-push (v12 as sent on Jan. 7th)
This patch is RFC, thinking about security best practice.
It's not enough to document the intended behavior in
Documentation/technical/protocol-capabilities.txt, but
rather enforce it in the code as well.
Any thoughts on that welcome!
Thanks,
Stefan
builtin/receive-pack.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 4c069c5..628d13a 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1276,7 +1276,8 @@ static struct command *read_head_info(struct sha1_array *shallow)
use_atomic = 1;
}
- if (!strcmp(line, "push-cert")) {
+ if (push_cert_nonce &&
+ !strcmp(line, "push-cert")) {
int true_flush = 0;
char certbuf[1024];
--
2.2.1.62.g3f15098
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html