openpgp/clearsign: Correctly handle whitespace-only lines clearsign.Encode currently creates bad signatures for inputs containing lines that that consist of only whitespace (gpg --verify and clearsign.Decode both agree the signature is bad). RFC 4880 section 7.1 says trailing whitespace is removed when generating the signature. The previous code correctly handled this for the case of not being at the beginning of the line by buffering the whitespace. The previous code had incorrect behavior for the case of being at the beginning of a line. It was only special-casing dashes and newlines, directly hashing all other characters. This happened to work for lines that had leading whitespace followed by non-whitespace characters, since in that case the leading whitespace is not trailing. However, this behavior is incorrect for whitespace-only lines: The previous code would incorrectly add the first whitespace character to the hash, when instead it should have been dropped. This commit moves the whitespace check so that it always happens, regardless of whether we are at the beginning of a line. This adds a few tests to capture the expected behavior. The last three tests fail without the included code change. Change-Id: I17848b8aaad6f7a4cee414d486be236f7edddd0b Reviewed-on: https://go-review.googlesource.com/13681 Reviewed-by: Adam Langley <agl@golang.org>
diff --git a/openpgp/clearsign/clearsign.go b/openpgp/clearsign/clearsign.go index c2aac09..6454d22 100644 --- a/openpgp/clearsign/clearsign.go +++ b/openpgp/clearsign/clearsign.go
@@ -35,7 +35,7 @@ // start is the marker which denotes the beginning of a clearsigned message. var start = []byte("\n-----BEGIN PGP SIGNED MESSAGE-----") -// dashEscape is prefixed to any lines that begin with a hypen so that they +// dashEscape is prefixed to any lines that begin with a hyphen so that they // can't be confused with endText. var dashEscape = []byte("- ") @@ -197,7 +197,17 @@ d.h.Write(crlf) } d.isFirstLine = false + } + // Any whitespace at the end of the line has to be removed so we + // buffer it until we find out whether there's more on this line. + if b == ' ' || b == '\t' || b == '\r' { + d.whitespace = append(d.whitespace, b) + d.atBeginningOfLine = false + continue + } + + if d.atBeginningOfLine { // At the beginning of a line, hyphens have to be escaped. if b == '-' { // The signature isn't calculated over the dash-escaped text so @@ -208,7 +218,7 @@ d.h.Write(d.byteBuf) d.atBeginningOfLine = false } else if b == '\n' { - // Nothing to do because we dely writing CRLF to the hash. + // Nothing to do because we delay writing CRLF to the hash. } else { d.h.Write(d.byteBuf) d.atBeginningOfLine = false @@ -217,15 +227,11 @@ return } } else { - // Any whitespace at the end of the line has to be removed so we - // buffer it until we find out whether there's more on this line. - if b == ' ' || b == '\t' || b == '\r' { - d.whitespace = append(d.whitespace, b) - } else if b == '\n' { + if b == '\n' { // We got a raw \n. Drop any trailing whitespace and write a // CRLF. d.whitespace = d.whitespace[:0] - // We dely writing CRLF to the hash until the start of the + // We delay writing CRLF to the hash until the start of the // next line. if err = d.buffered.WriteByte(b); err != nil { return
diff --git a/openpgp/clearsign/clearsign_test.go b/openpgp/clearsign/clearsign_test.go index 51538b9..406377c 100644 --- a/openpgp/clearsign/clearsign_test.go +++ b/openpgp/clearsign/clearsign_test.go
@@ -64,6 +64,16 @@ {"a\n", "a", "a\n"}, {"-a\n", "-a", "-a\n"}, {"--a\nb", "--a\r\nb", "--a\nb\n"}, + // leading whitespace + {" a\n", " a", " a\n"}, + {" a\n", " a", " a\n"}, + // trailing whitespace (should be stripped) + {"a \n", "a", "a\n"}, + {"a ", "a", "a\n"}, + // whitespace-only lines (should be stripped) + {" \n", "", "\n"}, + {" ", "", "\n"}, + {"a\n \n \nb\n", "a\r\n\r\n\r\nb", "a\n\n\nb\n"}, } func TestSigning(t *testing.T) {