Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

x/crypto/openpgp/clearsign: CRLF line endings not preserved in plaintext #43437

Closed
benprew opened this issue Dec 30, 2020 · 4 comments
Closed

x/crypto/openpgp/clearsign: CRLF line endings not preserved in plaintext #43437

benprew opened this issue Dec 30, 2020 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@benprew
Copy link

benprew commented Dec 30, 2020

What version of Go are you using (go version)?

$ go version
go version go1.15.5 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

GO111MODULE=""

GOARCH="amd64"

GOBIN=""

GOCACHE="/Users/bprew/Library/Caches/go-build"

GOENV="/Users/bprew/Library/Application Support/go/env"

GOEXE=""

GOFLAGS=""

GOHOSTARCH="amd64"

GOHOSTOS="darwin"

GOINSECURE=""

GOMODCACHE="/Users/bprew/go/pkg/mod"

GONOPROXY=""

GONOSUMDB=""

GOOS="darwin"

GOPATH="/Users/bprew/go"

GOPRIVATE=""

GOPROXY="https://proxy.golang.org,direct"

GOROOT="/usr/local/Cellar/go/1.15.5/libexec"

GOSUMDB="sum.golang.org"

GOTMPDIR=""

GOTOOLDIR="/usr/local/Cellar/go/1.15.5/libexec/pkg/tool/darwin_amd64"

GCCGO="gccgo"

AR="ar"

CC="clang"

CXX="clang++"

CGO_ENABLED="1"

GOMOD=""

CGO_CFLAGS="-g -O2"

CGO_CPPFLAGS=""

CGO_CXXFLAGS="-g -O2"

CGO_FFLAGS="-g -O2"

CGO_LDFLAGS="-g -O2"

PKG_CONFIG="pkg-config"

GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/v2/83cnrrx13rn5q7l3qj_vkk4sc90dkq/T/go-build010582822=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Hi, I'm a new Go user and this is my first issue, apologies in advance if I missed anything.

I'm trying to preserve CRLF line endings in a clearsigned message.

openpgp/clearsign.Encode strips out trailing whitespace and the line ending and replaces it with CRLF when passing it to the hasher (which matches RFC-4880). But for the plaintext, it replaces it with LF (which I didn't see specified in the RFC). I would like the CRLF to persist in the plaintext.

(Note: keeping CRLF in the clearsigned message matches the behavior of gpg 2.2.24)

Here's an example of the behavior:

https://play.golang.org/p/EC_S61QqbSJ

Also, I have a change that preserves trailing whitespace and line endings in the plaintext, and am happy to submit a PR to fix this behavior.

What did you expect to see?

I expected to see CRLF line endings in the clearsigned message.

What did you see instead?

The clearsigned message used LF as line endings.
@gopherbot gopherbot added this to the Unreleased milestone Dec 30, 2020
@benprew
Copy link
Author

benprew commented Dec 30, 2020

Also, here's the proposed change:

diff --git a/openpgp/clearsign/clearsign.go b/openpgp/clearsign/clearsign.go
index c360460..fdd99dc 100644
--- a/openpgp/clearsign/clearsign.go
+++ b/openpgp/clearsign/clearsign.go
@@ -253,14 +253,16 @@ func (d *dashEscaper) Write(data []byte) (n int, err error) {
 			}
 		} else {
 			if b == '\n' {
-				// We got a raw \n. Drop any trailing whitespace and write a
-				// CRLF.
+				// Write whitespace to buffer
+				d.whitespace = append(d.whitespace, b)
+				if _, err = d.buffered.Write(d.whitespace); err != nil {
+					return
+				}
+
+				// Drop any trailing whitespace for the hashers.
 				d.whitespace = d.whitespace[:0]
 				// We delay writing CRLF to the hash until the start of the
 				// next line.
-				if err = d.buffered.WriteByte(b); err != nil {
-					return
-				}
 				d.atBeginningOfLine = true
 			} else {
 				// Any buffered whitespace wasn't at the end of the line so

@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 5, 2021
@toothrot
Copy link
Contributor

toothrot commented Jan 5, 2021

/cc @FiloSottile

@gopherbot
Copy link

Change https://golang.org/cl/282173 mentions this issue: io/fs: minor corrections to Glob doc

@FiloSottile
Copy link
Contributor

Per the accepted #44226 proposal and due to lack of maintenance, the golang.org/x/crypto/openpgp package is now frozen and deprecated. No new changes will be accepted except for security fixes. The package will not be removed.

If this is a security issue, please email security@golang.org and we will assess it and provide a fix.

If you're looking for alternatives, consider the crypto/ed25519 package for simple signatures, golang.org/x/mod/sumdb/note for inline signatures, or filippo.io/age for encryption. You can read a summary of OpenPGP issues and alternatives here.

If you are required to interoperate with OpenPGP systems and need a maintained package, we suggest considering one of multiple community forks of golang.org/x/crypto/openpgp. We don't endorse any specific one.

Thank you!

@golang golang locked and limited conversation to collaborators Mar 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@toothrot @benprew @FiloSottile @gopherbot