Skip to content

Commit 7a4dc89

Browse files
committed
fix(api): update InternalRBACRules SPIFFE identifiers to nico-* prefix
After the carbide → nico platform rename, all newly deployed services present SPIFFE identifiers with the nico-* prefix, but InternalRBACRules in crates/api/src/auth/internal_rbac_rules.rs still matched against hardcoded carbide-* strings. Every internal service-to-api gRPC call failed mTLS authorization with HTTP 403, silently breaking all service-to-service communication. Switch RuleInfo::new from `map(|x| -> Principal)` to `flat_map(|x| -> Vec<Principal>)` so each rule can accept multiple acceptable SPIFFE identifiers, then have each renamed variant emit BOTH the new nico-* and the legacy carbide-* identifier via an svc_compat helper: Dns -> nico-dns, carbide-dns Dhcp -> nico-dhcp, carbide-dhcp Ssh -> nico-ssh-console, carbide-ssh-console SshRs -> nico-ssh-console-rs, carbide-ssh-console-rs Pxe -> nico-pxe, carbide-pxe BmcProxy -> nico-bmc-proxy, carbide-bmc-proxy Health -> nico-hardware-health, carbide-hardware-health Flow -> nico-flow, carbide-flow MaintenanceJobs -> nico-maintenance-jobs, carbide-maintenance-jobs DsxExchangeConsumer -> nico-dsx-exchange-consumer, carbide-dsx-exchange-consumer The `allowed()` matcher already walks this Vec with `.any(...)`, so the change is transparent to callers: deployed sites still presenting a carbide-* cert continue to authorize, and freshly deployed sites with nico-* certs work too. The carbide-* aliases should be dropped once every site has rotated to a nico-* cert. Failure mode before this fix: inbound gRPC from e.g. nico-dns to nico-api surfaced as WARN auth::internal_rbac_rules — principal SpiffeServiceIdentifier("nico-dns") not authorized for method LookupRecordLegacy — no matching rule with no TLS-level error, masking the root cause. Impact spanned DNS resolution, DHCP lease lookups, PXE GetCloudInitInstructions, SSH console access, hardware health reporting, and maintenance job scheduling — every internal principal that authenticates via SpiffeServiceIdentifier. Follow-up (not in this PR): these identifiers are stringly-typed with no compile-time link to the actual deployed service names. Worth deriving them from a shared constant or asserting consistency in an integration test that round-trips each principal through cert subject + RBAC lookup. Fixes #1891
1 parent 573ddb2 commit 7a4dc89

1 file changed

Lines changed: 79 additions & 60 deletions

File tree

crates/api/src/auth/internal_rbac_rules.rs

