Skip to content

Commit

Permalink
738 parse cert with trailing stuff (#739)
Browse files Browse the repository at this point in the history
* test: parse X509 certificate with some trailing stuff

* fix: parse X509 certificate with some trailing stuff; fail, if PEM doesn't contain any X509 certificate

* changelog entry
  • Loading branch information
johakoch authored Mar 7, 2023
1 parent fb9cce3 commit 4856fd6
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 3 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Unreleased changes are available as `avenga/couper:edge` container.
* `WWW-Authenticate` header `realm` param value for [`basic_auth`](https://docs.couper.io/configuration/block/basic_auth) ([#715](https://github.com/avenga/couper/pull/715))
* [JWT access control](https://docs.couper.io/configuration/block/jwt) now creating `401` error status code, adding a `WWW-Authenticate: Bearer[...]` response header if appropriate ([#719](https://github.com/avenga/couper/pull/719))
* Erroneous multiplying of [health probes](https://docs.couper.io/configuration/block/health), [jobs](https://docs.couper.io/configuration/block/job) and requests to [JWKS](https://docs.couper.io/configuration/block/jwt) and [OpenID configuration](https://docs.couper.io/configuration/block/oidc) resources after a reload with [`-watch`](https://docs.couper.io/configuration/command-line#basic-options) ([#730](https://github.com/avenga/couper/pull/730), [#736](https://github.com/avenga/couper/pull/736))
* Reading PEM-encoded CA certificates ([`ca_file` setting](https://docs.couper.io/configuration/block/settings#attributes) or [`-ca-file` option](https://docs.couper.io/configuration/command-line#tls-options)) containing bytes trailing the PEM message ([#739](https://github.com/avenga/couper/pull/739))

---

Expand Down
9 changes: 8 additions & 1 deletion command/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,13 @@ func readCertificateFile(file string) ([]byte, error) {
return nil, fmt.Errorf("error reading ca-certificate: empty file: %q", file)
}

hasValidCert := false
pemCerts := cert[:]
for len(pemCerts) > 0 {
var block *pem.Block
block, pemCerts = pem.Decode(pemCerts)
if block == nil {
return nil, fmt.Errorf("error parsing pem ca-certificate: missing pem block")
break
}
if block.Type != "CERTIFICATE" || len(block.Headers) != 0 {
continue
Expand All @@ -190,6 +191,12 @@ func readCertificateFile(file string) ([]byte, error) {
if _, err = x509.ParseCertificate(certBytes); err != nil {
return nil, fmt.Errorf("error parsing pem ca-certificate: %q: %v", file, err)
}

hasValidCert = true
}

if !hasValidCert {
return nil, fmt.Errorf("error parsing pem ca-certificate: has no valid X509 certificate")
}

return cert, nil
Expand Down
21 changes: 19 additions & 2 deletions command/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -427,8 +427,25 @@ func TestReadCAFile(t *testing.T) {
_, err = malformedFile.Write(ssc.CACertificate.Certificate[:100]) // incomplete
helper.Must(err)

exp := "error parsing pem ca-certificate: has no valid X509 certificate"
_, err = readCertificateFile(malformedFile.Name())
if err == nil || err.Error() != "error parsing pem ca-certificate: missing pem block" {
t.Error("expected: error parsing pem ca-certificate: missing pem block")
if err == nil {
t.Errorf("expected: %s", exp)
} else if err.Error() != exp {
t.Errorf("expected: %s\nwas: %s", exp, err.Error())
}

okFile, err := os.CreateTemp("", "ok.cert")
helper.Must(err)
defer os.Remove(okFile.Name())

_, err = okFile.Write(ssc.CACertificate.Certificate[:])
helper.Must(err)
_, err = okFile.WriteString("bogus trailing stuff")
helper.Must(err)

_, err = readCertificateFile(okFile.Name())
if err != nil {
t.Error("expected no error")
}
}

0 comments on commit 4856fd6

Please sign in to comment.