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

Add staging-with-rebase-focal to CI and fix testinfra tests #5638

Merged
merged 12 commits into from
Dec 7, 2020
38 changes: 38 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,37 @@ jobs:
- store_artifacts:
path: ~/sd/junit

staging-test-with-rebase-focal:
machine:
enabled: true

working_directory: ~/sd
steps:
- checkout
- *rebaseontarget
- *installenchant

- run:
name: Run Staging tests on GCE
command: |
BRANCH_MATCH=$(devops/scripts/match-ci-branch.sh "^(i18n)")
if [[ $BRANCH_MATCH =~ ^found ]]; then echo "Skipping: ${BRANCH_MATCH}"; exit 0; fi
BASE_OS=focal make ci-go
no_output_timeout: 35m

- run:
name: Ensure environment torn down
# Always report true, since env should will destroyed already
# if all tests passed.
command: make ci-teardown || true
when: always

- store_test_results:
path: ~/sd/junit

- store_artifacts:
path: ~/sd/junit

deb-tests:
docker:
- image: gcr.io/cloud-builders/docker
Expand Down Expand Up @@ -415,6 +446,13 @@ workflows:
- /i18n-.*/
requires:
- lint
- staging-test-with-rebase-focal:
filters:
branches:
ignore:
- /i18n-.*/
requires:
- lint
- translation-tests:
requires:
- lint
Expand Down
1 change: 1 addition & 0 deletions devops/gce-nested/ci-go.sh
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ set -e
set -u
set -o pipefail

export BASE_OS="${BASE_OS:-xenial}"

./devops/gce-nested/gce-start.sh
./devops/gce-nested/gce-runner.sh
Expand Down
11 changes: 9 additions & 2 deletions devops/gce-nested/gce-runner.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
# for storage as artifacts on the build, so devs can review via web.
set -e
set -u
BASE_OS="${BASE_OS:-xenial}"