Lines changed: 79 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -899,54 +899,60 @@ struct RuleInfo {
899899

900900
impl RuleInfo {
901901
pub fn new(principals: Vec<RulePrincipal>) -> Self {
902+
// Helper: emit both the nico-* and carbide-* SPIFFE service identifiers
903+
// for a renamed service. The matcher in `allowed()` walks this Vec with
904+
// `.any(...)`, so any cert presenting either string is accepted. Drop
905+
// the carbide-* alias once every deployed site has rotated to a cert
906+
// with the nico-* identifier.
907+
let svc_compat = |nico_name: &str, carbide_name: &str| {
908+
vec![
909+
Principal::SpiffeServiceIdentifier(nico_name.to_string()),
910+
Principal::SpiffeServiceIdentifier(carbide_name.to_string()),
911+
]
912+
};
902913
Self {
903914
principals: principals
904915
.iter()
905-
.map(|x| match *x {
906-
RulePrincipal::ForgeAdminCLI => Principal::ExternalUser(ExternalUserInfo::new(
907-
None,
908-
"Invalid".to_string(),
909-
None,
910-
)),
911-
RulePrincipal::Machineatron => {
912-
Principal::SpiffeServiceIdentifier("machine-a-tron".to_string())
913-
}
914-
RulePrincipal::SiteAgent => {
915-
Principal::SpiffeServiceIdentifier("elektra-site-agent".to_string())
916+
.flat_map(|x| match *x {
917+
RulePrincipal::ForgeAdminCLI => {
918+
vec![Principal::ExternalUser(ExternalUserInfo::new(
919+
None,
920+
"Invalid".to_string(),
921+
None,
922+
))]
916923
}
917-
RulePrincipal::Agent => Principal::SpiffeMachineIdentifier("".to_string()),
918-
RulePrincipal::Scout => Principal::SpiffeMachineIdentifier("".to_string()),
919-
RulePrincipal::Dns => {
920-
Principal::SpiffeServiceIdentifier("carbide-dns".to_string())
924+
RulePrincipal::Machineatron => vec![Principal::SpiffeServiceIdentifier(
925+
"machine-a-tron".to_string(),
926+
)],
927+
RulePrincipal::SiteAgent => vec![Principal::SpiffeServiceIdentifier(
928+
"elektra-site-agent".to_string(),
929+
)],
930+
RulePrincipal::Agent => {
931+
vec![Principal::SpiffeMachineIdentifier("".to_string())]
921932
}
922-
RulePrincipal::Dhcp => {
923-
Principal::SpiffeServiceIdentifier("carbide-dhcp".to_string())
924-
}
925-
RulePrincipal::Ssh => {
926-
Principal::SpiffeServiceIdentifier("carbide-ssh-console".to_string())
933+
RulePrincipal::Scout => {
934+
vec![Principal::SpiffeMachineIdentifier("".to_string())]
927935
}
936+
RulePrincipal::Dns => svc_compat("nico-dns", "carbide-dns"),
937+
RulePrincipal::Dhcp => svc_compat("nico-dhcp", "carbide-dhcp"),
938+
RulePrincipal::Ssh => svc_compat("nico-ssh-console", "carbide-ssh-console"),
928939
RulePrincipal::SshRs => {
929-
Principal::SpiffeServiceIdentifier("carbide-ssh-console-rs".to_string())
930-
}
931-
RulePrincipal::Pxe => {
932-
Principal::SpiffeServiceIdentifier("carbide-pxe".to_string())
933-
}
934-
RulePrincipal::BmcProxy => {
935-
Principal::SpiffeServiceIdentifier("carbide-bmc-proxy".to_string())
940+
svc_compat("nico-ssh-console-rs", "carbide-ssh-console-rs")
936941
}
942+
RulePrincipal::Pxe => svc_compat("nico-pxe", "carbide-pxe"),
943+
RulePrincipal::BmcProxy => svc_compat("nico-bmc-proxy", "carbide-bmc-proxy"),
937944
RulePrincipal::Health => {
938-
Principal::SpiffeServiceIdentifier("carbide-hardware-health".to_string())
939-
}
940-
RulePrincipal::Flow => {
941-
Principal::SpiffeServiceIdentifier("carbide-flow".to_string())
945+
svc_compat("nico-hardware-health", "carbide-hardware-health")
942946
}
947+
RulePrincipal::Flow => svc_compat("nico-flow", "carbide-flow"),
943948
RulePrincipal::MaintenanceJobs => {
944-
Principal::SpiffeServiceIdentifier("carbide-maintenance-jobs".to_string())
949+
svc_compat("nico-maintenance-jobs", "carbide-maintenance-jobs")
945950
}
946-
RulePrincipal::DsxExchangeConsumer => Principal::SpiffeServiceIdentifier(
947-
"carbide-dsx-exchange-consumer".to_string(),
951+
RulePrincipal::DsxExchangeConsumer => svc_compat(
952+
"nico-dsx-exchange-consumer",
953+
"carbide-dsx-exchange-consumer",
948954
),
949-
RulePrincipal::Anonymous => Principal::Anonymous,
955+
RulePrincipal::Anonymous => vec![Principal::Anonymous],
950956
})
951957
.collect(),
952958
}
@@ -1033,15 +1039,11 @@ mod rbac_rule_tests {
10331039
));
10341040
assert!(InternalRBACRules::allowed_from_static(
10351041
"GetCloudInitInstructions",
1036-
&[Principal::SpiffeServiceIdentifier(
1037-
"carbide-pxe".to_string()
1038-
)]
1042+
&[Principal::SpiffeServiceIdentifier("nico-pxe".to_string())]
10391043
));
10401044
assert!(!InternalRBACRules::allowed_from_static(
10411045
"GetCloudInitInstructions",
1042-
&[Principal::SpiffeServiceIdentifier(
1043-
"carbide-dns".to_string()
1044-
)]
1046+
&[Principal::SpiffeServiceIdentifier("nico-dns".to_string())]
10451047
));
10461048
assert!(!InternalRBACRules::allowed_from_static(
10471049
"GetCloudInitInstructions",
@@ -1059,9 +1061,7 @@ mod rbac_rule_tests {
10591061
));
10601062
assert!(!InternalRBACRules::allowed_from_static(
10611063
"CreateVpc",
1062-
&[Principal::SpiffeServiceIdentifier(
1063-
"carbide-dns".to_string()
1064-
)]
1064+
&[Principal::SpiffeServiceIdentifier("nico-dns".to_string())]
10651065
));
10661066

10671067
assert!(InternalRBACRules::allowed_from_static(
@@ -1086,7 +1086,7 @@ mod rbac_rule_tests {
10861086
assert!(InternalRBACRules::allowed_from_static(
10871087
"TrimTable",
10881088
&[Principal::SpiffeServiceIdentifier(
1089-
"carbide-maintenance-jobs".to_string()
1089+
"nico-maintenance-jobs".to_string()
10901090
)]
10911091
));
10921092

@@ -1109,34 +1109,26 @@ mod rbac_rule_tests {
11091109

11101110
assert!(InternalRBACRules::allowed_from_static(
11111111
"SetMaintenance",
1112-
&[Principal::SpiffeServiceIdentifier(
1113-
"carbide-flow".to_string()
1114-
)]
1112+
&[Principal::SpiffeServiceIdentifier("nico-flow".to_string())]
11151113
));
11161114
assert!(InternalRBACRules::allowed_from_static(
11171115
"InsertMachineHealthReport",
1118-
&[Principal::SpiffeServiceIdentifier(
1119-
"carbide-flow".to_string()
1120-
)]
1116+
&[Principal::SpiffeServiceIdentifier("nico-flow".to_string())]
11211117
));
11221118
assert!(InternalRBACRules::allowed_from_static(
11231119
"RemoveMachineHealthReport",
1124-
&[Principal::SpiffeServiceIdentifier(
1125-
"carbide-flow".to_string()
1126-
)]
1120+
&[Principal::SpiffeServiceIdentifier("nico-flow".to_string())]
11271121
));
11281122
assert!(InternalRBACRules::allowed_from_static(
11291123
"MachineSetAutoUpdate",
1130-
&[Principal::SpiffeServiceIdentifier(
1131-
"carbide-flow".to_string()
1132-
)]
1124+
&[Principal::SpiffeServiceIdentifier("nico-flow".to_string())]
11331125
));
11341126
for method in ["FindMacAddressByBmcIp", "GetBmcCredentials"] {
11351127
assert!(
11361128
InternalRBACRules::allowed_from_static(
11371129
method,
11381130
&[Principal::SpiffeServiceIdentifier(
1139-
"carbide-bmc-proxy".to_string()
1131+
"nico-bmc-proxy".to_string()
11401132
)]
11411133
),
11421134
"{method} should allow bmc-proxy"
@@ -1148,10 +1140,37 @@ mod rbac_rule_tests {
11481140
// SPIFFE identifiers, etc. We don't want to play any tricks by reusing principals here, so
11491141
// we gotta list both, until we've fully migrated to ssh-console-rs.)
11501142
ensure_identical_permissions(
1151-
&Principal::SpiffeServiceIdentifier("carbide-ssh-console".to_string()),
1152-
&Principal::SpiffeServiceIdentifier("carbide-ssh-console-rs".to_string()),
1143+
&Principal::SpiffeServiceIdentifier("nico-ssh-console".to_string()),
1144+
&Principal::SpiffeServiceIdentifier("nico-ssh-console-rs".to_string()),
11531145
);
11541146

1147+
// Backward-compat: every renamed service's carbide-* SPIFFE identifier
1148+
// must have *identical* permissions to its nico-* counterpart across
1149+
// every rule. RuleInfo::new emits both names side-by-side; this guards
1150+
// against accidental skew while we keep accepting carbide-*. Drop this
1151+
// block (and the svc_compat() carbide-* entries) once every deployed
1152+
// site has rotated to a nico-* cert.
1153+
for (nico, carbide) in [
1154+
("nico-dns", "carbide-dns"),
1155+
("nico-dhcp", "carbide-dhcp"),
1156+
("nico-ssh-console", "carbide-ssh-console"),
1157+
("nico-ssh-console-rs", "carbide-ssh-console-rs"),
1158+
("nico-pxe", "carbide-pxe"),
1159+
("nico-bmc-proxy", "carbide-bmc-proxy"),
1160+
("nico-hardware-health", "carbide-hardware-health"),
1161+
("nico-flow", "carbide-flow"),
1162+
("nico-maintenance-jobs", "carbide-maintenance-jobs"),
1163+
(
1164+
"nico-dsx-exchange-consumer",
1165+
"carbide-dsx-exchange-consumer",
1166+
),
1167+
] {
1168+
ensure_identical_permissions(
1169+
&Principal::SpiffeServiceIdentifier(nico.to_string()),
1170+
&Principal::SpiffeServiceIdentifier(carbide.to_string()),
1171+
);
1172+
}
1173+
11551174
Ok(())
11561175
}
11571176

0 commit comments

Comments
 (0)