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

Fix: do not redundantly authenticate notary calls #1351

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
23 changes: 23 additions & 0 deletions .github/actions/setup-notary/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: setup-notary-trust-data
description: 'Initialize trust data in running notary servvice container for existing test image'
runs:
using: "composite"
steps:
- name: Install notary and docker client
run: |
sudo apt install docker
sudo apt install notary
- name: Install expect
run: |
sudp apt install expect
- name: Trust root cert of notary instance
run: |
sudo cp ./tests/data/notary_service_container/certs/root/ca.crt /usr/local/share/ca-certificates/notary_root_ca.crt
sudo update-ca-certificates
- name: Configure notary client
run: |
./tests/integration/notary_init.sh
docker pull docker.io/securesystemsengineering/testimage:self-hosted-notary-signed
DIGEST=$(docker images --digests | grep self-hosted-notary-signed | awk '{print $3}')
DIGEST_WITHOUT_PREFIX=$(echo ${DIGEST#sha256:})
./tests/integration/notary_addhash.sh ${DIGEST_WITHOUT_PREFIX}
19 changes: 15 additions & 4 deletions .github/workflows/.reusable-integration-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ jobs:
image: securesystemsengineering/alerting-endpoint
ports:
- 56243:56243
notary:
image: notary:server
env:
NOTARY_SERVER_TRUST_SERVICE_TYPE: local
NOTARY_SERVER_STORAGE_BACKEND: memory
NOTARY_SERVER_LOGGING_LEVEL: debug
volumes:
- ./tests/data/notaryservice_container/certs/leaf/:/certs
steps:
- name: Checkout code
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
Expand All @@ -70,18 +78,21 @@ jobs:
- name: Install yq
run: |
sudo snap install yq
- name: Setup Test Notary Trust Data
uses: ./.github/actions/setup-notary
- uses: ./.github/actions/k8s-version-config
name: Setup k8s cluster
with:
k8s-version: v1.25
- name: Get alerting endpoint IP
id: get_ip
uses: ./.github/actions/alerting-endpoint
- name: Get alerting endpoint and notary IP
id: get_ips
uses: ./.github/actions/service-ips
- name: Run test
run: |
bash tests/integration/integration-test.sh "${{ matrix.integration-test-arg }}"
env:
ALERTING_ENDPOINT_IP: ${{ steps.get_ip.outputs.ip }}
ALERTING_ENDPOINT_IP: ${{ steps.get_ips.outputs.alerting_endpoint_ip }}
NOTARY_IP: ${{ steps.get_ips.outputs.notary_ip }}
- name: Display Connaisseur configuration
if: always()
run: |
Expand Down
1 change: 0 additions & 1 deletion connaisseur/flask_application.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,6 @@ async def __validate_image(
try:
policy_rule = CONFIG.get_policy_rule(image)
validator = CONFIG.get_validator(policy_rule.validator)

logging.debug(
'starting verification of image "%s" using rule "%s" with arguments %s and validator "%s".',
original_image,
Expand Down
73 changes: 45 additions & 28 deletions connaisseur/validators/notaryv1/notary.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,39 +89,46 @@ def healthy(self):
except Exception:
return False

async def get_trust_data(
self,
session: aiohttp.ClientSession,
image: Image,
role: TUFRole,
token: str = None,
):
im_repo = f"{image.repository}/" if image.repository else ""
url = (
f"https://{self.host}/v2/{image.registry}/{im_repo}"
f"{image.name}/_trust/tuf/{str(role)}.json"
)
async def get_auth(self, session: aiohttp.ClientSession, image: Image):
request_kwargs = self.__build_args(image, "root")

request_kwargs = {
"url": url,
"ssl": self.cert,
"headers": ({"Authorization": f"Bearer {token}"} if token else None),
}
async with session.get(**request_kwargs) as response:
status = response.status
if (
status == 401
and not token
and ("www-authenticate" in [k.lower() for k in response.headers])
if status == 401 and (
"www-authenticate" in [k.lower() for k in response.headers]
):
auth_url = self.__parse_auth(
{k.lower(): v for k, v in response.headers.items()}[
"www-authenticate"
]
)
token = await self.__get_auth_token(session, auth_url)
return await self.get_trust_data(session, image, role, token)
repo_token = await self.__get_auth_token(session, auth_url)
return repo_token
elif status == 401:
msg = (
"Unable to find auhorization endpoint in"
"'www-authenticate' header for notary {notary_name}."
)
raise NotFoundException(message=msg, notary_name=self.name)
else:
return None

async def get_trust_data(
self,
session: aiohttp.ClientSession,
image: Image,
role: TUFRole,
repo_token: str,
):
request_kwargs = self.__build_args(image, str(role))

if repo_token:
request_kwargs.update(
{"headers": ({"Authorization": f"Bearer {repo_token}"})}
)

async with session.get(**request_kwargs) as response:
status = response.status
if status == 404:
msg = "Unable to get {tuf_role} trust data from {notary_name}."
raise NotFoundException(
Expand All @@ -137,10 +144,10 @@ async def get_delegation_trust_data(
session: aiohttp.ClientSession,
image: Image,
role: TUFRole,
token: str = None,
repo_token: str,
):
try:
return await self.get_trust_data(session, image, role, token)
return await self.get_trust_data(session, image, role, repo_token)
except Exception as ex:
if ConnaisseurLoggingWrapper.is_debug_level():
raise ex
Expand Down Expand Up @@ -222,6 +229,7 @@ async def __get_auth_token(self, session: aiohttp.ClientSession, url: str):
"ssl": self.cert,
"auth": (aiohttp.BasicAuth(**self.auth) if self.auth else None),
}

async with session.get(**request_kwargs) as response:
if response.status >= 500:
msg = "Unable to get authentication token from {auth_url}."
Expand All @@ -233,7 +241,7 @@ async def __get_auth_token(self, session: aiohttp.ClientSession, url: str):

try:
token_key = "access_token" if self.is_acr else "token"
token = (await response.json(content_type=None))[token_key]
repo_token = (await response.json(content_type=None))[token_key]
except KeyError as err:
msg = (
"Unable to retrieve authentication token from {auth_url} response."
Expand All @@ -246,12 +254,21 @@ async def __get_auth_token(self, session: aiohttp.ClientSession, url: str):
r"^[A-Za-z0-9-_=]+\.[A-Za-z0-9-_=]+\.?[A-Za-z0-9-_.+/=]*$" # nosec
)

if not re.match(token_re, token):
if not re.match(token_re, repo_token):
msg = "{validation_kind} has an invalid format."
raise InvalidFormatException(
message=msg,
validation_kind="Authentication token",
notary_name=self.name,
auth_url=url,
)
return token
return repo_token

def __build_args(self, image: Image, role: str):
im_repo = f"{image.repository}/" if image.repository else ""
url = (
f"https://{self.host}/v2/{image.registry}/{im_repo}"
f"{image.name}/_trust/tuf/{role}.json"
)

return {"url": url, "ssl": self.cert}
12 changes: 8 additions & 4 deletions connaisseur/validators/notaryv1/notaryv1_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,13 @@ async def __process_chain_of_trust(

# load all trust data
t_start = dt.datetime.now()

# even if a registry is public like e.g. docker.io, it might still
# require a token on repository level, specifying the scope (e.g. pull)
repo_token = await self.notary.get_auth(session, image)
trust_data_list = await asyncio.gather(
*[
self.notary.get_trust_data(session, image, TUFRole(role))
self.notary.get_trust_data(session, image, TUFRole(role), repo_token)
for role in tuf_roles
]
)
Expand Down Expand Up @@ -185,7 +189,7 @@ async def __process_chain_of_trust(

# download only the required delegation files
await self.__update_with_delegation_trust_data(
session, trust_data, req_delegations, key_store, image
session, trust_data, req_delegations, key_store, image, repo_token
)

# if certain delegations are required, then only take the targets fields of the
Expand Down Expand Up @@ -263,12 +267,12 @@ def __search_image_targets(trust_data: dict, image: Image):
return None

async def __update_with_delegation_trust_data(
self, session: ClientSession, trust_data, delegations, key_store, image
self, session: ClientSession, trust_data, delegations, key_store, image, token
):
delegation_trust_data_list = await asyncio.gather(
*[
self.notary.get_delegation_trust_data(
session, image, TUFRole(delegation)
session, image, TUFRole(delegation), token
)
for delegation in delegations
]
Expand Down
7 changes: 3 additions & 4 deletions docs/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,9 @@ A simple starting point may be a minikube cluster with e.g. a [Docker Hub](https
In case you make changes to the Connaisseur container image itself or code for that matter, you need to re-build the image and install it locally for testing.
This requires a few steps:

1. In `helm/values.yaml`, set `imagePullPolicy` to `IfNotPresent`.
2. Configure your local environment to use the Kubernetes Docker daemon. In minikube, this can be done via `eval (minikube docker-env)`.
3. Build the Connaisseur container image via `make docker`.
4. Install Connaisseur as usual via `make install`.
1. Configure your local environment to use the Kubernetes Docker daemon. In minikube, this can be done via `eval $(minikube docker-env)`.
2. Build the Connaisseur container image via `make docker`.
3. Install Connaisseur as usual via `make dev-install`.

### Test changes
Tests and linting are important to ensure code quality, functionality and security.
Expand Down
3 changes: 2 additions & 1 deletion scripts/get_root_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
async def get_pub_root_key(host: str, image: Image):
notary = Notary("no", host, ["not_empty"])
async with (aiohttp.ClientSession()) as session:
root_td = await notary.get_trust_data(session, image, TUFRole("root"))
token = await notary.get_auth(session, image)
root_td = await notary.get_trust_data(session, image, TUFRole("root"), token)

root_key_id = root_td.signatures[0].get("keyid")
root_cert_base64 = (
Expand Down
19 changes: 12 additions & 7 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,16 @@ def mock_request_notary(match: re.Match, **kwargs):
match.group(5),
)

if registry == "auth.io" and not kwargs.get("headers"):
if kwargs.get("headers") and kwargs.get("headers").get("Authorization"):
if registry == "empty.io":
return MockResponse({}, status_code=404)

return MockResponse(get_td(f"{image}/{role}"))
elif registry == "empty.io":
return MockResponse({}, status_code=401)
elif registry == "notary_wo_auth.io":
return MockResponse(get_td(f"{image}/{role}"))
else:
return MockResponse(
{},
headers={
Expand All @@ -153,10 +162,6 @@ def mock_request_notary(match: re.Match, **kwargs):
},
status_code=401,
)
if registry == "empty.io":
return MockResponse({}, status_code=404)

return MockResponse(get_td(f"{image}/{role}"))


def mock_request_kube(match: re.Match, **kwargs):
Expand Down Expand Up @@ -399,12 +404,12 @@ def m_alerting_without_send(monkeypatch, m_safe_path_func, mocker):

@pytest.fixture
def count_loaded_delegations(monkeypatch):
async def get_delegation_trust_data_counted(self, image, role, token=None):
async def get_delegation_trust_data_counted(self, session, image, role, token):
monkeypatch.setenv(
"DELEGATION_COUNT", str(int(os.getenv("DELEGATION_COUNT")) + 1)
)
try:
return await no.Notary.get_trust_data(self, image, role, token)
return await no.Notary.get_trust_data(self, session, image, role, token)
except Exception as ex:
if os.environ.get("LOG_LEVEL", "INFO") == "DEBUG":
raise ex
Expand Down
32 changes: 32 additions & 0 deletions tests/data/notary/hanswurstnotary.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
name: harbor
host: notary.hans.io
cert: |
-----BEGIN CERTIFICATE-----
MIIDEzCCAfugAwIBAgIQEHy1Je1mbrt0RaLDjDajszANBgkqhkiG9w0BAQsFADAU
MRIwEAYDVQQDEwloYXJib3ItY2EwHhcNMjEwMTI2MTQyNTE5WhcNMjIwMTI2MTQy
NTE5WjAUMRIwEAYDVQQDEwloYXJib3ItY2EwggEiMA0GCSqGSIb3DQEBAQUAA4IB
DwAwggEKAoIBAQCfy2A79g4KGx1BN8LgNwF34pSJaKqzV9hsanNKi5iU6Sn2Qrjx
a++HlCYK8TAZ54cacP1T+d+eqlDwgMlbkXsjSFiRr3Z+KxtrrFbM9yNrNzyUiDVW
czUQM+PFEETk2uwp7GSHFFBXeo+6p/cI2vqSqxpkVVojKmX6vEdEdPh9mwBt9nuk
MNfaJxzzjpAPdH9TkWME+J+GpxuLhtRnE0PStC6ioYI4FeH5MCwLKv7ZVyxWYDpY
f5qG2H00rGNOHsq9jidyLbp90jboMbVHMO6ragM6sqrjPF/cLE8oifuguCR6dbVk
yQuIacfG/vglnp5juvFLDmf0ZVBytazWMUQzAgMBAAGjYTBfMA4GA1UdDwEB/wQE
AwICpDAdBgNVHSUEFjAUBggrBgEFBQcDAQYIKwYBBQUHAwIwDwYDVR0TAQH/BAUw
AwEB/zAdBgNVHQ4EFgQUwtWtGfG+NU6ZcqhJI+lKRHOW/qQwDQYJKoZIhvcNAQEL
BQADggEBABiBHCuadw+SlmQHuK9egZSzIjvaLdKcTWdYwICtzuymZyyAWxWGeY8O
ZRZ9ZvsVX8jgTsSlFe+nV/+3MokYCvCaaDmyre7zZmRsq65ILSrwJMWjSqyvt8/X
s78uvGgi8/ooP7eldlduOA3AdV81Ty9GeCWWqEVIjEgfVQhpYquNTyOcUp8Tuks6
5OkY1pS58NRkoIM6/jfGtgbzsvvHooZwslmq8eaT+MucuzuGpY2GelEE5pI9Q7tf
hMC42zeU+yxxy3vukMa4xX2BGzyjAg+qaDh6YwWui80r2/BlYXvSsSl3dIgtVwL4
DSo1s+3uJ4evVKDRf3vLwKLTtiYfd20=
-----END CERTIFICATE-----
trustRoots:
- name: library
key: |
-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEH+G0yM9CQ8KjN2cF8iHpiTA9Q69q
3brrzLkY1kjmRNOs0c2sx2nm8j2hFZRbyaVsd52Mkw0k5WrX+9vBfbjtdQ==
-----END PUBLIC KEY-----
auth:
username: hans
password: wurst
7 changes: 7 additions & 0 deletions tests/integration/cases.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,13 @@ test_cases:
namespace: default
expected_msg: Defaulting debug container name to debugger-
expected_result: VALID
- id: rsishn
txt: Testing signed image with trust data in self-hosted notary...
type: deploy
ref: securesystemsengineering/testimage:sef-hosted-notary-signed
namespace: default
expected_msg: pod/pod-rsishn-${RAND} created
expected_result: VALID
cosign:
- id: cu
txt: Testing unsigned cosign image...
Expand Down
2 changes: 2 additions & 0 deletions tests/integration/integration-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,8 @@ update_via_env_vars() {
yq '. *+ load("ghcr-values")' -i update
yq eval-all --inplace 'select(fileIndex == 0) * select(fileIndex == 1)' helm/values.yaml update
rm update
SELF_HOSTED_NOTARY_PUBIC_ROOT_KEY=$(python3 /scripts/get_root_key.py https://notary:4443 docker.io/securesystemsengineering/testimage:self-hosted-notary-signed)
yq -i '(.application.validators.[] | select(.name == "self-hosted-notary") | .trustRoots.[] | select(.name=="default") | .key) |= ${SELF_HOSTED_NOTARY_PUBIC_ROOT_KEY}' helm/values.yaml
}

update_helm_for_workloads() {
Expand Down
16 changes: 16 additions & 0 deletions tests/integration/notary_addhash.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
#!/usr/bin/expect

spawn notary addhash docker.io/securesystemsengineering/testimage self-hosted-notary-signed 528 --sha256 ${DIGEST_WITHOUT_PREFIX} --publish -c ./tests/data/notary_service_container/config/client_config.json -s https://notary:4443
expect "Enter passphrase for new root key with ID*\r"
send -- "0123456789\r"
expect "Repeat passphrase for new root key with ID*\r"
send -- "0123456789\r"
expect "Enter passphrase for new targets key with ID*\r"
send -- "0123456789\r"
expect "Repeat passphrase for new targets key with ID*\r"
send -- "0123456789\r"
expect "Enter passphrase for new snapshot key with ID*\r"
send -- "0123456789\r"
expect "Repeat passphrase for new snapshot key with ID*\r"
send -- "0123456789\r"
expect eof
8 changes: 8 additions & 0 deletions tests/integration/notary_init.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
#!/usr/bin/expect

spawn notary init docker.io/securesystemsengineering/testimage --publish -c ./tests/data/notary_service_container/config/client_config.json -s https://notary:4443
expect "Enter passphrase for targets key with ID*\r"
send -- "0123456789\r"
expect "Enter passphrase for snapshot key with ID*\r"
send -- "0123456789\r"
expect eof
Loading
Loading