Skip to content

Commit 3f31550

Browse files
committed
Use PubSub JWT auth for security instead of the hardcoded static key
1 parent 4fc29d6 commit 3f31550

10 files changed

+90
-90
lines changed

README.md

+12-3
Original file line numberDiff line numberDiff line change
@@ -83,21 +83,29 @@ names start `_gcp_`. The part of the function name after `_gcp_` is used for the
8383

8484
## Before deploying
8585

86-
### Org and IAM
86+
### In which Project
8787

8888
* You can deploy Iris in any project within your Google Cloud organization, but we recommend using a
8989
[new project](https://cloud.google.com/resource-manager/docs/creating-managing-projects#creating_a_project).
9090

91+
### Needed roles for deployment
92+
#### Organization-leve roles
93+
9194
* Here are the required organization-level roles for you, the deployer, to allow the deploy script to set up roles and log sink. (Note that *Organization Owner* is not enough).
9295
* *Organization Role Administrator* so the deployment script can create a custom IAM role for Iris that allows it to get and set labels.
9396
* *Security Admin* so the deployment script can grant the needed role bindings, e.g., to the App Engine service account.
9497
* *Logs Configuration Writer* so the deployment script can create an organization log sink that sends logs to
9598
PubSub.
9699

97-
* The required project-level roles: *Project Owner* or *Project Editor* on the project where Iris is deployed, so that the deployment script can create role bindings, topics and subscriptions, and deploy App Engine. Fine-granted "predefined roles" are not possible because deploying Cloud Scheduler cron requires at least Editor or Owner, per GCP docs.
100+
#### Project-level roles
101+
* The required project-level roles: *Project Owner* or *Project Editor* on the project where Iris is deployed, so that the deployment script can
102+
* create role bindings, topics and subscriptions
103+
* deploy App Engine.
104+
* `actAs` the serivice account `iris-msg-sender` for deploying it to allow JWT auth.
98105

99-
### App Engine Defaults
106+
* Fine-granted "predefined roles" are not possible because deploying Cloud Scheduler cron requires at least Editor or Owner, per GCP docs.
100107

108+
### App Engine Defaults
101109

102110
## Deployment
103111

@@ -159,6 +167,7 @@ names start `_gcp_`. The part of the function name after `_gcp_` is used for the
159167
* Another topic is a dead-letter topic.
160168
* PubSub subscriptions
161169
* There is one for each topic: These direct the messages to `/label_one` and `/do_label` in `main.py`, respectively.
170+
* For security, these two PubSub subscriptions [use JWT auth](These https://cloud.google.com/pubsub/docs/authenticate-push-subscriptions). The deployment script sets this up for you.
162171
* A dead-letter subscription. This is a pull subscription. By default, it just accumulates the messages. You can use it to see statistics, or you can pull messages from it.
163172

164173
# Local Development

TODO.md

+3-15
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,10 @@
55
* P2 Memory consumption: Even an empty AppEngine app (not Iris, just a Hello World with 3 lines of code in total) crashes on out-of-memory for the smalled AppEngine instance. Google has confirmed this. See if there is a workaround. This will save money.
66

77
* P2 PubSub push endpoint security:
8-
Note: The token by itself is not very secure, though
9-
Google has at times recommended this approach.
10-
11-
Alternatives:
12-
- Replace the `PUBSUB_VERIFICATION_TOKEN` with random value in `deploy.sh`
13-
- Or better: [Use JWT](https://cloud.google.com/pubsub/docs/push)
8+
Note: The token by itself is not very secure, though Google has at times recommended this approach.
149

15-
* P3 In `integration_test.sh`
16-
- Test more labels (in addition to `iris3_name` which is now tested)
17-
- Test the copying of labels from the project.
18-
- Support testing of the cron-based labeling, which would also allow testing of Cloud SQL and of attachment of
19-
Disks. In this test:
20-
1. Modify cron to run 1 minute after the deployment is launched (and restore it at the end of the test.)
21-
1. Call `deploy.sh` using with the `-c` switch to disable event-based labeling 1. Wait 1.5 minutes after deploy
22-
before checking that the labels are there
10+
[Use JWT](https://cloud.google.com/pubsub/docs/push)
11+
2312

2413
* P3 Label immediately after an event in certain cases, as opposed to using a daily cron as is now done.
2514
* Cloud SQL Instances
@@ -41,7 +30,6 @@
4130

4231
* P3 Rethink the need for title case in class names. This is clumsy for `Cloudsql`.
4332

44-
4533
* P4 Implement new labels, for example using ideas from
4634
the [GCP Auto Tag project](https://github.com/doitintl/gcp-auto-tag/)
4735
But **don't add too many**: There are *a lot* of fields on resources.

config.yaml.original

-12
Original file line numberDiff line numberDiff line change
@@ -51,15 +51,3 @@ from_project: True
5151

5252
label_all_on_cron: False
5353

54-
# Optionally change this token before first deployment for added security in
55-
# communication between PubSub and the Iris App on App Engine.
56-
# You could even re-generate a new token per deployment.
57-
# Note that this token-based approach is not very secure, though it was once recommended by Google.
58-
# However, so long as the GCP project running the Iris3 AppEngine service is otherwise secure,
59-
# setting this token protects against unwanted invocations of labeling.
60-
# If you then redeploy, don't change the token, as it is used for
61-
# communication between different parts of the system.
62-
pubsub_verification_token: 0b0a30cde7e3489f0a9cd74bb51c514d
63-
64-
65-

config.yaml.test.template

+1-2
Original file line numberDiff line numberDiff line change
@@ -11,5 +11,4 @@ specific_prefixes:
1111
Buckets: gcs${RUN_ID}
1212
from_project: True
1313
label_all_on_cron: False
14-
# We generate a random token per-test-run as an additional isolation measure
15-
pubsub_verification_token: ${PUBSUB_TEST_TOKEN}
14+

main.py

+50-25
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,21 @@
22

33
import sys
44

5-
from flask import Response
5+
print("Initializing ", file=sys.stderr)
6+
from util.utils import init_logging, sort_dict
67

8+
# Must init logging before any library code writes logs (which would then just override our config)
9+
init_logging()
710

8-
print("Initializing ", file=sys.stderr)
911
import flask
12+
from flask import Response
13+
from google.auth.transport import requests
14+
from google.oauth2 import id_token
1015
from util.gcp.gcp_utils import (
1116
increment_invocation_count,
1217
count_invocations_by_path,
18+
get_project,
19+
current_project_id,
1320
)
1421
from util.gcp.detect_gae import detect_gae
1522

@@ -19,11 +26,6 @@
1926

2027
app.wsgi_app = google.appengine.api.wrap_wsgi_app(app.wsgi_app)
2128

22-
from util.utils import init_logging, sort_dict
23-
24-
# Must init logging before any library code writes logs (which would then just override our config)
25-
init_logging()
26-
2729
from functools import lru_cache
2830

2931
from typing import Dict, Type
@@ -49,7 +51,6 @@
4951

5052
from util.config_utils import (
5153
is_project_enabled,
52-
pubsub_token,
5354
iris_homepage_text,
5455
)
5556
from util.utils import log_time, timing
@@ -96,7 +97,7 @@ def schedule():
9697
"""
9798
Send out a message per-plugin per-project to label all objects of that type and project.
9899
"""
99-
100+
# Not validated with JWT because validated with cron header (see below)
100101
increment_invocation_count("schedule")
101102
with gae_memory_logging("schedule"):
102103
try:
@@ -172,6 +173,11 @@ def __send_pubsub_per_projectplugin(configured_projects):
172173

173174
@app.route("/label_one", methods=["POST"])
174175
def label_one():
176+
"""Message received from PubSub when the log sink detects a new resource"""
177+
ok = __check_pubsub_jwt()
178+
if not ok:
179+
return "JWT Failed", 400
180+
175181
increment_invocation_count("label_one")
176182
with gae_memory_logging("label_one"):
177183

@@ -226,6 +232,37 @@ def label_one():
226232
return "Error", 500
227233

228234

235+
def __check_pubsub_jwt():
236+
try:
237+
bearer_token = flask.request.headers.get("Authorization")
238+
token = bearer_token.split(" ")[1]
239+
240+
claim = id_token.verify_oauth2_token(token, requests.Request())
241+
242+
# example claim: { "aud": "https://iris3-dot-myproj.appspot.com/label_one?token=xxxxxxxxx",
243+
# "azp": "1125239305332910191520",
244+
# "email": "[email protected]",
245+
# "email_verified": True, "exp": 1708281691, "iat": 1708278091,
246+
# "iss": "https://accounts.google.com", "sub": "1125239305332910191520"}
247+
248+
if not (is_email_verif := claim.get("email_verified")):
249+
logging.error(f"Email verified was {is_email_verif}")
250+
return False
251+
252+
logging.info("Claims: " + str(claim))
253+
if (
254+
email := claim.get("email")
255+
) != f"iris-msg-sender@{current_project_id()}.iam.gserviceaccount.com":
256+
logging.error("incorrect email in JWT " + email)
257+
return False
258+
except Exception as e:
259+
logging.exception(f"Invalid JWT token: {e}")
260+
return False
261+
logging.info("JWT Passed")
262+
263+
return True
264+
265+
229266
def __label_one_0(data, plugin_cls: Type[Plugin]):
230267
plugin = PluginHolder.get_plugin_instance(plugin_cls)
231268
gcp_object = plugin.get_gcp_object(data)
@@ -259,7 +296,6 @@ def __label_one_0(data, plugin_cls: Type[Plugin]):
259296
def __extract_pubsub_content() -> Dict:
260297
"""Take the value at the relevant key in the logging message from PubSub,
261298
Base64-decode, convert to Python object."""
262-
__check_pubsub_verification_token()
263299

264300
envelope = flask.request.get_json()
265301
msg = envelope.get("message", {})
@@ -283,6 +319,10 @@ def do_label():
283319
"""Receives a push message from PubSub, sent from schedule() above,
284320
and labels all objects of a given plugin and project_id.
285321
"""
322+
323+
ok = __check_pubsub_jwt()
324+
if not ok:
325+
return "JWT Failed", 400
286326
increment_invocation_count("do_label")
287327
with gae_memory_logging("do_label"):
288328

@@ -319,21 +359,6 @@ def do_label():
319359
return "Error", 500
320360

321361

322-
def __check_pubsub_verification_token():
323-
"""Token verifying that only PubSub accesses PubSub push endpoints"""
324-
expected_token = pubsub_token()
325-
if not expected_token:
326-
raise FlaskException(
327-
"Should define expected token in configuration.",
328-
400,
329-
)
330-
331-
token_from_args = flask.request.args.get("token", "")
332-
if expected_token != token_from_args:
333-
logging.info("Token was %s but expected %s", token_from_args, expected_token)
334-
raise FlaskException(f'Access denied: Invalid token "{expected_token}"', 403)
335-
336-
337362
class FlaskException(Exception):
338363
status_code = 400
339364

scripts/_deploy-project.sh

+16-4
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,9 @@ fi
3838

3939
gae_svc=$(grep "service:" app.yaml | awk '{print $2}')
4040

41-
# The following line depends on the the export PYTHON_PATH="." above.
42-
PUBSUB_VERIFICATION_TOKEN=$(python3 ./util/print_pubsub_token.py)
4341

44-
LABEL_ONE_SUBSCRIPTION_ENDPOINT="https://${gae_svc}-dot-${appengineHostname}/label_one?token=${PUBSUB_VERIFICATION_TOKEN}"
45-
DO_LABEL_SUBSCRIPTION_ENDPOINT="https://${gae_svc}-dot-${appengineHostname}/do_label?token=${PUBSUB_VERIFICATION_TOKEN}"
42+
LABEL_ONE_SUBSCRIPTION_ENDPOINT="https://${gae_svc}-dot-${appengineHostname}/label_one"
43+
DO_LABEL_SUBSCRIPTION_ENDPOINT="https://${gae_svc}-dot-${appengineHostname}/do_label?token"
4644

4745
declare -A enabled_services
4846
while read -r svc _; do
@@ -102,6 +100,16 @@ fi
102100
project_number=$(gcloud projects describe $PROJECT_ID --format json|jq -r '.projectNumber')
103101
PUBSUB_SERVICE_ACCOUNT="service-${project_number}@gcp-sa-pubsub.iam.gserviceaccount.com"
104102

103+
msg_sender_sa_name=iris-msg-sender
104+
105+
gcloud iam service-accounts create --project $PROJECT_ID $msg_sender_sa_name ||true
106+
107+
MSGSENDER_SERVICE_ACCOUNT=${msg_sender_sa_name}@${PROJECT_ID}.iam.gserviceaccount.com
108+
109+
gcloud projects add-iam-policy-binding ${PROJECT_ID} \
110+
--member="serviceAccount:${MSGSENDER_SERVICE_ACCOUNT}"\
111+
--role='roles/iam.serviceAccountTokenCreator'
112+
105113
set +e
106114
# Allow Pubsub to publish into the deadletter topic
107115
BINDING_ERR_OUTPUT=$(gcloud pubsub topics add-iam-policy-binding $DEADLETTER_TOPIC \
@@ -136,6 +144,7 @@ if [[ $? -eq 0 ]]; then
136144
gcloud pubsub subscriptions update "$DO_LABEL_SUBSCRIPTION" \
137145
--project="$PROJECT_ID" \
138146
--push-endpoint "$DO_LABEL_SUBSCRIPTION_ENDPOINT" \
147+
--push-auth-service-account $MSGSENDER_SERVICE_ACCOUNT \
139148
--ack-deadline=$ACK_DEADLINE \
140149
--max-delivery-attempts=$MAX_DELIVERY_ATTEMPTS \
141150
--dead-letter-topic=$DEADLETTER_TOPIC \
@@ -147,6 +156,7 @@ else
147156
gcloud pubsub subscriptions create "$DO_LABEL_SUBSCRIPTION" \
148157
--topic "$SCHEDULELABELING_TOPIC" --project="$PROJECT_ID" \
149158
--push-endpoint "$DO_LABEL_SUBSCRIPTION_ENDPOINT" \
159+
--push-auth-service-account $MSGSENDER_SERVICE_ACCOUNT \
150160
--ack-deadline=$ACK_DEADLINE \
151161
--max-delivery-attempts=$MAX_DELIVERY_ATTEMPTS \
152162
--dead-letter-topic=$DEADLETTER_TOPIC \
@@ -179,6 +189,7 @@ else
179189
echo >&2 "Updating $LABEL_ONE_SUBSCRIPTION"
180190
gcloud pubsub subscriptions update "$LABEL_ONE_SUBSCRIPTION" --project="$PROJECT_ID" \
181191
--push-endpoint="$LABEL_ONE_SUBSCRIPTION_ENDPOINT" \
192+
--push-auth-service-account $MSGSENDER_SERVICE_ACCOUNT \
182193
--ack-deadline=$ACK_DEADLINE \
183194
--max-delivery-attempts=$MAX_DELIVERY_ATTEMPTS \
184195
--dead-letter-topic=$DEADLETTER_TOPIC \
@@ -188,6 +199,7 @@ else
188199
else
189200
gcloud pubsub subscriptions create "$LABEL_ONE_SUBSCRIPTION" --topic "$LOGS_TOPIC" --project="$PROJECT_ID" \
190201
--push-endpoint="$LABEL_ONE_SUBSCRIPTION_ENDPOINT" \
202+
--push-auth-service-account $MSGSENDER_SERVICE_ACCOUNT \
191203
--ack-deadline=$ACK_DEADLINE \
192204
--max-delivery-attempts=$MAX_DELIVERY_ATTEMPTS \
193205
--dead-letter-topic=$DEADLETTER_TOPIC \

test_scripts/utils_for_tests.py

+1-4
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@
1010
from urllib.error import URLError
1111
from urllib.parse import urlencode
1212

13-
from util.config_utils import pubsub_token
14-
1513
logging.basicConfig(format="%(levelname)s: %(message)s", level=logging.INFO)
1614
logging.getLogger("googleapiclient.discovery_cache").setLevel(logging.ERROR)
1715

@@ -42,9 +40,8 @@ def do_local_http(
4240
host_and_port = f"localhost:{LOCAL_PORT}"
4341
args_s = ""
4442
if extra_args:
45-
args_s = "&"
4643
args_s += urlencode(extra_args)
47-
url_ = f"http://{host_and_port}/{path}?token={pubsub_token()}{args_s}"
44+
url_ = f"http://{host_and_port}/{path}?{args_s}"
4845
req = request.Request(
4946
url_,
5047
data=data_bytes,

uninstall_scripts/_uninstall-for-project.sh

+7
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@ LABEL_ONE_SUBSCRIPTION=label_one
1616

1717
project_number=$(gcloud projects describe $PROJECT_ID --format json|jq -r '.projectNumber')
1818
PUBSUB_SERVICE_ACCOUNT="service-${project_number}@gcp-sa-pubsub.iam.gserviceaccount.com"
19+
msg_sender_sa_name=iris-msg-sender
20+
MSGSENDER_SERVICE_ACCOUNT=${msg_sender_sa_name}@${PROJECT_ID}.iam.gserviceaccount.com
1921

2022
gcloud pubsub topics remove-iam-policy-binding $DEADLETTER_TOPIC \
2123
--member="serviceAccount:$PUBSUB_SERVICE_ACCOUNT"\
@@ -29,6 +31,11 @@ gcloud pubsub subscriptions remove-iam-policy-binding $LABEL_ONE_SUBSCRIPTION \
2931
--member="serviceAccount:$PUBSUB_SERVICE_ACCOUNT"\
3032
--role="roles/pubsub.subscriber" --project $PROJECT_ID ||true
3133

34+
gcloud projects remove-iam-policy-binding --project ${PROJECT_ID} \
35+
--member="serviceAccount:${MSGSENDER_SERVICE_ACCOUNT}"\
36+
--role='roles/iam.serviceAccountTokenCreator'
37+
38+
3239
gcloud pubsub subscriptions delete $DEADLETTER_SUB --project="$PROJECT_ID" -q || true
3340
gcloud pubsub subscriptions delete "$DO_LABEL_SUBSCRIPTION" -q --project="$PROJECT_ID" ||true
3441
gcloud pubsub subscriptions delete "$LABEL_ONE_SUBSCRIPTION" --project="$PROJECT_ID" 2>/dev/null || true

util/config_utils.py

-13
Original file line numberDiff line numberDiff line change
@@ -61,19 +61,6 @@ def label_all_on_cron() -> bool:
6161
return ret
6262

6363

64-
def pubsub_token() -> str:
65-
config = get_config()
66-
ret = config.get("pubsub_verification_token")
67-
68-
return ret
69-
70-
71-
def get_config_redact_token():
72-
c = get_config().copy()
73-
c["pubsub_verification_token"] = "[REDACTED]"
74-
return c
75-
76-
7764
@functools.lru_cache
7865
def get_config() -> typing.Dict:
7966
test_config = "config-test.yaml"

util/print_pubsub_token.py

-12
This file was deleted.

0 commit comments

Comments
 (0)