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

crypto/x509: DecryptPEMBlock sometimes passes when the password is wrong #27880

Closed
azr opened this issue Sep 26, 2018 · 3 comments
Closed

crypto/x509: DecryptPEMBlock sometimes passes when the password is wrong #27880

azr opened this issue Sep 26, 2018 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@azr
Copy link
Contributor

azr commented Sep 26, 2018

Hello, my following test will fail all the time.
Running it once always passes though.

Context: Was trying to address hashicorp/packer#3337

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

go version go1.11 darwin/amd64

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

go env
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/azr/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/azr/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.11/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.11/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
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"

What did you do?

func TestEncodeDecode(t *testing.T) {
  data := []byte("data that I want to encrypt")
  pass := []byte("very secret password")
  for i := 0; i < 1000; i++ {
    // Encrypt the file
    b, err := x509.EncryptPEMBlock(rand.Reader,
      "RSA PRIVATE KEY",
      data,
      pass,
      x509.PEMCipherAES256)
    if err != nil {
      t.Fatalf("err: %s", err)
    }
    bf := &bytes.Buffer{}

    err = pem.Encode(bf, b)
    if err != nil {
      t.Fatalf("err: %s", err)
    }
    privateKey := bf.Bytes()

    PEMBlock, _ := pem.Decode(privateKey)
    if PEMBlock == nil {
      t.Fatal("")
    }

    if x509.IsEncryptedPEMBlock(PEMBlock) {
      decryptedPrivateKeyBytes, err := x509.DecryptPEMBlock(PEMBlock, []byte("wrooooong"))
      if err == nil {
        if string(decryptedPrivateKeyBytes) != string(data) {
          t.Fatalf("this is not supposed to happen: %s", decryptedPrivateKeyBytes)
        }
      }
    }
  }
}

What did you expect to see?

PASS

What did you see instead?

--- FAIL: TestEncodeDecode (0.00s)
    private_key_test.go:149: this is not supposed to happen:   {n<r   WVzQ p(!0D  )^fv
FAIL
FAIL    github.com/hashicorp/packer/builder/googlecompute       0.079s
@andybons
Copy link
Member

@FiloSottile

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 26, 2018
@andybons andybons added this to the Unplanned milestone Sep 26, 2018
@minaevmike
Copy link
Contributor

minaevmike commented Sep 28, 2018

from docs to DecryptPEMBlock

// If an incorrect password
// is detected an IncorrectPasswordError is returned. Because of deficiencies
// in the encrypted-PEM format, it's not always possible to detect an incorrect
// password. In these cases no error will be returned but the decrypted DER
// bytes will be random noise.

seems this is documented behavior

@azr
Copy link
Contributor Author

azr commented Oct 1, 2018

Oh, I also totally missed that part, then I'm closing this issue.

@azr azr closed this as completed Oct 1, 2018
@golang golang locked and limited conversation to collaborators Oct 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants