From ea21f17f294ac6fcb6d7135faa8d562e45bf03b1 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Thu, 18 Jun 2015 10:55:16 -0400 Subject: [PATCH] PackCertificateParser: return null if nothing was received Add test for this case in both the enabled and disabled cases. Change-Id: If9d12192a2dc9f9dd1eac9844b5a7b0edadc0b34 --- .../transport/PushCertificateParserTest.java | 129 ++++++++++++++---- .../jgit/transport/BaseReceivePack.java | 3 +- .../jgit/transport/PushCertificateParser.java | 5 +- 3 files changed, 105 insertions(+), 32 deletions(-) diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushCertificateParserTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushCertificateParserTest.java index 7de933396..9c157c337 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushCertificateParserTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushCertificateParserTest.java @@ -43,55 +43,124 @@ package org.eclipse.jgit.transport; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.io.ByteArrayInputStream; import java.io.EOFException; import java.io.IOException; +import org.eclipse.jgit.errors.PackProtocolException; import org.eclipse.jgit.internal.storage.dfs.DfsRepositoryDescription; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.Repository; +import org.junit.Before; import org.junit.Test; /** Test for push certificate parsing. */ public class PushCertificateParserTest { - @Test - public void parseCertFromPktLine() throws Exception { - // Example push certificate generated by C git 2.2.0. - String input = "001ccertificate version 0.1\n" - + "0041pusher Dave Borowitz 1433954361 -0700\n" - + "0024pushee git://localhost/repo.git\n" - + "002anonce 1433954361-bde756572d665bba81d8\n" - + "0005\n" - + "00680000000000000000000000000000000000000000" - + " 6c2b981a177396fb47345b7df3e4d3f854c6bea7" - + " refs/heads/master\n" - + "0022-----BEGIN PGP SIGNATURE-----\n" - + "0016Version: GnuPG v1\n" - + "0005\n" - + "0045iQEcBAABAgAGBQJVeGg5AAoJEPfTicJkUdPkUggH/RKAeI9/i/LduuiqrL/SSdIa\n" - + "00459tYaSqJKLbXz63M/AW4Sp+4u+dVCQvnAt/a35CVEnpZz6hN4Kn/tiswOWVJf4CO7\n" - + "0045htNubGs5ZMwvD6sLYqKAnrM3WxV/2TbbjzjZW6Jkidz3jz/WRT4SmjGYiEO7aA+V\n" - + "00454ZdIS9f7sW5VsHHYlNThCA7vH8Uu48bUovFXyQlPTX0pToSgrWV3JnTxDNxfn3iG\n" - + "0045IL0zTY/qwVCdXgFownLcs6J050xrrBWIKqfcWr3u4D2aCLyR0v+S/KArr7ulZygY\n" - + "0045+SOklImn8TAZiNxhWtA6ens66IiammUkZYFv7SSzoPLFZT4dC84SmGPWgf94NoQ=\n" - + "000a=XFeC\n" - + "0020-----END PGP SIGNATURE-----\n" - + "0012push-cert-end\n"; + // Example push certificate generated by C git 2.2.0. + private static final String INPUT = "001ccertificate version 0.1\n" + + "0041pusher Dave Borowitz 1433954361 -0700\n" + + "0024pushee git://localhost/repo.git\n" + + "002anonce 1433954361-bde756572d665bba81d8\n" + + "0005\n" + + "00680000000000000000000000000000000000000000" + + " 6c2b981a177396fb47345b7df3e4d3f854c6bea7" + + " refs/heads/master\n" + + "0022-----BEGIN PGP SIGNATURE-----\n" + + "0016Version: GnuPG v1\n" + + "0005\n" + + "0045iQEcBAABAgAGBQJVeGg5AAoJEPfTicJkUdPkUggH/RKAeI9/i/LduuiqrL/SSdIa\n" + + "00459tYaSqJKLbXz63M/AW4Sp+4u+dVCQvnAt/a35CVEnpZz6hN4Kn/tiswOWVJf4CO7\n" + + "0045htNubGs5ZMwvD6sLYqKAnrM3WxV/2TbbjzjZW6Jkidz3jz/WRT4SmjGYiEO7aA+V\n" + + "00454ZdIS9f7sW5VsHHYlNThCA7vH8Uu48bUovFXyQlPTX0pToSgrWV3JnTxDNxfn3iG\n" + + "0045IL0zTY/qwVCdXgFownLcs6J050xrrBWIKqfcWr3u4D2aCLyR0v+S/KArr7ulZygY\n" + + "0045+SOklImn8TAZiNxhWtA6ens66IiammUkZYFv7SSzoPLFZT4dC84SmGPWgf94NoQ=\n" + + "000a=XFeC\n" + + "0020-----END PGP SIGNATURE-----\n" + + "0012push-cert-end\n"; - PacketLineIn pckIn = newPacketLineIn(input); + private Repository db; + + @Before + public void setUp() { + db = new InMemoryRepository(new DfsRepositoryDescription("repo")); + } + + private static SignedPushConfig newEnabledConfig() { Config cfg = new Config(); cfg.setString("receive", null, "certnonceseed", "sekret"); - Repository db = new InMemoryRepository( - new DfsRepositoryDescription("repo")); + return SignedPushConfig.KEY.parse(cfg); + } + + private static SignedPushConfig newDisabledConfig() { + return SignedPushConfig.KEY.parse(new Config()); + } + + @Test + public void noCert() throws Exception { + PushCertificateParser parser = + new PushCertificateParser(db, newEnabledConfig()); + assertTrue(parser.enabled()); + assertNull(parser.build()); + + ObjectId oldId = ObjectId.zeroId(); + ObjectId newId = + ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"); + String rawLine = + oldId.name() + " " + newId.name() + " refs/heads/master"; + ReceiveCommand cmd = BaseReceivePack.parseCommand(rawLine); + + parser.addCommand(cmd, rawLine); + parser.addCommand(rawLine); + assertNull(parser.build()); + } - PushCertificateParser parser = new PushCertificateParser( - db, new SignedPushConfig(cfg)); + @Test + public void disabled() throws Exception { + PacketLineIn pckIn = newPacketLineIn(INPUT); + PushCertificateParser parser = + new PushCertificateParser(db, newDisabledConfig()); + assertFalse(parser.enabled()); + assertNull(parser.build()); + + parser.receiveHeader(pckIn, false); + parser.addCommand(pckIn.readStringRaw()); + assertEquals(PushCertificateParser.BEGIN_SIGNATURE, pckIn.readStringRaw()); + parser.receiveSignature(pckIn); + assertNull(parser.build()); + } + + @Test + public void disabledParserStillRequiresCorrectSyntax() throws Exception { + PacketLineIn pckIn = newPacketLineIn("001ccertificate version XYZ\n"); + PushCertificateParser parser = + new PushCertificateParser(db, newDisabledConfig()); + assertFalse(parser.enabled()); + try { + parser.receiveHeader(pckIn, false); + fail("Expected PackProtocolException"); + } catch (PackProtocolException e) { + assertEquals( + "Push certificate has missing or invalid value for certificate" + + " version: XYZ", + e.getMessage()); + } + assertNull(parser.build()); + } + + @Test + public void parseCertFromPktLine() throws Exception { + PacketLineIn pckIn = newPacketLineIn(INPUT); + PushCertificateParser parser = + new PushCertificateParser(db, newEnabledConfig()); parser.receiveHeader(pckIn, false); parser.addCommand(pckIn.readStringRaw()); assertEquals(PushCertificateParser.BEGIN_SIGNATURE, pckIn.readStringRaw()); @@ -117,9 +186,9 @@ public class PushCertificateParserTest { assertEquals("6c2b981a177396fb47345b7df3e4d3f854c6bea7", cmd.getNewId().name()); - assertEquals(concatPacketLines(input, 0, 6), cert.toText()); + assertEquals(concatPacketLines(INPUT, 0, 6), cert.toText()); - String signature = concatPacketLines(input, 6, 17); + String signature = concatPacketLines(INPUT, 6, 17); assertTrue(signature.startsWith(PushCertificateParser.BEGIN_SIGNATURE)); assertTrue(signature.endsWith(PushCertificateParser.END_SIGNATURE)); assertEquals(signature, cert.getSignature()); diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java index 54eb747e9..37e5d3cd3 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java @@ -260,7 +260,8 @@ public abstract class BaseReceivePack { *

* Only valid after commands are read from the wire. * - * @return the parsed certificate, or null if push certificates are disabled. + * @return the parsed certificate, or null if push certificates are disabled + * or no cert was presented by the client. * @throws IOException if the certificate was present but invalid. * @since 4.1 */ diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateParser.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateParser.java index cb6a8598b..04871c7f9 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateParser.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateParser.java @@ -82,6 +82,7 @@ public class PushCertificateParser { private static final String END_CERT = "push-cert-end\n"; //$NON-NLS-1$ + private boolean received; private String version; private PushCertificateIdent pusher; private String pushee; @@ -135,7 +136,7 @@ public class PushCertificateParser { * @since 4.1 */ public PushCertificate build() throws IOException { - if (nonceGenerator == null) { + if (!received || nonceGenerator == null) { return null; } try { @@ -210,6 +211,7 @@ public class PushCertificateParser { */ public void receiveHeader(PacketLineIn pckIn, boolean stateless) throws IOException { + received = true; try { version = parseHeader(pckIn, VERSION); if (!version.equals(VERSION_0_1)) { @@ -255,6 +257,7 @@ public class PushCertificateParser { * @since 4.0 */ public void receiveSignature(PacketLineIn pckIn) throws IOException { + received = true; try { StringBuilder sig = new StringBuilder(BEGIN_SIGNATURE); String line;