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

BFD-3679: Temporarily, correctly disable PAC endpoints during SAMHSA Incident #2453

Merged

Conversation

malessi
Copy link
Contributor

@malessi malessi commented Sep 30, 2024

JIRA Ticket:
BFD-3679

What Does This PR Do?

This PR correctly disables the PAC endpoints by setting the pac_resources_enabled SSM parameter to false rather than the pac/enabled SSM parameter.

Until BFD-3045 is merged the former parameter controls whether or not the PAC endpoints are enabled.

Additional Context

For context, the Java source looks for a configuration value named pac/enabled but the BFD Server does not yet self-configure from SSM. So, this value needs to be provided to the BFD Server in the form of an environment variable named BFD_PAC_ENABLED (BFD_-prefixed environment variables are converted to configuration paths by removing the BFD_ prefix, lowercasing, and then replacing underscores with slashes). Unfortunately, the value of BFD_PAC_ENABLED is provided by pac_resources_enabled in ops/ansible/roles/bfd-server/templates/bfd-server.sh.j2:57 rather than pac/enabled. Thus, when I checked the Java code to confirm the parameter to modify, I saw that the path was pac/enabled which matched the corresponding SSM parameter and assumed that the value of said parameter was being used. Obviously, this was not the case.

What Should Reviewers Watch For?

If you're reviewing this PR, please check for these things in particular:

What Security Implications Does This PR Have?

Please indicate if this PR does any of the following:

  • Adds any new software dependencies

  • Modifies any security controls

  • Adds new transmission or storage of data

  • Any other changes that could possibly affect security?

  • I have considered the above security implications as it relates to this PR. (If one or more of the above apply, it cannot be merged without the ISSO or team security engineer's (@sb-benohe) approval.)

Validation

Have you fully verified and tested these changes? Is the acceptance criteria met? Please provide reproducible testing instructions, code snippets, or screenshots as applicable.

  • terraform planning base in prod, verifying that the pac_resources_enabled SSM parameter does not have any unexpected changes (that is, its value remains false due to out-of-band changes)
  • Setting pac_resources_enabled to false in prod out-of-band and re-running the user data scripts on all prod instances, verifying that the PAC endpoints are properly disabled (that is, the value of pac_resources_enabled is respected)

Copy link
Member

@mjburling mjburling left a comment

Choose a reason for hiding this comment

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

Noted that we've been working closely under BFD-3045 and we'll soon be finished with all of these duplicate entries. 🤞

@malessi malessi merged commit 0492e61 into master Sep 30, 2024
6 checks passed
@malessi malessi deleted the alessio/BFD-3679__disable-pac-endpoints-for-real-this-time branch September 30, 2024 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants