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

Configure apps to use HTTPS egress-proxy #1064

Closed
1 task done
Tracked by #725
mogul opened this issue May 1, 2023 · 16 comments · Fixed by #1081
Closed
1 task done
Tracked by #725

Configure apps to use HTTPS egress-proxy #1064

mogul opened this issue May 1, 2023 · 16 comments · Fixed by #1081
Assignees
Labels
compliance Stuff which may relate to a specific requirement or timelines for resolution MUST Things we gotta do for the tracking epic to be "minimal"

Comments

@mogul
Copy link
Contributor

mogul commented May 1, 2023

At a glance

In order to ensure apps can make outbound HTTPS requests to necessary services even after their space is egress-restricted
as a system architect
I want all apps to make use of bound HTTPS egress-proxy credentials

Acceptance Criteria

We use DRY behavior-driven development wherever possible.

Scenario:

Given I have configured the [gsa-fac|clamav] app to use the proxy credentials provided in the egress-creds UPSI
when the [gsa-fac|clamav] app makes an outbound connection
...

then...

Shepherd

  • Design shepherd: N/A
  • Engineering shepherd: @mogul

Background

We deployed an egress proxy service in #1015. This issue is about ensuring the apps pay attention to the proxy settings.

Security Considerations

Required per CM-4.

This change will cause apps to funnel all outbound requests through a controlled interface, which meets the intent of NIST control SC-7. A further change to block all outbound requests that are not going through the proxy, at the space level, is out of scope but coming as soon as we can also proxy SMTP requests.


Process checklist

Sketch

  • Design designs all the things
  • Engineering engineers all the things

Definition of Done

Triage

If not likely to be important in the next quarter...

  • Archived from the board

Otherwise...

  • Has a clear story statement
  • Design or Engineering accepts that it belongs in their respective backlog

Design Backlog

  • Has clearly stated/testable acceptance criteria
  • Meets the design Definition of Ready [citation needed]
  • A design shepherd has been identified

Design In Progress

  • Meets the design Definition of Done [citation needed]

Design Review Needed

  • Necessary outside review/sign-off was provided

Design Done

  • Presented in a sprint review
  • Includes screenshots or references to artifacts

If no engineering is necessary

  • Tagged with the sprint where it was finished
  • Archived

Engineering Backlog

  • Has clearly stated/testable acceptance criteria
  • Has a sketch or list of tasks
  • Can reasonably be done in a few days (otherwise, split this up!)

Engineering Available

  • There's capacity in the In Progress column
  • An engineering shepherd has been identified

Engineering In Progress

If there's UI...

  • Screen reader - Listen to the experience with a screen reader extension, ensure the information presented in order
  • Keyboard navigation - Run through acceptance criteria with keyboard tabs, ensure it works.
  • Text scaling - Adjust viewport to 1280 pixels wide and zoom to 200%, ensure everything renders as expected. Document 400% zoom issues with USWDS if appropriate.

Engineering Blocked

  • Blocker removed/resolved

Engineering Review Needed

  • Outside review/sign-off was provided

Engineering Done

  • Presented in a sprint review
  • Includes screenshots or references to artifacts
  • Tagged with the sprint where it was finished
  • Archived
@mogul mogul added the compliance Stuff which may relate to a specific requirement or timelines for resolution label May 1, 2023
@mogul mogul self-assigned this May 1, 2023
@mogul mogul linked a pull request May 4, 2023 that will close this issue
@mogul
Copy link
Contributor Author

mogul commented May 5, 2023

@asteel-gsa and I were having trouble confirming whether fac-av-dev was using the https_proxy environment variable or not, since we saw it was successfully updating the virus definitions database but there were no logs on the proxy. It turns out that...

The freshclam program is not designed to recognize standard Linux environmental variables used by Security Analytics which define the web proxy.

So we might need to make a custom ClamAV image or else deploy ClamAV with a buildpack to make the necessary changes to the config file at app startup.

@mogul
Copy link
Contributor Author

mogul commented May 5, 2023

So we might need to make a custom ClamAV image or else deploy ClamAV with a buildpack to make the necessary changes to the config file at app startup.

We resolved a previous problem by making an upstream PR that was accepted within one day; I'm going to try the same thing here.

@mogul
Copy link
Contributor Author

mogul commented May 5, 2023

Summary of the approach: In the entrypoint.sh script that runs at container startup, if the https_proxy environment variable is set, use a regexp to extract the protocol, username, password, host, and port elements from the https_proxy URL, and then use sed to make edits to the freshclam.conf file to add the proxy settings (similar to the existing lines that make edits to the clamd.conf file).

@mogul mogul reopened this May 5, 2023
@mogul
Copy link
Contributor Author

mogul commented May 5, 2023

For the swagger application: If the https_proxy variable is set while staging (pushing) the application, the docker process attempts to use it to reach Docker Hub and fails:

mogul@rocinante-w11:~/Documents/Code$ cf set-env swagger https_proxy [REDACTED]
Setting env variable https_proxy for app swagger in org gsa-tts-oros-fac / space dev as bret.mogilefsky@gsa.gov...
OK

TIP: Use 'cf restage swagger' to ensure your env variable changes take effect.

mogul@rocinante-w11:~/Documents/Code$ cf restage swagger
This action will cause app downtime.

Restaging app swagger in org gsa-tts-oros-fac / space dev as bret.mogilefsky@gsa.gov...

