code review 128920043: go.crypto/openpgp: fix clearsigned parsing of messages ...

go.crypto/openpgp: fix clearsigned parsing of messages with leading blank lines.

Fixes issue 8508.

[agl1, 2014-08-11 23:05:46]

Hello bradfitz@golang.org (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go.crypto

[bradfitz, 2014-08-11 23:28:33]

LGTM
 On Aug 11, 2014 4:05 PM, <agl@golang.org> wrote:

> Reviewers: bradfitz,
>
> Message:
> Hello bradfitz@golang.org (cc: golang-codereviews@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go.crypto
>
>
> Description:
> go.crypto/openpgp: fix clearsigned parsing of messages with leading
> blank lines.
>
> Fixes issue 8508.
>
> Please review this at https://codereview.appspot.com/128920043/
>
> Affected files (+38, -8 lines):
>   M openpgp/clearsign/clearsign.go
>   M openpgp/clearsign/clearsign_test.go
>
>
> Index: openpgp/clearsign/clearsign.go
> ===================================================================
> --- a/openpgp/clearsign/clearsign.go
> +++ b/openpgp/clearsign/clearsign.go
> @@ -113,6 +113,7 @@
>                 b.Headers.Add(string(key), string(val))
>         }
>
> +       firstLine := true
>         for {
>                 start := rest
>
> @@ -126,9 +127,12 @@
>
>                 // The final CRLF isn't included in the hash so we don't
> write it until
>                 // we've seen the next line.
> -               if len(b.Bytes) > 0 {
> +               if firstLine {
> +                       firstLine = false
> +               } else {
>                         b.Bytes = append(b.Bytes, crlf...)
>                 }
> +
>                 if bytes.HasPrefix(line, dashEscape) {
>                         line = line[2:]
>                 }
> Index: openpgp/clearsign/clearsign_test.go
> ===================================================================
> --- a/openpgp/clearsign/clearsign_test.go
> +++ b/openpgp/clearsign/clearsign_test.go
> @@ -10,8 +10,8 @@
>         "testing"
>  )
>
> -func TestParse(t *testing.T) {
> -       b, rest := Decode(clearsignInput)
> +func testParse(t *testing.T, input []byte, expected, expectedPlaintext
> string) {
> +       b, rest := Decode(input)
>         if b == nil {
>                 t.Fatal("failed to decode clearsign message")
>         }
> @@ -21,14 +21,12 @@
>         if b.ArmoredSignature.Type != "PGP SIGNATURE" {
>                 t.Errorf("bad armor type, got:%s, want:PGP SIGNATURE",
> b.ArmoredSignature.Type)
>         }
> -       expected := []byte("Hello world\r\nline 2")
> -       if !bytes.Equal(b.Bytes, expected) {
> +       if !bytes.Equal(b.Bytes, []byte(expected)) {
>                 t.Errorf("bad body, got:%x want:%x", b.Bytes, expected)
>         }
>
> -       expected = []byte("Hello world\nline 2\n")
> -       if !bytes.Equal(b.Plaintext, expected) {
> -               t.Errorf("bad plaintext, got:%x want:%x", b.Plaintext,
> expected)
> +       if !bytes.Equal(b.Plaintext, []byte(expectedPlaintext)) {
> +               t.Errorf("bad plaintext, got:%x want:%x", b.Plaintext,
> expectedPlaintext)
>         }
>
>         keyring, err := openpgp.ReadArmoredKeyRing(bytes.NewBufferString(
> signingKey))
> @@ -41,6 +39,11 @@
>         }
>  }
>
> +func TestParse(t *testing.T) {
> +       testParse(t, clearsignInput, "Hello world\r\nline 2", "Hello
> world\nline 2\n")
> +       testParse(t, clearsignInput2, "\r\n\r\n(This message has a couple
> of blank lines at the start and end.)\r\n\r\n", "\n\n(This message has a
> couple of blank lines at the start and end.)\n\n\n")
> +}
> +
>  func TestParseWithNoNewlineAtEnd(t *testing.T) {
>         input := clearsignInput
>         input = input[:len(input)-len("trailing")-1]
> @@ -125,6 +128,29 @@
>  -----END PGP SIGNATURE-----
>  trailing`)
>
> +var clearsignInput2 = []byte(`
> +asdlfkjasdlkfjsadf
> +
> +-----BEGIN PGP SIGNED MESSAGE-----
> +Hash: SHA256
> +
> +
> +
> +(This message has a couple of blank lines at the start and end.)
> +
> +
> +-----BEGIN PGP SIGNATURE-----
> +Version: GnuPG v1.4.11 (GNU/Linux)
> +
> +iJwEAQEIAAYFAlPpSREACgkQO9o98PRieSpZTAP+M8QUoCt/7Rf3YbXPcdzIL32v
> +pt1I+cMNeopzfLy0u4ioEFi8s5VkwpL1AFmirvgViCwlf82inoRxzZRiW05JQ5LI
> +ESEzeCoy2LIdRCQ2hcrG8pIUPzUO4TqO5D/dMbdHwNH4h5nNmGJUAEG6FpURlPm+
> +qZg6BaTvOxepqOxnhVU=
> +=e+C6
> +-----END PGP SIGNATURE-----
> +
> +trailing`)
> +
>  var signingKey = `-----BEGIN PGP PRIVATE KEY BLOCK-----
>  Version: GnuPG v1.4.10 (GNU/Linux)
>
>
>
>

[bradfitz, 2014-08-28 15:42:46]

Forget to check this in?

[agl1, 2014-08-28 17:37:28]

*** Submitted as
https://code.google.com/p/go/source/detail?r=00a7d3b31bba&repo=crypto ***

go.crypto/openpgp: fix clearsigned parsing of messages with leading blank lines.

Fixes issue 8508.

LGTM=bradfitz
R=bradfitz
CC=golang-codereviews
https://codereview.appspot.com/128920043

[agl1, 2014-08-28 17:37:49]

On 2014/08/28 15:42:46, bradfitz wrote:
> Forget to check this in?

Yep, thanks.