TOPLEVEL="$(git rev-parse --show-toplevel)"
Expand Down Expand Up @@ -51,9 +52,15 @@ function copy_securedrop_repo() {

# Main logic
copy_securedrop_repo
ssh_gce "make build-debs-notest"

# The test results should be collected regardless of pass/fail,
# so register a trap to ensure the fetch always runs.
trap fetch_junit_test_results EXIT
ssh_gce "make staging"
if [ "${BASE_OS:-'xenial'}" = "xenial" ]
then
ssh_gce "make build-debs-notest"
ssh_gce "make staging"
else
ssh_gce "make build-debs-notest-focal"
ssh_gce "make staging-focal"
fi
19 changes: 19 additions & 0 deletions install_files/ansible-base/roles/common/tasks/harden_dns.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,22 @@
tags:
- dns
- hardening

- name: Disable systemd-resolved on Focal
systemd:
name: systemd-resolved
state: stopped
enabled: no
when: ansible_distribution_release == 'focal'
tags:
- dns
- hardening

- name: Create resolv.conf for Focal
template:
src: dns_base
dest: /etc/resolv.conf
when: ansible_distribution_release == 'focal'
tags:
- dns
- hardening
Copy link
Contributor

Choose a reason for hiding this comment

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

No objections to disabling systemd-resolved, sticking with resolvconf which we've been using for a while will be fairly straightforward. Since we just copied the same dns_base source a few lines above, let's use that task to write the file. Sounds like on Xenial we want the /etc/resolvconf/resolve.conf.d/ path, whereas on Focal we should write it directly to /etc/resolv.conf.

Copy link
Contributor

Choose a reason for hiding this comment

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

Added a commit to consolidate the logic here a bit: under Focal, the old /etc/resolvconf/resolve.conf.d/ path is no longer written to, and the tests now inspect the correct file based on distro.

10 changes: 10 additions & 0 deletions install_files/ansible-base/roles/ossec/files/ossec.service
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[Unit]
Description=OSSEC service

[Service]
Type=forking
ExecStart=/var/ossec/bin/ossec-control start
ExecStop=/var/ossec/bin/ossec-control stop

[Install]
WantedBy=multi-user.target
2 changes: 1 addition & 1 deletion install_files/ansible-base/roles/ossec/handlers/main.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
# Single handler to operate on *both* OSSEC hosts, server & client.
- name: restart ossec
service:
systemd:
name: ossec
state: restarted
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,20 @@
creates: /var/ossec/etc/sslmanager.cert
tags:
- ossec_auth

- name: Copy the systemd service file
copy:
src: ossec.service
dest: "/etc/systemd/system/ossec.service"

- name: Remove the old style /etc/init.d/ossec file
file:
path: "/etc/init.d/ossec"
Copy link
Contributor

Choose a reason for hiding this comment

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

These files are provided by ossec-{agent,server} packages:

.
├── etc
│   ├── init.d
│   │   └── ossec
│   └── ossec-init.conf

This means that on subsequent ossec package upgrades, this init file will be re-added, unless we do specific tasks at build-time to remove it. If these steps are strictly required for functioning, we should address this in the build logic as to not provide the init.d file in focal (instead of doing it in ansible)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This means that on subsequent ossec package upgrades, this init file will be re-added, unless we do specific tasks at build-time to remove it. If these steps are strictly required for functioning, we should address this in the build logic as to not provide the init.d file in focal (instead of doing it in ansible)

I think we should remove it from the server package.

Copy link
Contributor

Choose a reason for hiding this comment

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

Definitely agree, let's remove the Ansible task and clean up that file at the packaging level. Be mindful of side-effects for Xenial.

state: absent

- name: Enable the OSSEC service
systemd:
name: ossec
daemon_reload: yes
enabled: yes
masked: no
3 changes: 2 additions & 1 deletion molecule/testinfra/app/test_appenv.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ def test_gpg_key_in_keyring(host):
with host.sudo(sdvars.securedrop_user):
c = host.run("gpg --homedir /var/lib/securedrop/keys "
"--list-keys 28271441")
assert "pub 4096R/28271441 2013-10-12" in c.stdout
assert "2013-10-12" in c.stdout
assert "28271441" in c.stdout


def test_ensure_logo(host):
Expand Down
6 changes: 1 addition & 5 deletions molecule/testinfra/common/test_fpf_apt_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,7 @@ def test_fpf_apt_repo_fingerprint(host):

c = host.run('apt-key finger')

fpf_gpg_pub_key_info = """/etc/apt/trusted.gpg.d/securedrop-keyring.gpg
---------------------------------------------
pub 4096R/00F4AD77 2016-10-20 [expires: 2021-06-30]
Key fingerprint = 2224 5C81 E3BA EB41 38B3 6061 310F 5612 00F4 AD77
uid SecureDrop Release Signing Key"""
fpf_gpg_pub_key_info = "2224 5C81 E3BA EB41 38B3 6061 310F 5612 00F4 AD77"
Copy link
Contributor

Choose a reason for hiding this comment

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

Having the full output seems more secure, especially if the testinfra tests are being used to verify a prod system.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The output is different in both Xenial and Focal, that is why I took this path.


assert c.rc == 0
assert fpf_gpg_pub_key_info in c.stdout
Expand Down
15 changes: 2 additions & 13 deletions molecule/testinfra/mon/test_mon_network.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,5 @@ def test_listening_ports(host, ossec_service):
"""
socket = "{proto}://{host}:{port}".format(**ossec_service)
with host.sudo():
# Really hacky work-around for bug found in testinfra 1.12.0
# https://github.com/philpep/testinfra/issues/311
if "udp" in socket:
lsof_socket = "{proto}@{host}:{port}".format(**ossec_service)
udp_check = host.run("lsof -n -i"+lsof_socket)

if ossec_service['listening']:
assert udp_check.rc == 0
else:
assert udp_check.rc == 1
else:
assert (host.socket(socket).is_listening ==
ossec_service['listening'])
assert (host.socket(socket).is_listening ==
ossec_service['listening'])
2 changes: 1 addition & 1 deletion molecule/testinfra/vars/app-prod.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ app_directories:
- /var/lib/securedrop/tmp

apparmor_enforce:
- "/sbin/dhclient"
- "sbin/dhclient"
- "/usr/lib/NetworkManager/nm-dhcp-client.action"
- "/usr/lib/connman/scripts/dhclient-script"
- "/usr/sbin/ntpd"
Expand Down
2 changes: 1 addition & 1 deletion molecule/testinfra/vars/app-staging.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ pip_deps:
apparmor_complain: []

apparmor_enforce:
- "/sbin/dhclient"
- "sbin/dhclient"
- "/usr/lib/NetworkManager/nm-dhcp-client.action"
- "/usr/lib/connman/scripts/dhclient-script"
- "/usr/sbin/ntpd"
Expand Down
2 changes: 1 addition & 1 deletion molecule/testinfra/vars/staging.yml
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ pip_deps:
apparmor_complain: []

apparmor_enforce:
- "/sbin/dhclient"
- "sbin/dhclient"
- "/usr/lib/NetworkManager/nm-dhcp-client.action"
- "/usr/lib/connman/scripts/dhclient-script"
- "/usr/sbin/ntpd"
Expand Down