[RFC/PATCH] receive-pack.c: only accept push-cert if push_cert_nonce was advertised

[Stefan Beller <sbeller@google.com>]

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@google.com>
---

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