Skip to content

Commit 065aad3

Browse files
jujubotjujubot
authoredJul 17, 2024
Merge pull request juju#17749 from tlm/juju-6342-authorized-keys-tests
juju#17749 This PR adds BASH tests for asserting authorised key functionality. We are doing this so as we move the CRUD and authorised keys functions over to DQlite in 4.0 we can have some certainty around how things operate and that the core contracts have been maintained. This PR is required by juju#17602 I also made a quick change to the authorized key worker so that logging on key changes is always at info now and not debug. @hpidcock and myself believed that this should always be info for security auditing purposes. ## Checklist - [x] Code style: imports ordered, good names, simple structure, etc - [x] Comments saying why design decisions were made - ~[ ] Go unit tests, with comments saying what you're testing~ - [x] [Integration tests](https://github.com/juju/juju/tree/main/tests), with comments saying what you're testing - ~[ ] [doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~ ## QA steps ```bash cd tests ./main.sh authorizedkeys ``` ## Documentation changes N/A ## Links **Jira card:** JUJU-6342
2 parents f62b8bb + e76419d commit 065aad3

File tree

5 files changed

+103
-2
lines changed

5 files changed

+103
-2
lines changed
 

‎tests/main.sh

+1
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import_subdir_files includes
4141
# Please keep these in alphabetic order.
4242
TEST_NAMES="agents \
4343
appdata \
44+
authorized_keys \
4445
backup \
4546
bootstrap \
4647
branches \

‎tests/suites/authorized_keys/machine.sh

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
run_machine_ssh() {
2+
# Echo out to ensure nice output to the test suite.
3+
echo
4+
5+
# Add a new machine for the test to assert sshing into.
6+
juju add-machine -n 1
7+
wait_for_machine_agent_status "0" "started"
8+
9+
# Generate a new ssh key pair for this test.
10+
ssh_key_file="${TEST_DIR}/machine-ssh-key"
11+
ssh_key_file_pub="${ssh_key_file}.pub"
12+
ssh-keygen -t ed25519 -f "$ssh_key_file" -C "isgreat@juju.is" -P ""
13+
14+
# Add the SSH key and see that it comes back out in the list of user keys.
15+
juju add-ssh-key "$(cat $ssh_key_file_pub)"
16+
17+
# Watch the debug log for the agent to assert that a key has been added to
18+
# the machine.
19+
timeout 5m juju debug-log --tail | grep -m 1 'adding ssh keys to authorized keys' || true
20+
21+
# Check that the test can ssh to the machine with the new keypair and run a
22+
# command.
23+
check_contains "$(juju ssh 0 -i \"${ssh_key_file}\" echo foobar)" "foobar"
24+
}
25+
26+
# test_machine_ssh is responsible for testing that adding authorized keys to a
27+
# model traverse through the controller and down to the machine agents. After
28+
# this has happened ssh access to a machine should be granted for the owner of
29+
# the newly added public key.
30+
test_machine_ssh() {
31+
if [ "$(skip 'test_machine_ssh')" ]; then
32+
echo "==> TEST SKIPPED: authorized keys machine ssh"
33+
return
34+
fi
35+
36+
(
37+
set_verbosity
38+
39+
run "run_machine_ssh"
40+
)
41+
}

‎tests/suites/authorized_keys/task.sh

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
test_authorized_keys() {
2+
if [ "$(skip 'test_authorized_keys')" ]; then
3+
echo "==> TEST SKIPPED: authorized_keys"
4+
return
5+
fi
6+
7+
set_verbosity
8+
9+
echo "==> Checking for dependencies"
10+
check_dependencies juju
11+
12+
log_file="${TEST_DIR}/authorized_keys.log"
13+
14+
ensure "authorizedkeys" "$log_file"
15+
16+
test_user_ssh_keys
17+
test_machine_ssh
18+
19+
destroy_controller "authorizedkeys"
20+
}

‎tests/suites/authorized_keys/user.sh

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
run_user_ssh_keys() {
2+
# Echo out to ensure nice output to the test suite.
3+
echo
4+
5+
ssh_key_file="${TEST_DIR}/juju-ssh-key"
6+
ssh_key_file_pub="${ssh_key_file}.pub"
7+
8+
ssh-keygen -t ed25519 -f "$ssh_key_file" -C "isgreat@juju.is" -P ""
9+
fingerprint=$(ssh-keygen -E md5 -lf "${ssh_key_file_pub}" | cut -f 2 -d ' ' | cut -f 2- -d ':')
10+
11+
# Add the SSH key and see that it comes back out in the list of user keys.
12+
juju add-ssh-key "$(cat $ssh_key_file_pub)"
13+
check_contains "$(juju ssh-keys)" "$fingerprint"
14+
15+
# Remove the SSH key by fingerprint
16+
juju remove-ssh-key "${fingerprint}"
17+
check_not_contains "$(juju ssh-keys)" "${fingerprint}"
18+
19+
# Add the SSH key and see that it comes back out in the list of user keys.
20+
juju add-ssh-key "$(cat $ssh_key_file_pub)"
21+
check_contains "$(juju ssh-keys)" "$fingerprint"
22+
23+
# Remove the SSH key by comment
24+
juju remove-ssh-key isgreat@juju.is
25+
check_not_contains "$(juju ssh-keys)" "${fingerprint}"
26+
}
27+
28+
test_user_ssh_keys() {
29+
if [ "$(skip 'test_user_ssh_keys')" ]; then
30+
echo "==> TEST SKIPPED: authorized keys user ssh keys"
31+
return
32+
fi
33+
34+
(
35+
set_verbosity
36+
37+
run "run_user_ssh_keys"
38+
)
39+
}

‎worker/authenticationworker/worker.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -123,8 +123,8 @@ func (kw *keyupdaterWorker) Handle(_ <-chan struct{}) error {
123123
deleted := kw.jujuKeys.Difference(newJujuKeys)
124124
added := newJujuKeys.Difference(kw.jujuKeys)
125125
if added.Size() > 0 || deleted.Size() > 0 {
126-
logger.Debugf("adding ssh keys to authorised keys: %v", added)
127-
logger.Debugf("deleting ssh keys from authorised keys: %v", deleted)
126+
logger.Infof("adding ssh keys to authorised keys: %v", added)
127+
logger.Infof("deleting ssh keys from authorised keys: %v", deleted)
128128
if err = kw.writeSSHKeys(newKeys); err != nil {
129129
err = errors.Annotate(err, "updating ssh keys")
130130
logger.Infof(err.Error())

0 commit comments

Comments
 (0)
Failed to load comments.
Merge pull request #17749 from tlm/juju-6342-authorized-keys-tests &middot; JoseFMP/juju@065aad3 · GitHub