From 2707c2f4b454742e4c74d062cd690017106bce4e Mon Sep 17 00:00:00 2001
From: David Christofas <dchristofas@owncloud.com>
Date: Wed, 6 Jul 2022 15:16:43 +0200
Subject: [PATCH 1/2] escape DN attribute values

---
 changelog/unreleased/ldap-escape-dn.md |  6 +++++
 ocis-pkg/ldap/ldap.go                  | 32 ++++++++++++++++++++++++++
 ocis-pkg/ldap/ldap_suite_test.go       | 13 +++++++++++
 ocis-pkg/ldap/ldap_test.go             | 25 ++++++++++++++++++++
 services/graph/pkg/identity/ldap.go    |  5 ++--
 5 files changed, 79 insertions(+), 2 deletions(-)
 create mode 100644 changelog/unreleased/ldap-escape-dn.md
 create mode 100644 ocis-pkg/ldap/ldap_suite_test.go
 create mode 100644 ocis-pkg/ldap/ldap_test.go

diff --git a/changelog/unreleased/ldap-escape-dn.md b/changelog/unreleased/ldap-escape-dn.md
new file mode 100644
index 00000000000..78e38cd5fde
--- /dev/null
+++ b/changelog/unreleased/ldap-escape-dn.md
@@ -0,0 +1,6 @@
+Bugfix: Escape DN attribute value
+
+Escaped the DN attribute value on creating users and groups.
+
+
+https://github.com/owncloud/ocis/pull/4117
diff --git a/ocis-pkg/ldap/ldap.go b/ocis-pkg/ldap/ldap.go
index 8eceaf3cb94..9276c470989 100644
--- a/ocis-pkg/ldap/ldap.go
+++ b/ocis-pkg/ldap/ldap.go
@@ -5,6 +5,7 @@ import (
 	"errors"
 	"io/ioutil"
 	"os"
+	"strings"
 	"time"
 
 	"github.com/owncloud/ocis/v2/ocis-pkg/log"
@@ -15,6 +16,20 @@ const (
 	caCheckSleep   = 2
 )
 
+var (
+	dnEscaper = strings.NewReplacer(
+		"\\", "\\\\",
+		",", "\\,",
+		"+", "\\+",
+		`"`, `\\"`,
+		"<", "\\<",
+		">", "\\>",
+		";", "\\;",
+		"=", "\\=",
+		"\000", "\\\000",
+	)
+)
+
 func WaitForCA(log log.Logger, insecure bool, caCert string) error {
 	if !insecure && caCert != "" {
 		for i := 0; i < caCheckRetries; i++ {
@@ -38,3 +53,20 @@ func WaitForCA(log log.Logger, insecure bool, caCert string) error {
 	}
 	return nil
 }
+
+// EscapeDNAttributeValue escapes special characters in an attribute value as [described in RFC4514](https://datatracker.ietf.org/doc/html/rfc4514).
+func EscapeDNAttributeValue(v string) string {
+	if v == "" {
+		return v
+	}
+
+	v = dnEscaper.Replace(v)
+
+	if strings.HasSuffix(v, " ") {
+		v = v[:len(v)-1] + "\\ "
+	}
+	if strings.HasPrefix(v, "#") || strings.HasPrefix(v, " ") {
+		v = "\\" + v
+	}
+	return v
+}
diff --git a/ocis-pkg/ldap/ldap_suite_test.go b/ocis-pkg/ldap/ldap_suite_test.go
new file mode 100644
index 00000000000..3eee47856d2
--- /dev/null
+++ b/ocis-pkg/ldap/ldap_suite_test.go
@@ -0,0 +1,13 @@
+package ldap_test
+
+import (
+	"testing"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+func TestLdap(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Ldap Suite")
+}
diff --git a/ocis-pkg/ldap/ldap_test.go b/ocis-pkg/ldap/ldap_test.go
new file mode 100644
index 00000000000..6925facb712
--- /dev/null
+++ b/ocis-pkg/ldap/ldap_test.go
@@ -0,0 +1,25 @@
+package ldap_test
+
+import (
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	"github.com/owncloud/ocis/v2/ocis-pkg/ldap"
+)
+
+var _ = Describe("ldap", func() {
+	DescribeTable("EscapeDNAttributeValue should escape special characters",
+		func(input, expected string) {
+			escaped := ldap.EscapeDNAttributeValue(input)
+			Expect(escaped).To(Equal(expected))
+		},
+		Entry("normal dn", "foobar", "foobar"),
+		Entry("including comma", "foo,bar", "foo\\,bar"),
+		Entry("including equals", "foo=bar", "foo\\=bar"),
+		Entry("beginning with number sign", "#foobar", "\\#foobar"),
+		Entry("beginning with space", " foobar", "\\ foobar"),
+		Entry("only one space", " ", "\\ "),
+		Entry("two spaces", "  ", "\\ \\ "),
+		Entry("ending with space", "foobar ", "foobar\\ "),
+		Entry("containing multiple special chars", `f+o>o,b<a;r="\00"`, `f\+o\>o\,b\<a\;r\=\\"\\00\\"`),
+	)
+})
diff --git a/services/graph/pkg/identity/ldap.go b/services/graph/pkg/identity/ldap.go
index a801c9510b4..398fabaca63 100644
--- a/services/graph/pkg/identity/ldap.go
+++ b/services/graph/pkg/identity/ldap.go
@@ -11,6 +11,7 @@ import (
 	"github.com/gofrs/uuid"
 	ldapdn "github.com/libregraph/idm/pkg/ldapdn"
 	libregraph "github.com/owncloud/libre-graph-api-go"
+	oldap "github.com/owncloud/ocis/v2/ocis-pkg/ldap"
 	"github.com/owncloud/ocis/v2/ocis-pkg/log"
 	"github.com/owncloud/ocis/v2/services/graph/pkg/config"
 	"github.com/owncloud/ocis/v2/services/graph/pkg/service/v0/errorcode"
@@ -116,7 +117,7 @@ func (i *LDAP) CreateUser(ctx context.Context, user libregraph.User) (*libregrap
 		return nil, errReadOnly
 	}
 	ar := ldap.AddRequest{
-		DN: fmt.Sprintf("uid=%s,%s", *user.OnPremisesSamAccountName, i.userBaseDN),
+		DN: fmt.Sprintf("uid=%s,%s", oldap.EscapeDNAttributeValue(*user.OnPremisesSamAccountName), i.userBaseDN),
 		Attributes: []ldap.Attribute{
 			// inetOrgPerson requires "cn"
 			{
@@ -691,7 +692,7 @@ func (i *LDAP) CreateGroup(ctx context.Context, group libregraph.Group) (*libreg
 		return nil, errorcode.New(errorcode.NotAllowed, "server is configured read-only")
 	}
 	ar := ldap.AddRequest{
-		DN: fmt.Sprintf("cn=%s,%s", *group.DisplayName, i.groupBaseDN),
+		DN: fmt.Sprintf("cn=%s,%s", oldap.EscapeDNAttributeValue(*group.DisplayName), i.groupBaseDN),
 		Attributes: []ldap.Attribute{
 			{
 				Type: i.groupAttributeMap.name,

From 00a27bfff4c279cac36ce6b6ea052451924d5928 Mon Sep 17 00:00:00 2001
From: David Christofas <dchristofas@owncloud.com>
Date: Wed, 6 Jul 2022 17:11:18 +0200
Subject: [PATCH 2/2] fix the null byte replace

---
 ocis-pkg/ldap/ldap.go      | 2 +-
 ocis-pkg/ldap/ldap_test.go | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ocis-pkg/ldap/ldap.go b/ocis-pkg/ldap/ldap.go
index 9276c470989..cfa452f1a5d 100644
--- a/ocis-pkg/ldap/ldap.go
+++ b/ocis-pkg/ldap/ldap.go
@@ -26,7 +26,7 @@ var (
 		">", "\\>",
 		";", "\\;",
 		"=", "\\=",
-		"\000", "\\\000",
+		"\000", "\\00",
 	)
 )
 
diff --git a/ocis-pkg/ldap/ldap_test.go b/ocis-pkg/ldap/ldap_test.go
index 6925facb712..d3ee3152cfb 100644
--- a/ocis-pkg/ldap/ldap_test.go
+++ b/ocis-pkg/ldap/ldap_test.go
@@ -20,6 +20,6 @@ var _ = Describe("ldap", func() {
 		Entry("only one space", " ", "\\ "),
 		Entry("two spaces", "  ", "\\ \\ "),
 		Entry("ending with space", "foobar ", "foobar\\ "),
-		Entry("containing multiple special chars", `f+o>o,b<a;r="\00"`, `f\+o\>o\,b\<a\;r\=\\"\\00\\"`),
+		Entry("containing multiple special chars", "f+o>o,b<a;r=\"\000", `f\+o\>o\,b\<a\;r\=\\"\00`),
 	)
 })