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.

Sign up for GitHub

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

Jump to bottom

Fix client cert syntax #242

Merged
merged 5 commits into from
Nov 30, 2023
Merged

Fix client cert syntax #242

merged 5 commits into from
Nov 30, 2023

Conversation

mrinnetmaki
Copy link
Member

This PR fixes a problem in production where the client certificate was not used properly.
Also, it allows running the app on more recent versions of Node.
Fixes #241.

Risk analysis

  • Risks of the implementation in this pull request have been analyzed with following results:
  • No significant risks.

Significance

  • Analysis, is the software change considered significant (ref. MDCG 2020-3, chart C), has been made with following result:
  • Not significant change, can be made to the product certified according MDD.
    • Reasons for the decision - Fix a bug in client certificate management

Security check-up

https://github.com/OWASP/www-project-top-ten/blob/master/index.md

  • A01:2021-Broken Access Control - Fixes a bug related to Omatietovaranto access control. No effects on access control on our side.
  • A02:2021-Cryptographic Failures - Fixes a bug related to PKI. No other changes to implementation.
  • A03:2021-Injection - No changes to input data handling or database code.
  • A04:2021-Insecure Design - No changes to security design, but fixes an issue in security implementation.
  • A05:2021-Security Misconfiguration - No changes to security configuration, but fixes an issue in security implementation.
  • A06:2021-Vulnerable and Outdated Components - Improvement, allows to run the app on more recent platforms. No changes to dependencies. Especially react related dependencies do have vulnerabilities. However, we consider these dev dependencies, as none of the libraries are actually used in production, only when the UI is built.
  • A07:2021-Identification and Authentication Failures - Fixes a bug in authentication. No other changes.
  • A08:2021-Software and Data Integrity Failures - No changes to artifact management or build process.
  • A09:2021-Security Logging and Monitoring Failures - No changes to logging, no new needs identified.
  • A10:2021-Server-Side Request Forgery - No changes to url or link management or request processing.

@mrinnetmaki
Use both ways of sending the cert
fe30913 
fhir-kit-client still expects the key and cert to be in options... https://github.com/Vermonster/fhir-kit-client/blob/3b36f30f29f5bbeac356feed0db6ce3be3eeb711/lib/client.js#L33
@mrinnetmaki
Upgrade fhir-kit-client, set version to 1.4.27
ca4501d
@mrinnetmaki
Enable more recent Node versions
a7fa79c 
Also, move react-scripts back to dependencies, we need to run them in CI environment, and they won't if they are in dev-dependencies.
@mrinnetmaki
Fix request options syntax
1d4aa69
@mrinnetmaki
Set version to 1.4.28
a5c7020
@mrinnetmaki mrinnetmaki mentioned this pull request Oct 16, 2023
Merged
12 tasks
mrinnetmaki added a commit that referenced this pull request Nov 30, 2023
@mrinnetmaki
Set version to 1.4.29
c363b72 
1.4.28 was already used, see #242.
@mrinnetmaki mrinnetmaki merged commit 48d6843 into master Nov 30, 2023
1 check passed
@mrinnetmaki mrinnetmaki deleted the node-16 branch November 30, 2023 21:22
ReettaValimaki
ReettaValimaki reviewed Apr 3, 2024
View reviewed changes
Copy link

@ReettaValimaki ReettaValimaki left a comment

Choose a reason for hiding this comment

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

Approved post merge due to critical timing of the change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ReettaValimaki ReettaValimaki ReettaValimaki left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Client certification fails
2 participants
@mrinnetmaki @ReettaValimaki