Staging app and tracing logs...
   Cell fca078c1-c655-4d4e-8dde-001c634c490b creating container for instance 7a241e19-487b-4c8c-98e3-0291490c9109
   Security group rules were updated
   Cell fca078c1-c655-4d4e-8dde-001c634c490b successfully created container for instance 7a241e19-487b-4c8c-98e3-0291490c9109
   Staging...
   Staging process started ...
   Failed getting docker image manifest by tag: pinging docker registry returned: Get "https://registry-1.docker.io/v2/": Forbidden  Going to retry attempt: 1
   Failed getting docker image manifest by tag: pinging docker registry returned: Get "https://registry-1.docker.io/v2/": Forbidden  Going to retry attempt: 2
   Failed getting docker image manifest by tag: pinging docker registry returned: Get "https://registry-1.docker.io/v2/": Forbidden  Going to retry attempt: 3
   Failed getting docker image manifest by tag: pinging docker registry returned: Get "https://registry-1.docker.io/v2/": Forbidden
   Staging process failed: Exit trace for group:
   builder exited with error: failed to fetch metadata from [swaggerapi/swagger-ui] with tag [latest] and insecure registries [] due to pinging docker registry returned: Get "https://registry-1.docker.io/v2/": Forbidden
   Exit status 2
   Staging Failed: STG: Exited with status 2
   Cell fca078c1-c655-4d4e-8dde-001c634c490b stopping instance 7a241e19-487b-4c8c-98e3-0291490c9109
   Cell fca078c1-c655-4d4e-8dde-001c634c490b destroying container for instance 7a241e19-487b-4c8c-98e3-0291490c9109
Error staging application: StagingError - Staging error: staging failed
FAILED

So we need to set the value for the https_proxy variable under another name prior to staging, then set it again based on that value under the right name at app startup. This is probably doable with a start-command, but I haven't done that with Docker much in the past.

@mogul
Copy link
Contributor Author

mogul commented May 8, 2023

For the swagger application: If the https_proxy variable is set while staging (pushing) the application, the docker process attempts to use it to reach Docker Hub and fails:

[...]

So we need to set the value for the https_proxy variable under another name prior to staging, then set it again based on that value under the right name at app startup. This is probably doable with a start-command, but I haven't done that with Docker much in the past.

This was looking iffy, so we sidestepped the need for swagger to use the egress-proxy by having it point to a private route on the apps.internal domain instead.

@mogul
Copy link
Contributor Author

mogul commented May 9, 2023

From DMs with Alex today:

Hey based on this I think we need have our PR to clamav-rest to get the values from some variable name other than https_proxy.
Maybe just have it accept the variables for the settings just as it does the others:
https://github.com/ajilach/clamav-rest#environment-variables
And then we can change the egress-proxy module to output exactly the values we need rather than round-tripping through the URL.

@asteel-gsa
Copy link
Contributor

Added a PR for clamav-rest: PR 20

@mogul
Copy link
Contributor Author

mogul commented May 15, 2023

This PR should enable the proxy for the gsa-fac application.

@mogul
Copy link
Contributor Author

mogul commented May 19, 2023

Summary of where we are here:

  • gsa-fac is properly accessing login.gov and api.sam.gov via the proxy, and sending data to New Relic
  • Alex made a PR for clamav-rest that is still pending.
    • If we don't see movement on this by the middle of next week we'll start building/maintaining our own image.
  • postgrest does not need to use the proxy because it only accesses the bound DB services
  • swagger does not need to use the proxy because it operates in the user's browser and only accesses the public postgrest API endpoint

@mogul
Copy link
Contributor Author

mogul commented May 24, 2023

  • If we don't see movement on this by the middle of next week we'll start building/maintaining our own image.

@asteel-gsa Given no response from the upstream I guess we will need to start maintaining your fork. Want to move asteel-gsa/clamav-rest into GSA-TTS and set up a GitHub action to build, scan, and push it to GHCR?

@asteel-gsa
Copy link
Contributor

  • If we don't see movement on this by the middle of next week we'll start building/maintaining our own image.

@asteel-gsa Given no response from the upstream I guess we will need to start maintaining your fork. Want to move asteel-gsa/clamav-rest into GSA-TTS and set up a GitHub action to build, scan, and push it to GHCR?

Sure! What would be the best way to move it into GSA-TTS?

@asteel-gsa
Copy link
Contributor

Summary of where we are here:

  • gsa-fac is properly accessing login.gov and api.sam.gov via the proxy, and sending data to New Relic

  • Alex made a PR for clamav-rest that is still pending.

    • If we don't see movement on this by the middle of next week we'll start building/maintaining our own image.
  • postgrest does not need to use the proxy because it only accesses the bound DB services

  • swagger does not need to use the proxy because it operates in the user's browser and only accesses the public postgrest API endpoint

docker pull ghcr.io/gsa-tts/clamav-rest/clamav:20230530 TTS ClamAV

@mogul
Copy link
Contributor Author

mogul commented May 31, 2023

Still to do:

@asteel-gsa
Copy link
Contributor

Still to do:

Draft PR for FAC: Updating outputs for https proxy
PR for 18F/Terraform to update clamav: clamav update

@mogul
Copy link
Contributor Author

mogul commented Jun 13, 2023

As of this week's prod deploy, we believe this is done.

@mogul mogul closed this as completed Jun 13, 2023
@mogul mogul reopened this Jun 22, 2023
@mogul
Copy link
Contributor Author

mogul commented Jun 22, 2023

We still need to configure ClamAV to use the proxy.

@mogul mogul closed this as completed Jul 12, 2023
@mogul mogul added the MUST Things we gotta do for the tracking epic to be "minimal" label Jul 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compliance Stuff which may relate to a specific requirement or timelines for resolution MUST Things we gotta do for the tracking epic to be "minimal"
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants