Allow OUs by MayRosenbaum · Pull Request #198 · hyperledger/fabric-x-common

MayRosenbaum · 2026-09-27T20:51:25Z

Type of change

New feature
Test update

Description

channelconfig floors the MSP version of every channel org to MSPv3_0, so an MSP that declares NodeOUs enforces them (admins are identified by the admin OU) even when the channel declares no capabilities. MSPs without NodeOUs keep using admincerts.
The floor is MSPv3_0 on purpose, rather than the MSPv1_4_3 that NodeOU admin classification strictly needs: Fabric-X is never going back to an MSP version below v3, and we have no installed base, so we do not care about upgrades.
cryptogen.ConfigBlockParameters and testcrypto.ConfigBlock gain an EnableNodeOUs option (default false) that generates every org with a NodeOUs-enabled config.yaml and an empty admincerts folder.

Related issues

#196

coveralls · 2026-09-27T21:04:37Z

coverage: 82.044% (+0.02%) from 82.029% — MayRosenbaum:allow_OUs into hyperledger:main

tock-ibm

Automated /code-review (max effort) — 4 comment-only findings, most severe first. The key claim was checked empirically (branch built; new test run with and without the channel.go change; channelconfig/capabilities/msp/genesis/configtx/cryptogen suites green).

Headline: the new test passes identically with or without the channel.go floor, so the PR's central production change currently has no regression protection.

tock-ibm · 2026-09-28T09:33:41Z

+					},
+				},
+			})
+			readBundle(t, block) // fails unless the generated config loads into a valid bundle.


The new test does not guard the channel.go floor change — the docstring's central guarantee is false.

The docstring (lines 563-564) claims a valid bundle "proves channelconfig enforces OUs regardless of capabilities, since an OU-only MSP … would otherwise be left with no admins." But SampleFabricX declares empty channel capabilities (MSPVersion = MSPv1_0), and at MSPv1_0 setupAdminsPreV142 permits zero admins and NewBundleFromEnvelope loads fine — so readBundle succeeds with or without the floor.

Verified: checking out common/channelconfig/channel.go from main and re-running leaves both subtests green. The test only asserts the bundle loads and inspects the on-disk crypto layout; it never evaluates the org Admins policy against an OU-classified admin identity (as requireSign does at line 267). So the channel.go change is effectively untested here and the claimed proof is vacuous.

Suggest asserting something that actually depends on OU enforcement (e.g. that an OU-classified admin identity satisfies the org Admins policy), so reverting the floor would fail the test.

tock-ibm · 2026-09-28T09:33:41Z

+	// declares no capabilities, while MSPs without NodeOUs keep using admincerts.
+	mspVersion := channelCapabilities.MSPVersion()
+	if mspVersion < msp.MSPv3_0 {
+		mspVersion = msp.MSPv3_0


Flooring the MSP version to MSPv3_0 over-reaches the stated "honor OUs" goal.

NodeOU admin classification requires only MSPv1_4_3 (setupV142 / setupNodeOUsV142 / setupAdminsV142). MSPv3_0 additionally runs setupV3, which sets supportedPublicKeyAlgorithms[x509.Ed25519] = true (mspimplsetup.go:693).

By flooring to MSPv3_0, a channel that declared no capabilities (or V1_1/V1_3/V1_4_3/V2_0) now accepts Ed25519 member certificates it previously rejected — and any node still on an older library version derives MSPv1_0 for the same config block, so the two interpret identity acceptance differently. That is a determinism/fork risk during a rolling upgrade of a consumer (committer/orderer).

The minimal, on-goal change is to floor to msp.MSPv1_4_3 instead of msp.MSPv3_0.

Put it the issue description the fact that we are never going back to ver < v3 and that we do not have an installed base, thus don't care about upgrades.

tock-ibm · 2026-09-28T09:33:41Z

+	// local MSP version) — an MSP that declares NodeOUs then enforces them even when the channel
+	// declares no capabilities, while MSPs without NodeOUs keep using admincerts.
+	mspVersion := channelCapabilities.MSPVersion()
+	if mspVersion < msp.MSPv3_0 {


Bundle-load regression for a sub-V1_4_3 channel whose org MSP has both admincerts and a NodeOUs-enabled config.yaml.

Consider an existing channel with no channel capabilities (previously MSPv1_0) whose org verifying MSP supplies admincerts and a config.yaml with NodeOUs.Enable = true whose admin certificate carries neither the client nor the admin OU.

Before: MSPv1_0 ignores NodeOUs (setupV1, ouEnforcement = false) and the admincert authorizes admins → bundle loads.

After: MSPv3_0 sets ouEnforcement = true, and postSetupV142 (mspimplsetup.go:737-743) runs hasOURole(admin, CLIENT/ADMIN) on each admincert identity and returns "admin i is invalid" → NewBundleFromEnvelope now fails to load a config block that loaded before the upgrade.

Flooring to MSPv1_4_3 (see the comment on the next line) does not remove this exposure on its own, since V1_4_3 also enforces OUs; worth calling out the upgrade-compatibility implication in the PR description.

Put it the issue description the fact that we are never going back to ver < v3 and that we do not have an installed base, thus don't care about upgrades.

tock-ibm · 2026-09-28T09:33:41Z

+	// local MSP version) — an MSP that declares NodeOUs then enforces them even when the channel
+	// declares no capabilities, while MSPs without NodeOUs keep using admincerts.
+	mspVersion := channelCapabilities.MSPVersion()
+	if mspVersion < msp.MSPv3_0 {


Minor (cleanup): the manual floor can use the Go builtin max() (repo is Go 1.27).

mspVersion := max(channelCapabilities.MSPVersion(), msp.MSPv3_0)

is equivalent to the three-line if and compiles with the named MSPVersion int type plus the untyped constant. Readability/maintenance only. (If you take the MSPv1_4_3 suggestion above, this becomes max(…, msp.MSPv1_4_3).)

tock-ibm

Eventually we want these capabilities gone in fabric-x

tock-ibm · 2026-09-28T13:06:58Z

+	// EnableNodeOUs turns on node-OU based identity classification for every generated
+	// organization: each MSP gets a NodeOUs-enabled config.yaml and admin authority is
+	// conveyed by the admin OU instead of by admincerts. Defaults to false.
+	EnableNodeOUs bool


If we are flooring to v3 that the default should be true; or alternatively, set it to true regardless? since we are never going back to ver < v3

If we are flooring to v3, we should enable both "admincerts" and "OUs"

tock-ibm · 2026-09-28T13:08:57Z

+	// local MSP version) — an MSP that declares NodeOUs then enforces them even when the channel
+	// declares no capabilities, while MSPs without NodeOUs keep using admincerts.
+	mspVersion := channelCapabilities.MSPVersion()
+	if mspVersion < msp.MSPv3_0 {


Put it the issue description the fact that we are never going back to ver < v3 and that we do not have an installed base, thus don't care about upgrades.

tock-ibm · 2026-09-28T13:09:18Z

+	// declares no capabilities, while MSPs without NodeOUs keep using admincerts.
+	mspVersion := channelCapabilities.MSPVersion()
+	if mspVersion < msp.MSPv3_0 {
+		mspVersion = msp.MSPv3_0


Put it the issue description the fact that we are never going back to ver < v3 and that we do not have an installed base, thus don't care about upgrades.

Signed-off-by: May.Buzaglo <May.Buzaglo@ibm.com>

MayRosenbaum changed the title ~~Allow o us~~ Allow OUs Sep 27, 2026

MayRosenbaum force-pushed the allow_OUs branch from 7b8295f to 780630f Compare September 27, 2026 21:06

tock-ibm reviewed Sep 28, 2026

View reviewed changes

tock-ibm force-pushed the allow_OUs branch from 780630f to 970165b Compare September 30, 2026 12:22

MayRosenbaum force-pushed the allow_OUs branch from 970165b to 278e151 Compare September 30, 2026 15:00

tock-ibm approved these changes Oct 1, 2026

View reviewed changes

MayRosenbaum added 3 commits October 1, 2026 12:13

allow OUs

f37895b

Signed-off-by: May.Buzaglo <May.Buzaglo@ibm.com>

allow OUs regardless of capabilities

ee5b716

Signed-off-by: May.Buzaglo <May.Buzaglo@ibm.com>

address review comments

d8971c2

Signed-off-by: May.Buzaglo <May.Buzaglo@ibm.com>

tock-ibm force-pushed the allow_OUs branch from 278e151 to d8971c2 Compare October 1, 2026 09:13

tock-ibm merged commit 6c69522 into hyperledger:main Oct 1, 2026
4 checks passed

MayRosenbaum mentioned this pull request Oct 1, 2026

[config-TX-rules] Reject config updates where a party's signing CA and its MSP node OUs disagree hyperledger/fabric-x-orderer#1257

Open

Conversation

MayRosenbaum commented Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Type of change

Description

Related issues

Uh oh!

coveralls commented Sep 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tock-ibm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tock-ibm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MayRosenbaum commented Sep 27, 2026 •

edited

Loading

coveralls commented Sep 27, 2026 •

edited

Loading