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

Add license check for FIPS #181187

Merged
merged 53 commits into from
Jul 2, 2024
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
fd898bc
Adding check when getting the config for the sec plugin to verify FIP…
kc13greiner Jan 9, 2024
392bee9
Merge branch 'main' into feature/add_fips_config
kc13greiner Jan 9, 2024
eff7830
changing fips_mode -> fipsMode
kc13greiner Jan 11, 2024
be473ba
Merge branch 'main' into feature/add_fips_config
kc13greiner Jan 12, 2024
d6f8f5c
Merge branch 'main' into feature/add_fips_config
kc13greiner Jan 16, 2024
eccc9e9
Changing expect to log statement to clarify that it shouldnt throw an…
kc13greiner Jan 22, 2024
ea425ec
Merge branch 'main' into feature/add_fips_config
kc13greiner Jan 24, 2024
d2cc083
Merge branch 'main' into feature/add_fips_config
kc13greiner Feb 2, 2024
b65471c
Merge branch 'main' into feature/add_fips_config
kc13greiner Mar 12, 2024
a0b72e6
Merge branch 'feature/add_fips_config' into feature/fips_license_check
kc13greiner Apr 16, 2024
683531c
Adding license check for FIPS
kc13greiner Apr 18, 2024
03f4b9e
Removing unused line in snapshot
kc13greiner Apr 18, 2024
ff51670
Merge branch 'main' into feature/fips_license_check
kc13greiner Apr 18, 2024
8959fbb
Adding new property
kc13greiner Apr 18, 2024
d50f7c5
Refactoring usage of License in favor of just using SecurityLicense
kc13greiner Apr 23, 2024
b1fa762
Merge branch 'main' into feature/fips_license_check
kc13greiner Apr 23, 2024
9d4d035
Removing console logs
kc13greiner Apr 23, 2024
fd58057
Merge branch 'main' into feature/fips_license_check
kc13greiner Apr 23, 2024
16d7ad8
Fixing jest tests
kc13greiner Apr 23, 2024
2bee3ea
Merge branch 'main' into feature/fips_license_check
kc13greiner Apr 23, 2024
b2fc622
snapshot update
kc13greiner Apr 23, 2024
eb5d11f
Updating snapshots and expected licenses
kc13greiner Apr 23, 2024
b1cab38
Merge branch 'main' into feature/fips_license_check
kc13greiner Apr 23, 2024
abed490
Need to add logic that waits for the license to load, run the start c…
kc13greiner Apr 24, 2024
15ab04b
Merge branch 'main' into feature/fips_license_check
kc13greiner Apr 24, 2024
955cf08
Throwing error instead of exiting
kc13greiner Apr 24, 2024
bc31503
Merge branch 'main' into feature/fips_license_check
kc13greiner Apr 24, 2024
80a3867
Merge branch 'main' into feature/fips_license_check
kc13greiner May 30, 2024
cb93a25
Changing config to experimental and adding updated FIPS docs
kc13greiner May 30, 2024
4816c26
Merge branch 'main' into feature/fips_license_check
kc13greiner May 30, 2024
4daab8c
Removing merge conflict code
kc13greiner May 30, 2024
6672ff9
Merge branch 'main' into feature/fips_license_check
kc13greiner May 30, 2024
27c9d15
Updating jest test snapshot
kc13greiner May 30, 2024
58a9e1b
Merge branch 'main' into feature/fips_license_check
kc13greiner Jun 10, 2024
2126400
Exposing fips setting from security setup
kc13greiner Jun 13, 2024
1b91334
Merge branch 'main' into feature/fips_license_check
kc13greiner Jun 13, 2024
d871efc
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Jun 13, 2024
c37004a
[DOCS] Minor doc edits
lcawl Jun 18, 2024
90050f1
Merge branch 'main' into feature/fips_license_check
lcawl Jun 18, 2024
57c2f36
Exposing fips flag from core
kc13greiner Jun 20, 2024
ff3586a
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Jun 20, 2024
31b7546
Fixing config type and adding tests
kc13greiner Jun 20, 2024
7b1a16a
Merge branch 'main' into feature/fips_license_check
kc13greiner Jun 20, 2024
d5cd54e
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Jun 20, 2024
19244c4
fixing mocks
kc13greiner Jun 20, 2024
d468deb
Merge branch 'main' into feature/fips_license_check
kc13greiner Jun 20, 2024
b7f1de0
Removing tests
kc13greiner Jun 20, 2024
7b7ed01
PR feedback from Pierre
kc13greiner Jun 25, 2024
50015b0
Merge branch 'main' into feature/fips_license_check
kc13greiner Jun 25, 2024
2c11610
Moving FIPS config comparison check to Core > Security Service setup
kc13greiner Jun 26, 2024
fccc836
Merge branch 'main' into feature/fips_license_check
kc13greiner Jun 26, 2024
b82345d
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Jun 26, 2024
33db0d6
Merge branch 'main' into feature/fips_license_check
kc13greiner Jul 2, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,7 @@ kibana_vars=(
xpack.security.authc.selector.enabled
xpack.security.cookieName
xpack.security.encryptionKey
xpack.security.fipsMode.enabled
xpack.security.loginAssistanceMessage
xpack.security.loginHelp
xpack.security.sameSiteCookies
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type { SecurityLicenseFeatures } from './license_features';

export interface SecurityLicense {
isLicenseAvailable(): boolean;
getLicenseType(): string | undefined;
getUnavailableReason: () => string | undefined;
isEnabled(): boolean;
getFeatures(): SecurityLicenseFeatures;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,4 +78,10 @@ export interface SecurityLicenseFeatures {
* Describes the layout of the login form if it's displayed.
*/
readonly layout?: LoginLayout;

/**
* Indicates whether we allow FIPS mode
*/

readonly allowFips: boolean;
}
27 changes: 24 additions & 3 deletions x-pack/plugins/security/common/licensing/index.mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,33 @@ import type { SecurityLicense, SecurityLicenseFeatures } from '@kbn/security-plu
export const licenseMock = {
create: (
features: Partial<SecurityLicenseFeatures> | Observable<Partial<SecurityLicenseFeatures>> = {},
licenseType: LicenseType = 'basic' // default to basic if this is not specified
licenseType: LicenseType = 'basic', // default to basic if this is not specified,
isAvailable: Observable<boolean> = of(true)
): jest.Mocked<SecurityLicense> => ({
isLicenseAvailable: jest.fn().mockReturnValue(true),
isLicenseAvailable: jest.fn().mockImplementation(() => {
let result = true;

isAvailable.subscribe((next) => {
result = next;
});

return result;
}),
getLicenseType: jest.fn().mockReturnValue(licenseType),
getUnavailableReason: jest.fn(),
isEnabled: jest.fn().mockReturnValue(true),
getFeatures: jest.fn().mockReturnValue(features),
getFeatures:
features instanceof Observable
? jest.fn().mockImplementation(() => {
let subbedFeatures: Partial<SecurityLicenseFeatures> = {};

features.subscribe((next) => {
subbedFeatures = next;
});

return subbedFeatures;
})
: jest.fn().mockReturnValue(features),
hasAtLeast: jest
.fn()
.mockImplementation(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ describe('license features', function () {
allowSubFeaturePrivileges: false,
allowAuditLogging: false,
allowUserProfileCollaboration: false,
allowFips: false,
});
});

Expand All @@ -55,6 +56,7 @@ describe('license features', function () {
allowSubFeaturePrivileges: false,
allowAuditLogging: false,
allowUserProfileCollaboration: false,
allowFips: false,
});
});

Expand All @@ -76,6 +78,7 @@ describe('license features', function () {
Object {
"allowAccessAgreement": false,
"allowAuditLogging": false,
"allowFips": false,
"allowLogin": false,
"allowRbac": false,
"allowRoleDocumentLevelSecurity": false,
Expand All @@ -99,6 +102,7 @@ describe('license features', function () {
Object {
"allowAccessAgreement": true,
"allowAuditLogging": true,
"allowFips": true,
"allowLogin": true,
"allowRbac": true,
"allowRoleDocumentLevelSecurity": true,
Expand Down Expand Up @@ -141,6 +145,7 @@ describe('license features', function () {
allowSubFeaturePrivileges: false,
allowAuditLogging: false,
allowUserProfileCollaboration: false,
allowFips: false,
});
expect(getFeatureSpy).toHaveBeenCalledTimes(1);
expect(getFeatureSpy).toHaveBeenCalledWith('security');
Expand Down Expand Up @@ -168,6 +173,7 @@ describe('license features', function () {
allowSubFeaturePrivileges: false,
allowAuditLogging: false,
allowUserProfileCollaboration: false,
allowFips: false,
});
});

Expand All @@ -194,6 +200,7 @@ describe('license features', function () {
allowSubFeaturePrivileges: false,
allowAuditLogging: false,
allowUserProfileCollaboration: true,
allowFips: false,
});
});

Expand All @@ -220,6 +227,7 @@ describe('license features', function () {
allowSubFeaturePrivileges: true,
allowAuditLogging: true,
allowUserProfileCollaboration: true,
allowFips: false,
});
});

Expand All @@ -246,6 +254,7 @@ describe('license features', function () {
allowSubFeaturePrivileges: true,
allowAuditLogging: true,
allowUserProfileCollaboration: true,
allowFips: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: update test description to include "allow FIPS"

});
});
});
5 changes: 5 additions & 0 deletions x-pack/plugins/security/common/licensing/license_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ export class SecurityLicenseService {
license: Object.freeze({
isLicenseAvailable: () => rawLicense?.isAvailable ?? false,

getLicenseType: () => rawLicense?.type ?? undefined,

getUnavailableReason: () => rawLicense?.getUnavailableReason(),

isEnabled: () => this.isSecurityEnabledFromRawLicense(rawLicense),
Expand Down Expand Up @@ -80,6 +82,7 @@ export class SecurityLicenseService {
allowRbac: false,
allowSubFeaturePrivileges: false,
allowUserProfileCollaboration: false,
allowFips: false,
layout:
rawLicense !== undefined && !rawLicense?.isAvailable
? 'error-xpack-unavailable'
Expand All @@ -101,6 +104,7 @@ export class SecurityLicenseService {
allowRbac: false,
allowSubFeaturePrivileges: false,
allowUserProfileCollaboration: false,
allowFips: false,
};
}

Expand All @@ -121,6 +125,7 @@ export class SecurityLicenseService {
allowRoleRemoteIndexPrivileges: isLicensePlatinumOrBetter,
allowRbac: true,
allowUserProfileCollaboration: isLicenseStandardOrBetter,
allowFips: isLicensePlatinumOrBetter,
};
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions x-pack/plugins/security/public/plugin.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ describe('Security Plugin', () => {
authz: { isRoleManagementEnabled: expect.any(Function) },
license: {
isLicenseAvailable: expect.any(Function),
getLicenseType: expect.any(Function),
isEnabled: expect.any(Function),
getUnavailableReason: expect.any(Function),
getFeatures: expect.any(Function),
Expand Down Expand Up @@ -71,6 +72,7 @@ describe('Security Plugin', () => {
authc: { getCurrentUser: expect.any(Function), areAPIKeysEnabled: expect.any(Function) },
license: {
isLicenseAvailable: expect.any(Function),
getLicenseType: expect.any(Function),
isEnabled: expect.any(Function),
getUnavailableReason: expect.any(Function),
getFeatures: expect.any(Function),
Expand Down
97 changes: 97 additions & 0 deletions x-pack/plugins/security/server/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,13 @@
* 2.0.
*/

const mockGetFipsFn = jest.fn();
jest.mock('crypto', () => ({
randomBytes: jest.fn(),
constants: jest.requireActual('crypto').constants,
get getFips() {
return mockGetFipsFn;
},
}));

jest.mock('@kbn/utils', () => ({
Expand Down Expand Up @@ -61,6 +65,9 @@ describe('config schema', () => {
},
"cookieName": "sid",
"encryptionKey": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
"fipsMode": Object {
"enabled": false,
},
"loginAssistanceMessage": "",
"public": Object {},
"secureCookies": false,
Expand Down Expand Up @@ -115,6 +122,9 @@ describe('config schema', () => {
},
"cookieName": "sid",
"encryptionKey": "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
"fipsMode": Object {
"enabled": false,
},
"loginAssistanceMessage": "",
"public": Object {},
"secureCookies": false,
Expand Down Expand Up @@ -168,6 +178,9 @@ describe('config schema', () => {
"selector": Object {},
},
"cookieName": "sid",
"fipsMode": Object {
"enabled": false,
},
"loginAssistanceMessage": "",
"public": Object {},
"secureCookies": false,
Expand Down Expand Up @@ -224,6 +237,9 @@ describe('config schema', () => {
"selector": Object {},
},
"cookieName": "sid",
"fipsMode": Object {
"enabled": false,
},
"loginAssistanceMessage": "",
"public": Object {},
"roleManagementEnabled": false,
Expand Down Expand Up @@ -2446,3 +2462,84 @@ describe('createConfig()', () => {
});
});
});

describe('checkFipsConfig', () => {
let mockExit: jest.SpyInstance;
beforeAll(() => {
mockExit = jest.spyOn(process, 'exit').mockImplementation((exitCode) => {
throw new Error(`Fake Exit: ${exitCode}`);
});
});

afterAll(() => {
mockExit.mockRestore();
});

it('should log an error message if FIPS mode is misconfigured - xpack.security.fipsMode.enabled true, Nodejs FIPS mode false', async () => {
const logger = loggingSystemMock.create().get();

try {
createConfig(ConfigSchema.validate({ fipsMode: { enabled: true } }), logger, {
isTLSEnabled: true,
});
} catch (e) {
expect(mockExit).toHaveBeenNthCalledWith(1, 78);
}

expect(loggingSystemMock.collect(logger).error).toMatchInlineSnapshot(`
Array [
Array [
"Configuration mismatch error. xpack.security.fipsMode.enabled is set to true and the configured Node.js environment has FIPS disabled",
],
]
`);
});

it('should log an error message if FIPS mode is misconfigured - xpack.security.fipsMode.enabled false, Nodejs FIPS mode true', async () => {
mockGetFipsFn.mockImplementationOnce(() => {
return 1;
});

const logger = loggingSystemMock.create().get();

try {
createConfig(ConfigSchema.validate({ fipsMode: { enabled: false } }), logger, {
isTLSEnabled: true,
});
} catch (e) {
expect(mockExit).toHaveBeenNthCalledWith(1, 78);
}

expect(loggingSystemMock.collect(logger).error).toMatchInlineSnapshot(`
Array [
Array [
"Configuration mismatch error. xpack.security.fipsMode.enabled is set to false and the configured Node.js environment has FIPS enabled",
],
]
`);
});

it('should log an info message if FIPS mode is properly configured - xpack.security.fipsMode.enabled true, Nodejs FIPS mode true', async () => {
mockGetFipsFn.mockImplementationOnce(() => {
return 1;
});

const logger = loggingSystemMock.create().get();

try {
createConfig(ConfigSchema.validate({ fipsMode: { enabled: true } }), logger, {
isTLSEnabled: true,
});
} catch (e) {
logger.error('Should not throw error!');
}

expect(loggingSystemMock.collect(logger).info).toMatchInlineSnapshot(`
Array [
Array [
"Kibana is running in FIPS mode.",
],
]
`);
});
});
27 changes: 26 additions & 1 deletion x-pack/plugins/security/server/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import crypto from 'crypto';
import crypto, { getFips } from 'crypto';
import type { Duration } from 'moment';
import path from 'path';

Expand Down Expand Up @@ -313,13 +313,38 @@ export const ConfigSchema = schema.object({
roleMappingManagementEnabled: schema.boolean({ defaultValue: true }),
}),
}),
fipsMode: schema.object({
enabled: schema.boolean({ defaultValue: false }),
}),
});

function checkFipsConfig(config: RawConfigType, logger: Logger) {
const isFipsEnabled = config.fipsMode.enabled;
const isNodeRunningWithFipsEnabled = getFips() === 1;

// Check if FIPS is enabled in either setting
if (isFipsEnabled || isNodeRunningWithFipsEnabled) {
// FIPS must be enabled on both or log and error an exit Kibana
if (isFipsEnabled !== isNodeRunningWithFipsEnabled) {
logger.error(
`Configuration mismatch error. xpack.security.fipsMode.enabled is set to ${isFipsEnabled} and the configured Node.js environment has FIPS ${
isNodeRunningWithFipsEnabled ? 'enabled' : 'disabled'
}`
);
process.exit(78);
Copy link
Contributor

Choose a reason for hiding this comment

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

🙈 I gonna do like I did not saw a plugin's configuration validation call process.exit with a return code not used anywhere else in of our software.

More seriously though, can't this be done at Core's level? I would have way less concerns and issues with Core performing this check (or at least this non-standard process termination). This is done before the license is added to the equation, right? So from my understanding nothing blocks it from being done in Core?

(but if we can't I'll just 🙈)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😅 fair points, I'll take a look to see if there might be a better place for this check in core.

Did you have any place in mind that might be best?

Copy link
Contributor

Choose a reason for hiding this comment

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

Could this possibly be done in the setup phase of Core's security service (where you added the logic to return the isFIPS API from the contract)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 2c11610

} else {
logger.info('Kibana is running in FIPS mode.');
}
}
}

export function createConfig(
config: RawConfigType,
logger: Logger,
{ isTLSEnabled }: { isTLSEnabled: boolean }
) {
checkFipsConfig(config, logger);

let encryptionKey = config.encryptionKey;
if (encryptionKey === undefined) {
logger.warn(
Expand Down
Loading