From d43703624ce4ac3379a4632b3dbf1049cd96c918 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Wed, 10 Jun 2015 12:47:04 -0700 Subject: [PATCH 1/2] Allow trailing newlines in receive-pack C git's receive-pack.c strips trailing newlines in command lists when present[1], although send-pack.c does not send them, at least in the case of command lists[2]. Change JGit to match this behavior. Add tests. This also fixes parsing of commands in the push cert, which, unlike commands sent in the non-push case, always have trailing newlines. [1] https://github.com/git/git/blob/7974889a053574e449b55ca543a486e38e74864f/builtin/receive-pack.c#L1380 where packet_read_line chomps newlines: https://github.com/git/git/blob/7974889a053574e449b55ca543a486e38e74864f/pkt-line.c#L202 [2] https://github.com/git/git/blob/7974889a053574e449b55ca543a486e38e74864f/send-pack.c#L470 Change-Id: I4bca6342a7482a53c9a5815a94b3c181a479d04b --- .../jgit/transport/BaseReceivePackTest.java | 70 +++++++++++++++++++ .../jgit/transport/BaseReceivePack.java | 37 +++++++--- 2 files changed, 97 insertions(+), 10 deletions(-) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BaseReceivePackTest.java diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BaseReceivePackTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BaseReceivePackTest.java new file mode 100644 index 000000000..98164d933 --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/BaseReceivePackTest.java @@ -0,0 +1,70 @@ +/* + * Copyright (C) 2015, Google Inc. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.eclipse.jgit.transport; + +import static org.junit.Assert.assertEquals; + +import org.eclipse.jgit.lib.ObjectId; +import org.junit.Test; + +/** Tests for base receive-pack utilities. */ +public class BaseReceivePackTest { + @Test + public void chomp() { + assertEquals("foo", BaseReceivePack.chomp("foo")); + assertEquals("foo", BaseReceivePack.chomp("foo\n")); + assertEquals("foo\n", BaseReceivePack.chomp("foo\n\n")); + } + + @Test + public void parseCommand() { + String input = "0000000000000000000000000000000000000000" + + " deadbeefdeadbeefdeadbeefdeadbeefdeadbeef" + + " refs/heads/master"; + ReceiveCommand cmd = BaseReceivePack.parseCommand(input); + assertEquals(ObjectId.zeroId(), cmd.getOldId()); + assertEquals("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef", + cmd.getNewId().name()); + assertEquals("refs/heads/master", cmd.getRefName()); + } +} 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 9112ecbe3..7a1757722 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java @@ -1037,16 +1037,18 @@ public abstract class BaseReceivePack { */ protected void recvCommands() throws IOException { for (;;) { - String line; + String rawLine; try { - line = pckIn.readStringRaw(); + rawLine = pckIn.readStringRaw(); } catch (EOFException eof) { if (commands.isEmpty()) return; throw eof; } - if (line == PacketLineIn.END) + if (rawLine == PacketLineIn.END) { break; + } + String line = chomp(rawLine); if (line.length() >= 48 && line.startsWith("shallow ")) { //$NON-NLS-1$ clientShallowCommits.add(ObjectId.fromString(line.substring(8, 48))); @@ -1066,8 +1068,11 @@ public abstract class BaseReceivePack { if (line.equals("-----BEGIN PGP SIGNATURE-----\n")) //$NON-NLS-1$ pushCertificateParser.receiveSignature(pckIn); - if (pushCertificateParser.enabled()) - pushCertificateParser.addCommand(line); + if (pushCertificateParser.enabled()) { + // Must use raw line with optional newline so signed payload can be + // reconstructed. + pushCertificateParser.addCommand(rawLine); + } if (line.length() < 83) { final String m = JGitText.get().errorInvalidProtocolWantedOldNewRef; @@ -1075,11 +1080,8 @@ public abstract class BaseReceivePack { throw new PackProtocolException(m); } - final ObjectId oldId = ObjectId.fromString(line.substring(0, 40)); - final ObjectId newId = ObjectId.fromString(line.substring(41, 81)); - final String name = line.substring(82); - final ReceiveCommand cmd = new ReceiveCommand(oldId, newId, name); - if (name.equals(Constants.HEAD)) { + final ReceiveCommand cmd = parseCommand(line); + if (cmd.getRefName().equals(Constants.HEAD)) { cmd.setResult(Result.REJECTED_CURRENT_BRANCH); } else { cmd.setRef(refs.get(cmd.getRefName())); @@ -1088,6 +1090,21 @@ public abstract class BaseReceivePack { } } + static String chomp(String line) { + if (line != null && !line.isEmpty() + && line.charAt(line.length() - 1) == '\n') { + return line.substring(0, line.length() - 1); + } + return line; + } + + static ReceiveCommand parseCommand(String line) { + ObjectId oldId = ObjectId.fromString(line.substring(0, 40)); + ObjectId newId = ObjectId.fromString(line.substring(41, 81)); + String name = line.substring(82); + return new ReceiveCommand(oldId, newId, name); + } + /** Enable capabilities based on a previously read capabilities line. */ protected void enableCapabilities() { sideBand = isCapabilityEnabled(CAPABILITY_SIDE_BAND_64K); From a85e817dc29a1d6a96beeb92383aa265b0303415 Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Tue, 9 Jun 2015 17:23:03 -0700 Subject: [PATCH 2/2] Rewrite push certificate parsing - Consistently return structured data, such as actual ReceiveCommands, which is more useful for callers that are doing things other than verifying the signature, e.g. recording the set of commands. - Store the certificate version field, as this is required to be part of the signed payload. - Add a toText() method to recreate the actual payload for signature verification. This requires keeping track of the un-chomped command strings from the original protocol stream. - Separate the parser from the certificate itself, so the actual PushCertificate object can be immutable. Make a fair attempt at deep immutability, but this is not possible with the current mutable ReceiveCommand structure. - Use more detailed error messages that don't involve NON-NLS strings. - Document null return values more thoroughly. Instead of having the undocumented behavior of throwing NPE from certain methods if they are not first guarded by enabled(), eliminate enabled() and return null from those methods. - Add tests for parsing a push cert from a section of pkt-line stream using a real live stream captured with Wireshark (which, it should be noted, uncovered several simply incorrect statements in C git's Documentation/technical/pack-protocol.txt). This is a slightly breaking API change to classes that were technically public and technically released in 4.0. However, it is highly unlikely that people were actually depending on public behavior, since there were no public methods to create PushCertificates with anything other than null field values, or a PushCertificateParser that did anything other than infinite loop or throw exceptions when reading. Change-Id: I5382193347a8eb1811032d9b32af9651871372d0 --- .../transport/PushCertificateParserTest.java | 160 ++++++++++++++ .../eclipse/jgit/internal/JGitText.properties | 5 +- .../org/eclipse/jgit/internal/JGitText.java | 5 +- .../jgit/transport/BaseReceivePack.java | 33 +-- .../jgit/transport/PushCertificate.java | 150 ++++++++++--- .../jgit/transport/PushCertificateParser.java | 205 +++++++++++++----- 6 files changed, 462 insertions(+), 96 deletions(-) create mode 100644 org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushCertificateParserTest.java 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 new file mode 100644 index 000000000..1308fab19 --- /dev/null +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/PushCertificateParserTest.java @@ -0,0 +1,160 @@ +/* + * Copyright (C) 2015, Google Inc. + * + * This program and the accompanying materials are made available + * under the terms of the Eclipse Distribution License v1.0 which + * accompanies this distribution, is reproduced below, and is + * available at http://www.eclipse.org/org/documents/edl-v10.php + * + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or + * without modification, are permitted provided that the following + * conditions are met: + * + * - Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * - Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following + * disclaimer in the documentation and/or other materials provided + * with the distribution. + * + * - Neither the name of the Eclipse Foundation, Inc. nor the + * names of its contributors may be used to endorse or promote + * products derived from this software without specific prior + * written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND + * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, + * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER + * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, + * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF + * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + */ + +package org.eclipse.jgit.transport; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; + +import java.io.ByteArrayInputStream; +import java.io.EOFException; +import java.io.IOException; + +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.eclipse.jgit.transport.BaseReceivePack.ReceiveConfig; +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"; + + PacketLineIn pckIn = newPacketLineIn(input); + Config cfg = new Config(); + cfg.setString("receive", null, "certnonceseed", "sekret"); + Repository db = new InMemoryRepository( + new DfsRepositoryDescription("repo")); + + PushCertificateParser parser = new PushCertificateParser( + db, new ReceiveConfig(cfg)); + parser.receiveHeader(pckIn, false); + parser.addCommand(pckIn.readStringRaw()); + assertEquals(PushCertificateParser.BEGIN_SIGNATURE, pckIn.readStringRaw()); + parser.receiveSignature(pckIn); + + PushCertificate cert = parser.build(); + assertEquals("0.1", cert.getVersion()); + assertEquals("Dave Borowitz", cert.getPusherIdent().getName()); + assertEquals("dborowitz@google.com", + cert.getPusherIdent().getEmailAddress()); + assertEquals(1433954361000L, cert.getPusherIdent().getWhen().getTime()); + assertEquals(-7 * 60, cert.getPusherIdent().getTimeZoneOffset()); + assertEquals("git://localhost/repo.git", cert.getPushee()); + assertEquals("1433954361-bde756572d665bba81d8", cert.getNonce()); + + assertNotEquals(cert.getNonce(), parser.getAdvertiseNonce()); + assertEquals(PushCertificate.NonceStatus.BAD, cert.getNonceStatus()); + + assertEquals(1, cert.getCommands().size()); + ReceiveCommand cmd = cert.getCommands().get(0); + assertEquals("refs/heads/master", cmd.getRefName()); + assertEquals(ObjectId.zeroId(), cmd.getOldId()); + assertEquals("6c2b981a177396fb47345b7df3e4d3f854c6bea7", + cmd.getNewId().name()); + + assertEquals(concatPacketLines(input, 0, 6), cert.toText()); + + String signature = concatPacketLines(input, 7, 16); + assertFalse(signature.contains(PushCertificateParser.BEGIN_SIGNATURE)); + assertFalse(signature.contains(PushCertificateParser.END_SIGNATURE)); + assertEquals(signature, cert.getSignature()); + } + + @Test + public void testConcatPacketLines() throws Exception { + String input = "000bline 1\n000bline 2\n000bline 3\n"; + assertEquals("line 1\n", concatPacketLines(input, 0, 1)); + assertEquals("line 1\nline 2\n", concatPacketLines(input, 0, 2)); + assertEquals("line 2\nline 3\n", concatPacketLines(input, 1, 3)); + assertEquals("line 2\nline 3\n", concatPacketLines(input, 1, 4)); + } + + private static String concatPacketLines(String input, int begin, int end) + throws IOException { + StringBuilder result = new StringBuilder(); + int i = 0; + PacketLineIn pckIn = newPacketLineIn(input); + while (i < end) { + String line; + try { + line = pckIn.readStringRaw(); + } catch (EOFException e) { + break; + } + if (++i > begin) { + result.append(line); + } + } + return result.toString(); + } + + private static PacketLineIn newPacketLineIn(String input) { + return new PacketLineIn(new ByteArrayInputStream(Constants.encode(input))); + } +} diff --git a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties index dbe5973a7..509027daf 100644 --- a/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties +++ b/org.eclipse.jgit/resources/org/eclipse/jgit/internal/JGitText.properties @@ -237,7 +237,6 @@ errorDecodingFromFile=Error decoding from file {0} errorEncodingFromFile=Error encoding from file {0} errorInBase64CodeReadingStream=Error in Base64 code reading stream. errorInPackedRefs=error in packed-refs -errorInvalidPushCert=error: invalid protocol: {0} errorInvalidProtocolWantedOldNewRef=error: invalid protocol: wanted 'old new ref' errorListing=Error listing {0} errorOccurredDuringUnpackingOnTheRemoteEnd=error occurred during unpacking on the remote end: {0} @@ -473,6 +472,10 @@ pruneLooseUnreferencedObjects=Prune loose, unreferenced objects pullOnRepoWithoutHEADCurrentlyNotSupported=Pull on repository without HEAD currently not supported pullTaskName=Pull pushCancelled=push cancelled +pushCertificateInvalidField=Push certificate has missing or invalid value for {0} +pushCertificateInvalidFieldValue=Push certificate has missing or invalid value for {0}: {1} +pushCertificateInvalidHeader=Push certificate has invalid header format +pushCertificateInvalidSignature=Push certificate has invalid signature format pushIsNotSupportedForBundleTransport=Push is not supported for bundle transport pushNotPermitted=push not permitted rawLogMessageDoesNotParseAsLogEntry=Raw log message does not parse as log entry diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java index 494114f70..f903f2337 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/JGitText.java @@ -296,7 +296,6 @@ public class JGitText extends TranslationBundle { /***/ public String errorEncodingFromFile; /***/ public String errorInBase64CodeReadingStream; /***/ public String errorInPackedRefs; - /***/ public String errorInvalidPushCert; /***/ public String errorInvalidProtocolWantedOldNewRef; /***/ public String errorListing; /***/ public String errorOccurredDuringUnpackingOnTheRemoteEnd; @@ -532,6 +531,10 @@ public class JGitText extends TranslationBundle { /***/ public String pullOnRepoWithoutHEADCurrentlyNotSupported; /***/ public String pullTaskName; /***/ public String pushCancelled; + /***/ public String pushCertificateInvalidField; + /***/ public String pushCertificateInvalidFieldValue; + /***/ public String pushCertificateInvalidHeader; + /***/ public String pushCertificateInvalidSignature; /***/ public String pushIsNotSupportedForBundleTransport; /***/ public String pushNotPermitted; /***/ public String rawLogMessageDoesNotParseAsLogEntry; 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 7a1757722..111a22734 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/BaseReceivePack.java @@ -252,13 +252,19 @@ public abstract class BaseReceivePack { /** The size of the received pack, including index size */ private Long packSize; - PushCertificateParser pushCertificateParser; + private PushCertificateParser pushCertificateParser; /** - * @return the push certificate used to verify the pushers identity. + * Get the push certificate used to verify the pusher's identity. + *

+ * Only valid after commands are read from the wire. + * + * @return the parsed certificate, or null if push certificates are disabled. + * @throws IOException if the certificate was present but invalid. + * @since 4.1 */ - PushCertificate getPushCertificate() { - return pushCertificateParser; + public PushCertificate getPushCertificate() throws IOException { + return pushCertificateParser.build(); } /** @@ -1014,9 +1020,10 @@ public abstract class BaseReceivePack { adv.advertiseCapability(CAPABILITY_REPORT_STATUS); if (allowQuiet) adv.advertiseCapability(CAPABILITY_QUIET); - if (pushCertificateParser.enabled()) - adv.advertiseCapability( - pushCertificateParser.getAdvertiseNonce()); + String nonce = pushCertificateParser.getAdvertiseNonce(); + if (nonce != null) { + adv.advertiseCapability(nonce); + } if (db.getRefDatabase().performsAtomicTransactions()) adv.advertiseCapability(CAPABILITY_ATOMIC); if (allowOfsDelta) @@ -1065,13 +1072,8 @@ public abstract class BaseReceivePack { !isBiDirectionalPipe()); } - if (line.equals("-----BEGIN PGP SIGNATURE-----\n")) //$NON-NLS-1$ + if (line.equals(PushCertificateParser.BEGIN_SIGNATURE)) { pushCertificateParser.receiveSignature(pckIn); - - if (pushCertificateParser.enabled()) { - // Must use raw line with optional newline so signed payload can be - // reconstructed. - pushCertificateParser.addCommand(rawLine); } if (line.length() < 83) { @@ -1087,6 +1089,11 @@ public abstract class BaseReceivePack { cmd.setRef(refs.get(cmd.getRefName())); } commands.add(cmd); + if (pushCertificateParser.enabled()) { + // Must use raw line with optional newline so signed payload can be + // reconstructed. + pushCertificateParser.addCommand(cmd, rawLine); + } } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificate.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificate.java index 8ee4c17bf..2eda2b713 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificate.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificate.java @@ -43,21 +43,26 @@ package org.eclipse.jgit.transport; +import static org.eclipse.jgit.transport.PushCertificateParser.NONCE; +import static org.eclipse.jgit.transport.PushCertificateParser.PUSHEE; +import static org.eclipse.jgit.transport.PushCertificateParser.PUSHER; +import static org.eclipse.jgit.transport.PushCertificateParser.VERSION; + +import java.text.MessageFormat; +import java.util.List; + +import org.eclipse.jgit.internal.JGitText; +import org.eclipse.jgit.lib.PersonIdent; + /** * The required information to verify the push. + *

+ * A valid certificate will not return null from any getter methods; callers may + * assume that any null value indicates a missing or invalid certificate. * * @since 4.0 */ public class PushCertificate { - /** The tuple "name <email>" as presented in the push certificate. */ - String pusher; - - /** The remote URL the signed push goes to. */ - String pushee; - - /** What we think about the returned signed nonce. */ - NonceStatus nonceStatus; - /** Verification result of the nonce returned during push. */ public enum NonceStatus { /** Nonce was not expected, yet client sent one anyway. */ @@ -72,41 +77,136 @@ public class PushCertificate { SLOP } - String commandList; - String signature; + private final String version; + private final PersonIdent pusher; + private final String pushee; + private final String nonce; + private final NonceStatus nonceStatus; + private final List commands; + private final String rawCommands; + private final String signature; + + PushCertificate(String version, PersonIdent pusher, String pushee, + String nonce, NonceStatus nonceStatus, List commands, + String rawCommands, String signature) { + if (version == null || version.isEmpty()) { + throw new IllegalArgumentException(MessageFormat.format( + JGitText.get().pushCertificateInvalidField, VERSION)); + } + if (pusher == null) { + throw new IllegalArgumentException(MessageFormat.format( + JGitText.get().pushCertificateInvalidField, PUSHER)); + } + if (pushee == null || pushee.isEmpty()) { + throw new IllegalArgumentException(MessageFormat.format( + JGitText.get().pushCertificateInvalidField, PUSHEE)); + } + if (nonce == null || nonce.isEmpty()) { + throw new IllegalArgumentException(MessageFormat.format( + JGitText.get().pushCertificateInvalidField, NONCE)); + } + if (nonceStatus == null) { + throw new IllegalArgumentException(MessageFormat.format( + JGitText.get().pushCertificateInvalidField, + "nonce status")); //$NON-NLS-1$ + } + if (commands == null || commands.isEmpty() + || rawCommands == null || rawCommands.isEmpty()) { + throw new IllegalArgumentException(MessageFormat.format( + JGitText.get().pushCertificateInvalidField, + "command")); //$NON-NLS-1$ + } + if (signature == null || signature.isEmpty()) { + throw new IllegalArgumentException( + JGitText.get().pushCertificateInvalidSignature); + } + this.version = version; + this.pusher = pusher; + this.pushee = pushee; + this.nonce = nonce; + this.nonceStatus = nonceStatus; + this.commands = commands; + this.rawCommands = rawCommands; + this.signature = signature; + } /** - * @return the signature, consisting of the lines received between the lines - * '----BEGIN GPG SIGNATURE-----\n' and the '----END GPG - * SIGNATURE-----\n' + * @return the certificate version string. + * @since 4.1 */ - public String getSignature() { - return signature; + public String getVersion() { + return version; } /** - * @return the list of commands as one string to be feed into the signature - * verifier. + * @return the identity of the pusher who signed the cert, as a string. + * @since 4.0 */ - public String getCommandList() { - return commandList; + public String getPusher() { + return pusher.toExternalString(); } /** - * @return the tuple "name <email>" as presented by the client in the - * push certificate. + * @return identity of the pusher who signed the cert. + * @since 4.1 */ - public String getPusher() { + public PersonIdent getPusherIdent() { return pusher; } - /** @return URL of the repository the push was originally sent to. */ + /** + * @return URL of the repository the push was originally sent to. + * @since 4.0 + */ public String getPushee() { return pushee; } - /** @return verification status of the nonce embedded in the certificate. */ + /** + * @return the raw nonce value that was presented by the pusher. + * @since 4.0 + */ + public String getNonce() { + return nonce; + } + + /** + * @return verification status of the nonce embedded in the certificate. + * @since 4.0 + */ public NonceStatus getNonceStatus() { return nonceStatus; } + + /** + * @return the list of commands as one string to be feed into the signature + * verifier. + * @since 4.1 + */ + public List getCommands() { + return commands; + } + + /** + * @return the raw signature, consisting of the lines received between the + * lines {@code "----BEGIN GPG SIGNATURE-----\n"} and + * {@code "----END GPG SIGNATURE-----\n}", exclusive + * @since 4.0 + */ + public String getSignature() { + return signature; + } + + /** @return text payload of the certificate for the signature verifier. */ + public String toText() { + return new StringBuilder() + .append(VERSION).append(' ').append(version).append('\n') + .append(PUSHER).append(' ').append(pusher.toExternalString()) + .append('\n') + .append(PUSHEE).append(' ').append(pushee).append('\n') + .append(NONCE).append(' ').append(nonce).append('\n') + .append('\n') + .append(rawCommands) + .toString(); + } } 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 4bb3d6bf8..fc7c19bfa 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateParser.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/PushCertificateParser.java @@ -42,56 +42,78 @@ */ package org.eclipse.jgit.transport; +import static org.eclipse.jgit.transport.BaseReceivePack.chomp; +import static org.eclipse.jgit.transport.BaseReceivePack.parseCommand; import static org.eclipse.jgit.transport.GitProtocolConstants.CAPABILITY_PUSH_CERT; import java.io.EOFException; import java.io.IOException; import java.text.MessageFormat; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.concurrent.TimeUnit; import org.eclipse.jgit.internal.JGitText; +import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.transport.BaseReceivePack.ReceiveConfig; +import org.eclipse.jgit.transport.PushCertificate.NonceStatus; +import org.eclipse.jgit.util.RawParseUtils; /** - * Parser for Push certificates + * Parser for signed push certificates. * * @since 4.0 */ -public class PushCertificateParser extends PushCertificate { +public class PushCertificateParser { + static final String BEGIN_SIGNATURE = + "-----BEGIN PGP SIGNATURE-----\n"; //$NON-NLS-1$ + static final String END_SIGNATURE = + "-----END PGP SIGNATURE-----\n"; //$NON-NLS-1$ - private static final String VERSION = "version "; //$NON-NLS-1$ + static final String VERSION = "certificate version"; //$NON-NLS-1$ - private static final String PUSHER = "pusher"; //$NON-NLS-1$ + static final String PUSHER = "pusher"; //$NON-NLS-1$ - private static final String PUSHEE = "pushee"; //$NON-NLS-1$ + static final String PUSHEE = "pushee"; //$NON-NLS-1$ - private static final String NONCE = "nonce"; //$NON-NLS-1$ + static final String NONCE = "nonce"; //$NON-NLS-1$ - /** The individual certificate which is presented to the client */ + private static final String VERSION_0_1 = "0.1"; //$NON-NLS-1$ + + private static final String END_CERT = "push-cert-end\n"; //$NON-NLS-1$ + + private String version; + private PersonIdent pusher; + private String pushee; + + /** The nonce that was sent to the client. */ private String sentNonce; /** - * The nonce the pusher signed. This may vary from pushCertNonce See - * git-core documentation for reasons. + * The nonce the pusher signed. + *

+ * This may vary from {@link #sentNonce}; see git-core documentation for + * reasons. */ private String receivedNonce; + private NonceStatus nonceStatus; + private String signature; + + /** Database we write the push certificate into. */ + private final Repository db; + /** * The maximum time difference which is acceptable between advertised nonce * and received signed nonce. */ - private int nonceSlopLimit; + private final int nonceSlopLimit; - NonceGenerator nonceGenerator; - - /** - * used to build up commandlist - */ - StringBuilder commandlistBuilder; - - /** Database we write the push certificate into. */ - private Repository db; + private final NonceGenerator nonceGenerator; + private final List commands; + private final StringBuilder rawCommands; PushCertificateParser(Repository into, ReceiveConfig cfg) { nonceSlopLimit = cfg.certNonceSlopLimit; @@ -99,10 +121,32 @@ public class PushCertificateParser extends PushCertificate { ? new HMACSHA1NonceGenerator(cfg.certNonceSeed) : null; db = into; + commands = new ArrayList<>(); + rawCommands = new StringBuilder(); + } + + /** + * @return the parsed certificate, or null if push certificates are disabled. + * @throws IOException + * if the push certificate has missing or invalid fields. + * @since 4.1 + */ + public PushCertificate build() throws IOException { + if (nonceGenerator == null) { + return null; + } + try { + return new PushCertificate(version, pusher, pushee, receivedNonce, + nonceStatus, Collections.unmodifiableList(commands), + rawCommands.toString(), signature); + } catch (IllegalArgumentException e) { + throw new IOException(e.getMessage(), e); + } } /** * @return if the server is configured to use signed pushes. + * @since 4.0 */ public boolean enabled() { return nonceGenerator != null; @@ -110,28 +154,43 @@ public class PushCertificateParser extends PushCertificate { /** * @return the whole string for the nonce to be included into the capability - * advertisement. + * advertisement, or null if push certificates are disabled. + * @since 4.0 */ public String getAdvertiseNonce() { - sentNonce = nonceGenerator.createNonce(db, - TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis())); - return CAPABILITY_PUSH_CERT + "=" + sentNonce; //$NON-NLS-1$ + String nonce = sentNonce(); + if (nonce == null) { + return null; + } + return CAPABILITY_PUSH_CERT + '=' + nonce; } - private String parseNextLine(PacketLineIn pckIn, String startingWith) + private String sentNonce() { + if (sentNonce == null && nonceGenerator != null) { + sentNonce = nonceGenerator.createNonce(db, + TimeUnit.MILLISECONDS.toSeconds(System.currentTimeMillis())); + } + return sentNonce; + } + + private static String parseHeader(PacketLineIn pckIn, String header) throws IOException { String s = pckIn.readString(); - if (!s.startsWith(startingWith)) + if (s.length() <= header.length() + || !s.startsWith(header) + || s.charAt(header.length()) != ' ') { throw new IOException(MessageFormat.format( - JGitText.get().errorInvalidPushCert, - "expected " + startingWith)); //$NON-NLS-1$ - return s.substring(startingWith.length()); + JGitText.get().pushCertificateInvalidHeader, header)); + } + return s.substring(header.length() + 1); } /** * Receive a list of commands from the input encapsulated in a push - * certificate. This method doesn't parse the first line "push-cert \NUL - * <capabilities>", but assumes the first line including the + * certificate. + *

+ * This method doesn't parse the first line {@code "push-cert \NUL + * <capabilities>"}, but assumes the first line including the * capabilities has already been handled by the caller. * * @param pckIn @@ -144,62 +203,96 @@ public class PushCertificateParser extends PushCertificate { * @throws IOException * if the certificate from the client is badly malformed or the * client disconnects before sending the entire certificate. + * @since 4.0 */ public void receiveHeader(PacketLineIn pckIn, boolean stateless) throws IOException { try { - String version = parseNextLine(pckIn, VERSION); - if (!version.equals("0.1")) { //$NON-NLS-1$ + version = parseHeader(pckIn, VERSION); + if (!version.equals(VERSION_0_1)) { throw new IOException(MessageFormat.format( - JGitText.get().errorInvalidPushCert, - "version not supported")); //$NON-NLS-1$ + JGitText.get().pushCertificateInvalidFieldValue, VERSION, version)); } - pusher = parseNextLine(pckIn, PUSHER); - pushee = parseNextLine(pckIn, PUSHEE); - receivedNonce = parseNextLine(pckIn, NONCE); - // an empty line - if (!pckIn.readString().isEmpty()) { + String pusherStr = parseHeader(pckIn, PUSHER); + pusher = RawParseUtils.parsePersonIdent(pusherStr); + if (pusher == null) { throw new IOException(MessageFormat.format( - JGitText.get().errorInvalidPushCert, - "expected empty line after header")); //$NON-NLS-1$ + JGitText.get().pushCertificateInvalidFieldValue, + PUSHER, pusherStr)); + } + pushee = parseHeader(pckIn, PUSHEE); + receivedNonce = parseHeader(pckIn, NONCE); + // An empty line. + if (!pckIn.readString().isEmpty()) { + throw new IOException( + JGitText.get().pushCertificateInvalidHeader); } } catch (EOFException eof) { - throw new IOException(MessageFormat.format( - JGitText.get().errorInvalidPushCert, - "broken push certificate header")); //$NON-NLS-1$ + throw new IOException( + JGitText.get().pushCertificateInvalidHeader, eof); } - nonceStatus = nonceGenerator.verify(receivedNonce, sentNonce, db, - stateless, nonceSlopLimit); + nonceStatus = nonceGenerator != null + ? nonceGenerator.verify( + receivedNonce, sentNonce(), db, stateless, nonceSlopLimit) + : NonceStatus.UNSOLICITED; } /** - * Reads the gpg signature. This method assumes the line "-----BEGIN PGP - * SIGNATURE-----\n" has already been parsed and continues parsing until an - * "-----END PGP SIGNATURE-----\n" is found. + * Read the PGP signature. + *

+ * This method assumes the line + * {@code "-----BEGIN PGP SIGNATURE-----\n"} has already been parsed, + * and continues parsing until an {@code "-----END PGP SIGNATURE-----\n"} is + * found, followed by {@code "push-cert-end\n"}. * * @param pckIn * where we read the signature from. * @throws IOException + * if the signature is invalid. + * @since 4.0 */ public void receiveSignature(PacketLineIn pckIn) throws IOException { try { StringBuilder sig = new StringBuilder(); - String line = pckIn.readStringRaw(); - while (!line.equals("-----END PGP SIGNATURE-----\n")) //$NON-NLS-1$ + String line; + while (!(line = pckIn.readStringRaw()).equals(END_SIGNATURE)) { sig.append(line); + } signature = sig.toString(); - commandList = commandlistBuilder.toString(); + if (!pckIn.readStringRaw().equals(END_CERT)) { + throw new IOException(JGitText.get().pushCertificateInvalidSignature); + } } catch (EOFException eof) { - throw new IOException(MessageFormat.format( - JGitText.get().errorInvalidPushCert, - "broken push certificate signature")); //$NON-NLS-1$ + throw new IOException( + JGitText.get().pushCertificateInvalidSignature, eof); } } /** + * Add a command to the signature. + * + * @param cmd + * the command. + * @param rawLine + * the exact line read from the wire that produced this + * command, including trailing newline if present. + * @since 4.1 + */ + public void addCommand(ReceiveCommand cmd, String rawLine) { + commands.add(cmd); + rawCommands.append(rawLine); + } + + /** + * Add a command to the signature. + * * @param rawLine + * the exact line read from the wire that produced this + * command, including trailing newline if present. + * @since 4.0 */ public void addCommand(String rawLine) { - commandlistBuilder.append(rawLine); + commands.add(parseCommand(chomp(rawLine))); + rawCommands.append(rawLine); } }