Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

To handle nested claims. #10

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 28 additions & 4 deletions governance/constitution/kms_actions_maa.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,32 @@ actions.set(
checkType(args.claims, "object");
},
function (args) {
// Helper function to check equality of two objects
function isEqual(obj1, obj2) {
return JSON.stringify(obj1) === JSON.stringify(obj2);
}
// Helper function to check if an array contains an object
function includes(array, obj) {
return array.some((el) => JSON.stringify(el) === JSON.stringify(obj));
}


const CLAIMS = {
secureboot: "boolean",
"x-ms-azurevm-os-provisioning": {
"node-policy-identity": {
"eventVersion": "number",
"policyId": "string",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where are these claims tested when they appear in a KRP?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this will eventually become very confusing. E.g. policyId is a claim that we want to compare on value while eventVersion is typically a gte check.
If we use flattened claims it will be clearer and we can easily check if these claim is allowed in CLAIMS:
"x-ms-azurevm-os-provisioning.node-policy-identity.eventVersion"
"x-ms-azurevm-os-provisioning.node-policy-identity.policyId"

"signer": "string",
"svn": "number",
},
"os-image-identity": {
"diskId": "string",
"eventVersion" : "number",
"signer": "string",
"svn": "number",
}
},
"x-ms-attestation-type": "string",
"x-ms-azurevm-attestation-protocol-ver": "string",
"x-ms-azurevm-attested-pcrs": "number[]",
Expand Down Expand Up @@ -173,7 +197,7 @@ actions.set(
item.forEach((i) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the non-flattened model we should test if the key represents an object and than drill down in the object to check presence in CLAIMS. Way harder than using flattened claims.

console.log(`[INFO] [scope=set_key_release_policy->add] KRP add ${type}=>Adding ${i} to ${key}`);
// Only push if the element is not already in the array
if (!items[key].includes(i)) {
if (!includes(items[key],i)) {
items[key].push(i);
}
});
Expand Down Expand Up @@ -303,7 +327,7 @@ actions.set(
if (items[key] !== undefined) {
item.forEach((i) => {
console.log(`[INFO] [scope=set_key_release_policy->remove] KRP remove ${type}=>Removing ${i} from ${key}`);
items[key] = items[key].filter((value) => value !== i);
items[key] = items[key].filter((value) => !isEqual(value, i));
if (items[key].length === 0) {
delete items[key];
}
Expand All @@ -318,7 +342,7 @@ actions.set(
}
});

// Safe into KV
// Save into KV
console.log(`[INFO] [scope=set_key_release_policy->remove] KRP remove ${type}=>items: `, items);
let jsonItems = JSON.stringify(items);
console.log(
Expand Down Expand Up @@ -427,4 +451,4 @@ actions.set(
}
},
),
);
);
35 changes: 35 additions & 0 deletions governance/policies/key-release-policy-disk-add.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
{
"actions": [
{
"name": "set_key_release_policy",
"args": {
"service": "tien-test",
"type": "add",
"claims": {
"x-ms-azurevm-os-provisioning": {
"node-policy-identity": {
"eventVersion": "1",
"policyId": "openai-whisper",
"signer": "8fe6e7a314b8695b21710cebf0265e8d7bbaabde26f431c407faf16fcbd6b924"
},
"os-image-identity": {
"diskId": "singularity.ubuntu-22.04",
"eventVersion" : "1",
"signer": "f9cce5b7bdc2aaacfc4c78cb2b7515459aded8149287b74667bb2f178b0cf7b9"
}
}
},
"gte": {
"x-ms-azurevm-os-provisioning": {
"node-policy-identity": {
"svn": "1"
},
"os-image-identity": {
"svn": "1"
}
}
}
}
}
]
}
2 changes: 1 addition & 1 deletion governance/policies/key-release-policy-maa-add.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"x-ms-azurevm-osversion-major": [22, 23]
},
"gte": {
"x-ms-azurevm-osversion-major": "23"
"x-ms-azurevm-osversion-major": "22"
},
"gt": {
"x-ms-azurevm-osversion-major": 21
Expand Down
1 change: 1 addition & 0 deletions src/authorization/jwt/JwtIdentityProviderEnum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,5 @@ export enum JwtIdentityProviderEnum {
MAA_NoSecureBootTyFu = "https://maanosecureboottestyfu.eus.attest.azure.net",
MAA_NoSecureBootWeu = "https://accnosecurebootmaawesteu.weu.attest.azure.net",
MAA_NoSecureBootEus = "https://accnosecurebootmaa.eus2.attest.azure.net",
MAA_NoSecureBootInteTest = "https://confinfermaaeus2test.eus2.test.attest.azure.net",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have removed the necessity to add an issuer in code. This will not scale. I will sync my changes so we do no longer need this

}
9 changes: 9 additions & 0 deletions src/authorization/jwt/JwtValidator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,15 @@ export class JwtValidator implements IValidatorService {
JwtIdentityProviderEnum.MAA_NoSecureBootEus,
this.logContext
);
this.identityProviders.set(
JwtIdentityProviderEnum.MAA_NoSecureBootIntgTest,
new MsJwtProvider("JwtMaaProvider", this.logContext),
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

Logger.debug(
"JwtIdentityProviderEnum.MAA_NoSecureBootIntgTest",
JwtIdentityProviderEnum.MAA_NoSecureBootIntgTest,
this.logContext
);
this.identityProviders.set(
JwtIdentityProviderEnum.MS_AAD,
new MsJwtProvider("JwtProvider", this.logContext),
Expand Down
137 changes: 83 additions & 54 deletions src/policies/KeyReleasePolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,75 @@ export class KeyReleasePolicy implements IKeyReleasePolicy {
"x-ms-attestation-type": ["snp"],
};

private static checkSubset(//Check if o1 is a subset of o2
o1: number | string | boolean | Record<string, any>,
o2: number | string | boolean | Record<string, any>
): boolean {
// If both values are primitives, compare them directly
if (o1 === null || o2 === null || typeof o1 !== 'object' || typeof o2 !== 'object') {
// Ensure neither is an object
if (typeof o1 === 'object' || typeof o2 === 'object') {
return false;
}
// Both should be primities, compare them directly
return o1.toString() === o2.toString();
}

// Both values are objects; compare keys
for (const key in o1) {
if (o2.hasOwnProperty(key)) {
if (!KeyReleasePolicy.checkSubset(o1[key], o2[key])) {
return false;
}
} else {
// If a key in o1 does not exist in o2, return false
return false;
}
}

// All checks passed, return true
return true;
}

private static checkValuesLte( //Check if all values in o1 are smaller than or equal to the corresponding values in o2
type: string,
o1: number | string | Record<string, any>,
o2: number | string | Record<string, any>
): boolean
{
// Helper to compare primitive values
const comparePrimitives = (a: number | string, b: number | string): boolean => {
const numA = typeof a === 'string' ? parseFloat(a) : a;
const numB = typeof b === 'string' ? parseFloat(b) : b;
if (isNaN(numA) || isNaN(numB)) return false;
return type === "gte" ? numB >= numA : numB > numA;
};

// If either value is null or not an object, compare primitives directly
if (o1 === null || o2 === null || typeof o1 !== 'object' || typeof o2 !== 'object') {
// Ensure neither is an object
if (typeof o1 === 'object' || typeof o2 === 'object') return false;

// Both should be primities, compare them directly
return comparePrimitives(o1 as number | string, o2 as number | string);
}

// Both are objects, we compare keys
for (let key in o1) {
if (o2.hasOwnProperty(key)) {
if (!KeyReleasePolicy.checkValuesLte(type, o1[key], o2[key])) {
return false;
}
}
else {
// If key in o1 does not exist in o2, return false
return false;
}
}
// All checks passed, return true
return true;
}

private static validateKeyReleasePolicyClaims(
keyReleasePolicyClaims: IKeyReleasePolicySnpProps,
attestationClaims: IAttestationReport,
Expand Down Expand Up @@ -60,7 +129,7 @@ export class KeyReleasePolicy implements IKeyReleasePolicy {
if (
policyValue.filter((p) => {
Logger.debug(`Check if policy value ${p} === ${attestationValue}`, logContext);
return p.toString() === attestationValue.toString();
return KeyReleasePolicy.checkSubset(p, attestationValue);
}).length === 0
) {
return ServiceResult.Failed<string>(
Expand Down Expand Up @@ -98,13 +167,15 @@ export class KeyReleasePolicy implements IKeyReleasePolicy {
logContext,
);
}
const gte = type === "gte";

for (let inx = 0; inx < Object.keys(keyReleasePolicyClaims).length; inx++) {
const key = Object.keys(keyReleasePolicyClaims)[inx];

// check if key is in attestation
let attestationValue = attestationClaims[key];
let policyValue = keyReleasePolicyClaims[key];


const isUndefined = typeof attestationValue === "undefined";
Logger.debug(
`Checking key ${key}, typeof attestationValue: ${typeof attestationValue}, isUndefined: ${isUndefined}, attestation value: ${attestationValue}, policyValue: ${policyValue}`,
Expand All @@ -128,66 +199,20 @@ export class KeyReleasePolicy implements IKeyReleasePolicy {
logContext
);
}
if (
typeof policyValue !== "number" &&
(typeof policyValue !== "string" || isNaN(parseFloat(policyValue)))
) {
return ServiceResult.Failed<string>(
{
errorMessage: `Policy value for claim ${key} is not a number or a string representing a number for operator type ${type}`,
},
400,
logContext,
);
}

if (typeof policyValue !== "number") {
policyValue = parseFloat(policyValue);
}

if (typeof policyValue !== "number") {
Logger.info(
`Checking if attestation value ${attestationValue} is greater than (or equal) to policy value ${policyValue}`,
logContext,
);
if (!KeyReleasePolicy.checkValuesLte(type, policyValue, attestationValue)) {
return ServiceResult.Failed<string>(
{
errorMessage: `Policy value for claim ${key} is not a number or a string representing a number for operator type ${type} after conversion`,
errorMessage: `Attestation claim ${key}, value ${attestationValue} is not greater than (or equal) to policy value ${policyValue}`,
},
400,
logContext,
);
}

if (typeof attestationValue !== "number") {
attestationValue = parseFloat(attestationValue);
}

if (gte) {
Logger.info(
`Checking if attestation value ${attestationValue} is greater than or equal to policy value ${policyValue}`,
logContext,
);
if (attestationValue >= policyValue === false) {
return ServiceResult.Failed<string>(
{
errorMessage: `Attestation claim ${key}, value ${attestationValue} is not greater than or equal to policy value ${policyValue}`,
},
400,
logContext,
);
}
} else {
Logger.info(
`Checking if attestation value ${attestationValue} is greater than policy value ${policyValue}`,
logContext,
);
if (attestationValue > policyValue === false) {
return ServiceResult.Failed<string>(
{
errorMessage: `Attestation claim ${key}, value ${attestationValue} is not greater than policy value ${policyValue}`,
},
400,
logContext,
);
}
}
}
return ServiceResult.Succeeded<IAttestationReport>(attestationClaims, undefined, logContext);
}
Expand Down Expand Up @@ -232,6 +257,10 @@ export class KeyReleasePolicy implements IKeyReleasePolicy {
logContext
);
}
if (!policyValidationResult.success) {
return policyValidationResult;
}

if (keyReleasePolicy.gt !== null && keyReleasePolicy.gt !== undefined) {
Logger.info(`Validating gt operator`, logContext, keyReleasePolicy.gt);
policyValidationResult =
Expand Down