Fix a memory bug in ldap's ParseDN function by disabling part of the functionality

[inteon]

Quick fix for memory issues caused by github.com/go-asn1-ber/asn1-ber library.
Similar to notaryproject/notation-go#275.
Will be superseded by #6761, the goal of this PR is to backport this quick fix to older cert-manager versions.

Kind

/kind bug

Release Note

Bugfix: LiteralSubjects with a #= value can result in memory issues due to faulty BER parser (github.com/go-asn1-ber/asn1-ber).

[ThatsMrTalbot]

/lgtm

[inteon]

/approve

[jetstack-bot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: inteon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [inteon]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[inteon]

/cherrypick release-1.14

[jetstack-bot]

@inteon: once the present PR merges, I will cherry-pick it on top of release-1.14 in a new PR and assign it to you.

In response to this:

/cherrypick release-1.14

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[inteon]

/cherrypick release-1.13

[jetstack-bot]

@inteon: once the present PR merges, I will cherry-pick it on top of release-1.13 in a new PR and assign it to you.

In response to this:

/cherrypick release-1.13

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[inteon]

/cherrypick release-1.12

[jetstack-bot]

@inteon: once the present PR merges, I will cherry-pick it on top of release-1.12 in a new PR and assign it to you.

In response to this:

/cherrypick release-1.12

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[SgtCoDFish]

/hold

Should this even be merged into master? Why not merge into release-1.14 and then cherry-pick from there into release-1.13 and release-1.12?

I don't think we ever intend for this code to be used in a future release.

I'm happy to merge as-is, hold is just to ask the question

[inteon]

@SgtCoDFish My plan is to merge this bugfix in master and backport to all older versions.
The goal is to already fix the problem here, so we can later have a new implementation that still works even for the new test case that was added in this PR.
I do agree that this will only be in the master branch for a short time, but I do think it is a good approach to not exceed master in terms of bugfixes etc.
/unhold

[SgtCoDFish]

diff --git a/pkg/util/pki/parse_test.go b/pkg/util/pki/parse_test.go
index a219564c4..34307b0da 100644
--- a/pkg/util/pki/parse_test.go
+++ b/pkg/util/pki/parse_test.go
@@ -47,6 +47,15 @@ func generatePKCS8PrivateKey(keyAlgo v1.PrivateKeyAlgorithm, keySize int) ([]byt
 	return EncodePKCS8PrivateKey(privateKey)
 }
 
+func BenchmarkParseSubject(b *testing.B) {
+	for n := 0; n < b.N; n++ {
+		_, err := ParseSubjectStringToRawDERBytes("DF=#6666666666665006838820013100000746939546349182108463491821809FBFFFFFFFFF")
+		if err == nil {
+			b.Fatal("expected error, but got none")
+		}
+	}
+}
+
 func TestDecodePrivateKeyBytes(t *testing.T) {
 	type testT struct {
 		name         string

Run with:

go test -bench=. -benchmem ./pkg/util/pki/...

Before this change:

goos: darwin
goarch: arm64
pkg: github.com/cert-manager/cert-manager/pkg/util/pki
BenchmarkParseSubject-12              58          20398892 ns/op        1665740367 B/op       58 allocs/op

After this change:

goos: darwin
goarch: arm64
pkg: github.com/cert-manager/cert-manager/pkg/util/pki
BenchmarkParseSubject-12         2340868               475.9 ns/op           304 B/op          4 allocs/op

MIght be worth adding that benchmark to the followup PR for comparison

[inteon]

/retest

[jetstack-bot]

@inteon: new pull request created: #6772

In response to this:

/cherrypick release-1.13

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[jetstack-bot]

@inteon: new pull request created: #6773

In response to this:

/cherrypick release-1.12

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[jetstack-bot]

@inteon: new pull request created: #6774

In response to this:

/cherrypick release-1.14

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.