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

coreos-ostree-importer: switch to plain http(s) url #66

Merged
merged 3 commits into from
Dec 20, 2019
Merged

coreos-ostree-importer: switch to plain http(s) url #66

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
183a71c
coreos-ostree-importer: move to Fedora 31
dustymabe Dec 17, 2019
eaf4753
coreos-ostree-importer: switch to plain http(s) url
dustymabe Dec 17, 2019
821889e
coreos-ostree-importer: update EXAMPLE fedora message
dustymabe Dec 18, 2019
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
10 changes: 2 additions & 8 deletions coreos-ostree-importer/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM registry.fedoraproject.org/fedora:30
FROM registry.fedoraproject.org/fedora:31

# set PYTHONUNBUFFERED env var to non-empty string so that our
# periods with no newline get printed immediately to the screen
Expand All @@ -8,7 +8,7 @@ ENV PYTHONUNBUFFERED=true
RUN dnf update -y && dnf clean all

# Install boto/fedmsg/ostree libraries
RUN dnf -y install python3-boto3 fedora-messaging ostree && dnf clean all
RUN dnf -y install fedora-messaging ostree && dnf clean all

# Put the file into a location that can be imported
ADD coreos_ostree_importer.py /usr/lib/python3.7/site-packages/
Expand All @@ -17,12 +17,6 @@ ADD coreos_ostree_importer.py /usr/lib/python3.7/site-packages/
# default location
ADD fedora-messaging-config.toml /etc/fedora-messaging/config.toml

# Environment variable to be defined by the user that defines the
# location of the AWS credentials file and also the path to the
# filesystem path to the keytab file. If blank it will be ignored
# and privileged (write) operations won't be attempted
ENV AWS_CONFIG_FILE ''

# Call fedora-messaging CLI and tell it to use the Consumer
# class from the included module.
CMD fedora-messaging consume --callback=coreos_ostree_importer:Consumer
14 changes: 1 addition & 13 deletions coreos-ostree-importer/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,6 @@ From your local git directory:
podman build -t coreos-ostree-importer .
```

Create a file with aws credentials somewhere:

```
cat <<'EOF' > /dev/shm/secret
[default]
aws_access_key_id=keyid
aws_secret_access_key=key
EOF
```

Create some empty OSTree repos:

```
Expand All @@ -43,8 +33,6 @@ Run the importer:
```
podman run -it --rm \
-v $PWD/:/pwd/ \
-v /dev/shm/secret:/.aws/config \
-e AWS_CONFIG_FILE=/.aws/config \
-v /srv/composerepo/:/mnt/koji/compose/ostree/repo/:z \
-v /srv/prodrepo/:/mnt/koji/ostree/repo/:z \
coreos-ostree-importer
Expand Down Expand Up @@ -98,7 +86,7 @@ body = {
"build_id": "30.20190905.0",
"stream": "testing",
"basearch": "x86_64",
"commit": "s3://fcos-builds/prod/streams/testing/builds/30.20190905.0/x86_64/ostree-commit.tar",
"commit": "https://fcos-builds/prod/streams/testing/builds/30.20190905.0/x86_64/ostree-commit.tar",
"checksum": "sha256:d01db6939e7387afa2492ac8e2591c53697fc21cf16785585f7f1ac0de692863",
"ostree_ref": "fedora/x86_64/coreos/testing",
"ostree_checksum": "b4beca154dab3696fd04f32ddab818102caa9247ec3192403adb9aaecc991bd9",
Expand Down
27 changes: 8 additions & 19 deletions coreos-ostree-importer/coreos_ostree_importer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
#!/usr/bin/python3

import boto3
import botocore
import fedora_messaging
import fedora_messaging.api
import hashlib
Expand All @@ -13,6 +11,7 @@
import tarfile
import tempfile
import traceback
import urllib.request

# Set local logging
logger = logging.getLogger(__name__)
Expand All @@ -33,7 +32,7 @@
"build_id": "30.20190905.0",
"stream": "testing",
"basearch": "x86_64",
"commit": "s3://fcos-builds/prod/streams/testing/builds/30.20190905.0/x86_64/ostree-commit.tar",
"commit": "https://fcos-builds/prod/streams/testing/builds/30.20190905.0/x86_64/ostree-commit.tar",
"checksum": "sha256:d01db6939e7387afa2492ac8e2591c53697fc21cf16785585f7f1ac0de692863",
"ostree_ref": "fedora/x86_64/coreos/testing",
"ostree_checksum": "b4beca154dab3696fd04f32ddab818102caa9247ec3192403adb9aaecc991bd9",
Expand Down Expand Up @@ -127,8 +126,8 @@ def process(self, message: fedora_messaging.api.Message):
logger.info("Commit exists in compose repo. Importing from there")
source_repo_path = KNOWN_OSTREE_REPOS["compose"]
else:
# Grab the file from s3 and then pull local
untar_file_from_s3(url=commit_url, tmpdir=tmpdir, sha256sum=sha256sum)
# Grab the file from a web url and then pull local
untar_file_from_url(url=commit_url, tmpdir=tmpdir, sha256sum=sha256sum)
source_repo_path = tmpdir

# one more sanity check: make sure buildid == version
Expand Down Expand Up @@ -184,23 +183,13 @@ def get_sha256sum(filepath: str) -> str:
return h.hexdigest()


def parse_s3_url(url: str) -> tuple:
if not url.startswith("s3://"):
raise Exception(f"Unable to parse the s3 url: {url}")
# Chop off s3:// and break into bucket / key
bucket, key = url[5:].split("/", 1)
return (bucket, key)


def untar_file_from_s3(url: str, tmpdir: str, sha256sum: str):
def untar_file_from_url(url: str, tmpdir: str, sha256sum: str):
filename = "ostree.tar"
filepath = os.path.join(tmpdir, filename)

# Grab file from s3
logger.info(f"Downloading object from s3: {url}")
s3 = boto3.client("s3")
bucket, key = parse_s3_url(url)
s3.download_file(bucket, key, filepath)
# Grab file from the url
logger.info(f"Downloading object from url: {url}")
urllib.request.urlretrieve(url, filepath)
Copy link
Member

Choose a reason for hiding this comment

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

Just a side note, but we should also download the signature and verify it before importing. E.g. we can add a commit-sig to the message?

Copy link
Member Author

Choose a reason for hiding this comment

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

We already check the sha256sum. The only thing I can think that the signature file would help us with is to make sure only signed content got into the repo (i.e. if we're worried about an attacker trying to send a message to import invalid content), but I can think of even better ways to do that too. One of them would be to verify the commit in the ostree repo after the file was untarred, but before it was imported.

Copy link
Member

Choose a reason for hiding this comment

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

One of them would be to verify the commit in the ostree repo after the file was untarred, but before it was imported.

Hmm, I thought that was already the case. We definitely should do this too if not. Ahh OK yup, we need to explicitly do --gpgverify.

We already check the sha256sum. The only thing I can think that the signature file would help us with is to make sure only signed content got into the repo (i.e. if we're worried about an attacker trying to send a message to import invalid content)

One argument for requiring a signature on the tarball itself is to sanity check that it's not malicious before trying to extract it. Actually, even the docs for TarFile.extractall() has a warning related to this.

So I think we should do both.

Copy link
Member Author

Choose a reason for hiding this comment

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

so:

  • remove checksum and apply signature checking for the tarball
  • add commit checking in extracted ostree repo

?

Copy link
Member Author

Choose a reason for hiding this comment

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

pushing that bit to a followup: #68


# Verify file has correct checksum
calcuatedsum = get_sha256sum(filepath)
Expand Down