sql,security: add parsing and validation of SUBJECT role option

[rafiss]

See individual commits.

informs #110616
fixes CRDB-35939
Epic CRDB-34126

Release note (enterprise change): Added a new SUBJECT role option. This
role option can be set to a subject distinguished name in RFC2253 or
RFC4514 format. If set, then during client certificate authentication,
a certificate with the given distinguished name can be used to
authenticate the role whose SUBJECT was configured. The root user cannot
have its SUBJECT configured.

[cockroach-teamcity]

This change is 

[bdarnell]

Reviewed 6 of 6 files at r1, 29 of 30 files at r2, 6 of 6 files at r3.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @rafiss)

---

-- commits line 12 at r2:
This gets the sense of the change backwards: it's already true by default that a cert can be used (matching only on the CN and ignoring the rest of the DN). The change is that when a subject is set, certs that don't match the subject DN will be rejected.

---

-- commits line 14 at r2:
I would leave out any mention of the root user for now. "cannot have its subject configured" is unclear. In context of the rest of the message as written it implies that root cannot log in with a cert, when in fact it can use a cert with fewer checks than regular users. That's an unacceptable outcome so we're going to have to do something about it but we haven't settled on the design yet, so we can wait to mention root until we've decided that.

---

pkg/security/distinguishedname/parse.go line 18 at r2 (raw file):

)

// ValidateDN validates a distinguished name string to verify that it is

As discussed on slack, we want an additional validation that the CN field matches the user name, so we'll need either a more complicated interface here or do that part of the validation at a different level.

---

pkg/sql/user.go line 88 at r3 (raw file):

	isSuperuser bool,
	defaultSettings []sessioninit.SettingsCacheEntry,
	subject *ldap.DN,

Should probably be a separate PR, but this function's return values are overdue for a refactoring into a struct.

[rafiss]

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @bdarnell)

---

-- commits line 12 at r2:

Previously, bdarnell (Ben Darnell) wrote…

This gets the sense of the change backwards: it's already true by default that a cert can be used (matching only on the CN and ignoring the rest of the DN). The change is that when a subject is set, certs that don't match the subject DN will be rejected.

done

---

-- commits line 14 at r2:

Previously, bdarnell (Ben Darnell) wrote…

I would leave out any mention of the root user for now. "cannot have its subject configured" is unclear. In context of the rest of the message as written it implies that root cannot log in with a cert, when in fact it can use a cert with fewer checks than regular users. That's an unacceptable outcome so we're going to have to do something about it but we haven't settled on the design yet, so we can wait to mention root until we've decided that.

that makes sense, done

---

pkg/security/distinguishedname/parse.go line 18 at r2 (raw file):

Previously, bdarnell (Ben Darnell) wrote…

As discussed on slack, we want an additional validation that the CN field matches the user name, so we'll need either a more complicated interface here or do that part of the validation at a different level.

i'll add the SQLUsername to the parameters here.

---

pkg/sql/user.go line 88 at r3 (raw file):

Previously, bdarnell (Ben Darnell) wrote…

Should probably be a separate PR, but this function's return values are overdue for a refactoring into a struct.

agreed

[bdarnell]

Reviewed 1 of 37 files at r4, 27 of 31 files at r5, 10 of 10 files at r6, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @BabuSrithar and @rafiss)

---

pkg/security/distinguishedname/parse.go line 37 at r6 (raw file):

				}
				sawCN = true
				if attr.Value != u.Normalized() {

Do we want to normalize (lowercase?) the attr.Value too?

[rafiss]

tftr!

bors r+

Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @BabuSrithar and @bdarnell)

---

pkg/security/distinguishedname/parse.go line 37 at r6 (raw file):

Previously, bdarnell (Ben Darnell) wrote…

Do we want to normalize (lowercase?) the attr.Value too?

done

[craig]

Build failed (retrying...):

• Bazel Essential CI (Cockroach)

[rafiss]

bors r-

[craig]

Canceled.

[rafiss]

bors r+

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)