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

Development: Add support for ephemeral SSH keys for the authentication of build agents #8951

Merged
merged 28 commits into from
Aug 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
359fe80
Add initial implementation
bensofficial Jun 10, 2024
d6bd6c9
fix variable usage
Jun 13, 2024
9b7c357
Improve SSH key generation
bensofficial Jun 20, 2024
d2c41d2
Merge branch 'develop' of github.com:ls1intum/Artemis into feature/lo…
bensofficial Jun 20, 2024
3262028
Update SSH key generation and file permissions
bensofficial Jun 20, 2024
82327a1
Fix key generation and use public key in Hazelcast
bensofficial Jun 24, 2024
2a85ab4
Merge branch 'develop' of github.com:ls1intum/Artemis into feature/lo…
bensofficial Jun 24, 2024
5d10ce9
Refactor code
bensofficial Jun 25, 2024
74fa5de
Merge branch 'develop' of github.com:ls1intum/Artemis into feature/lo…
bensofficial Jun 27, 2024
049413e
Add tests
bensofficial Jul 1, 2024
2da993b
Merge branch 'develop' of github.com:ls1intum/Artemis into feature/lo…
bensofficial Jul 1, 2024
786d8a2
Add ssh template clone url
bensofficial Jul 1, 2024
2a37c45
Fix test
bensofficial Jul 1, 2024
ff6f8df
Merge branch 'develop' into feature/localci/build-agent-ssh
bensofficial Jul 2, 2024
ce3e830
Add Javadoc
bensofficial Jul 2, 2024
866ab2a
Merge branch 'feature/localci/build-agent-ssh' of github.com:ls1intum…
bensofficial Jul 2, 2024
25a9c9a
Replace orElseThrow with get
bensofficial Jul 3, 2024
1f5789b
Merge branch 'develop' of github.com:ls1intum/Artemis into feature/lo…
bensofficial Jul 3, 2024
827e443
Merge branch 'develop' of github.com:ls1intum/Artemis into feature/lo…
bensofficial Jul 6, 2024
0efc79a
Apply suggestions from code review
bensofficial Jul 6, 2024
25f11f9
Rename BuildAgentSSHKeyService.java to BuildAgentSshKeyService.java
bensofficial Jul 6, 2024
70271db
Rename BuildAgentSSHAuthenticationIntegrationTest.java to BuildAgentS…
bensofficial Jul 6, 2024
baa2d2b
Replace lambda with method call
bensofficial Jul 6, 2024
8eaa78c
Merge branch 'develop' into feature/localci/build-agent-ssh
bensofficial Jul 9, 2024
5f01e91
Merge branch 'develop' into feature/localci/build-agent-ssh
bensofficial Jul 15, 2024
ed9ef0c
Merge branch 'develop' into feature/localci/build-agent-ssh
krusche Jul 21, 2024
4f95170
Merge branch 'develop' into feature/localci/build-agent-ssh
krusche Jul 28, 2024
ba9b9a2
Merge branch 'develop' into feature/localci/build-agent-ssh
maximiliansoelch Aug 2, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,7 @@
@Profile(PROFILE_LOCALVC)
public class SshConstants {

public static final AttributeRepository.AttributeKey<Boolean> IS_BUILD_AGENT_KEY = new AttributeRepository.AttributeKey<>();

public static final AttributeRepository.AttributeKey<User> USER_KEY = new AttributeRepository.AttributeKey<>();
}
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ public List<BuildAgentInformation> getBuildAgentInformation() {

public List<BuildAgentInformation> getBuildAgentInformationWithoutRecentBuildJobs() {
return buildAgentInformation.values().stream().map(agent -> new BuildAgentInformation(agent.name(), agent.maxNumberOfConcurrentBuildJobs(),
agent.numberOfCurrentBuildJobs(), agent.runningBuildJobs(), agent.status(), null)).toList();
agent.numberOfCurrentBuildJobs(), agent.runningBuildJobs(), agent.status(), null, null)).toList();
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
package de.tum.in.www1.artemis.service.connectors.localci.buildagent;

import static de.tum.in.www1.artemis.config.Constants.PROFILE_BUILDAGENT;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.PosixFilePermissions;
import java.security.GeneralSecurityException;
import java.security.KeyPair;
import java.security.KeyPairGenerator;
import java.security.NoSuchAlgorithmException;
import java.util.Optional;

import org.apache.sshd.common.config.keys.writer.openssh.OpenSSHKeyEncryptionContext;
import org.apache.sshd.common.config.keys.writer.openssh.OpenSSHKeyPairResourceWriter;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.boot.context.event.ApplicationReadyEvent;
import org.springframework.context.annotation.Profile;
import org.springframework.context.event.EventListener;
import org.springframework.stereotype.Service;

@Service
@Profile(PROFILE_BUILDAGENT)
public class BuildAgentSshKeyService {

private static final Logger log = LoggerFactory.getLogger(BuildAgentSshKeyService.class);

private KeyPair keyPair;

@Value("${artemis.version-control.ssh-private-key-folder-path:#{null}}")
private Optional<String> gitSshPrivateKeyPath;

@Value("${artemis.version-control.build-agent-use-ssh:false}")
private boolean useSshForBuildAgent;

@Value("${info.contact}")
private String sshKeyComment;

/**
* Generates the SSH key pair and writes the private key when the application is started and the build agents should use SSH for their git operations.
*/
@EventListener(ApplicationReadyEvent.class)
public void applicationReady() {
if (!useSshForBuildAgent) {
return;
}

log.info("Using SSH for build agent authentication.");

if (gitSshPrivateKeyPath.isEmpty()) {
throw new RuntimeException("No SSH private key folder was set but should use SSH for build agent authentication.");
}

try {
generateKeyPair();
writePrivateKey();
}
catch (IOException | GeneralSecurityException e) {
throw new RuntimeException(e);
}
}
bensofficial marked this conversation as resolved.
Show resolved Hide resolved

private void generateKeyPair() throws NoSuchAlgorithmException {
KeyPairGenerator keyGen = KeyPairGenerator.getInstance("RSA");
keyGen.initialize(4096);
keyPair = keyGen.generateKeyPair();
}

private void writePrivateKey() throws IOException, GeneralSecurityException {
Path privateKeyPath = Path.of(gitSshPrivateKeyPath.orElseThrow(), "id_rsa");
OpenSSHKeyPairResourceWriter writer = new OpenSSHKeyPairResourceWriter();

try (OutputStream outputStream = Files.newOutputStream(privateKeyPath)) {
writer.writePrivateKey(keyPair, sshKeyComment, new OpenSSHKeyEncryptionContext(), outputStream);
}

Files.setPosixFilePermissions(privateKeyPath, PosixFilePermissions.fromString("rw-------"));
}

/**
* Returns the formated SSH public key.
* If SSH is not used for the build agent, it returns {@code null}.
*
* @return the public key
*/
public String getPublicKeyAsString() {
if (!useSshForBuildAgent) {
return null;
}

OpenSSHKeyPairResourceWriter writer = new OpenSSHKeyPairResourceWriter();
try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) {
writer.writePublicKey(keyPair, sshKeyComment, outputStream);
return outputStream.toString();
}
catch (IOException | GeneralSecurityException e) {
throw new RuntimeException(e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ public class SharedQueueProcessingService {

private final AtomicInteger localProcessingJobs = new AtomicInteger(0);

private final BuildAgentSshKeyService buildAgentSSHKeyService;

/**
* Lock to prevent multiple nodes from processing the same build job.
*/
Expand All @@ -87,11 +89,12 @@ public class SharedQueueProcessingService {
private UUID listenerId;

public SharedQueueProcessingService(@Qualifier("hazelcastInstance") HazelcastInstance hazelcastInstance, ExecutorService localCIBuildExecutorService,
BuildJobManagementService buildJobManagementService, BuildLogsMap buildLogsMap) {
BuildJobManagementService buildJobManagementService, BuildLogsMap buildLogsMap, BuildAgentSshKeyService buildAgentSSHKeyService) {
this.hazelcastInstance = hazelcastInstance;
this.localCIBuildExecutorService = (ThreadPoolExecutor) localCIBuildExecutorService;
this.buildJobManagementService = buildJobManagementService;
this.buildLogsMap = buildLogsMap;
this.buildAgentSSHKeyService = buildAgentSSHKeyService;
}

/**
Expand All @@ -104,7 +107,7 @@ public void init() {
this.sharedLock = this.hazelcastInstance.getCPSubsystem().getLock("buildJobQueueLock");
this.queue = this.hazelcastInstance.getQueue("buildJobQueue");
this.resultQueue = this.hazelcastInstance.getQueue("buildResultQueue");
this.listenerId = this.queue.addItemListener(new SharedQueueProcessingService.QueuedBuildJobItemListener(), true);
this.listenerId = this.queue.addItemListener(new QueuedBuildJobItemListener(), true);
}

@PreDestroy
Expand Down Expand Up @@ -268,7 +271,9 @@ private BuildAgentInformation getUpdatedLocalBuildAgentInformation(BuildJobQueue
recentBuildJobs.add(recentBuildJob);
}

return new BuildAgentInformation(memberAddress, maxNumberOfConcurrentBuilds, numberOfCurrentBuildJobs, processingJobsOfMember, active, recentBuildJobs);
String publicSshKey = buildAgentSSHKeyService.getPublicKeyAsString();

return new BuildAgentInformation(memberAddress, maxNumberOfConcurrentBuilds, numberOfCurrentBuildJobs, processingJobsOfMember, active, recentBuildJobs, publicSshKey);
}

private List<BuildJobQueueItem> getProcessingJobsOfNode(String memberAddress) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
@JsonIgnoreProperties(ignoreUnknown = true)
@JsonInclude(JsonInclude.Include.NON_EMPTY)
public record BuildAgentInformation(String name, int maxNumberOfConcurrentBuildJobs, int numberOfCurrentBuildJobs, List<BuildJobQueueItem> runningBuildJobs, boolean status,
List<BuildJobQueueItem> recentBuildJobs) implements Serializable {
List<BuildJobQueueItem> recentBuildJobs, String publicSshKey) implements Serializable {

@Serial
private static final long serialVersionUID = 1L;
Expand All @@ -23,6 +23,6 @@ public record BuildAgentInformation(String name, int maxNumberOfConcurrentBuildJ
*/
public BuildAgentInformation(BuildAgentInformation agentInformation, List<BuildJobQueueItem> recentBuildJobs) {
this(agentInformation.name(), agentInformation.maxNumberOfConcurrentBuildJobs(), agentInformation.numberOfCurrentBuildJobs(), agentInformation.runningBuildJobs,
agentInformation.status(), recentBuildJobs);
agentInformation.status(), recentBuildJobs, agentInformation.publicSshKey());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@

import static de.tum.in.www1.artemis.config.Constants.PROFILE_LOCALVC;

import java.io.IOException;
import java.security.GeneralSecurityException;
import java.security.PublicKey;
import java.util.Objects;
import java.util.Optional;

import org.apache.sshd.common.config.keys.AuthorizedKeyEntry;
import org.apache.sshd.server.auth.pubkey.PublickeyAuthenticator;
Expand All @@ -16,6 +19,8 @@
import de.tum.in.www1.artemis.config.icl.ssh.HashUtils;
import de.tum.in.www1.artemis.config.icl.ssh.SshConstants;
import de.tum.in.www1.artemis.repository.UserRepository;
import de.tum.in.www1.artemis.service.connectors.localci.SharedQueueManagementService;
import de.tum.in.www1.artemis.service.connectors.localci.dto.BuildAgentInformation;

@Profile(PROFILE_LOCALVC)
@Service
Expand All @@ -25,8 +30,11 @@ public class GitPublickeyAuthenticatorService implements PublickeyAuthenticator

private final UserRepository userRepository;

public GitPublickeyAuthenticatorService(UserRepository userRepository) {
private final Optional<SharedQueueManagementService> localCIBuildJobQueueService;

public GitPublickeyAuthenticatorService(UserRepository userRepository, Optional<SharedQueueManagementService> localCIBuildJobQueueService) {
this.userRepository = userRepository;
this.localCIBuildJobQueueService = localCIBuildJobQueueService;
}

@Override
Expand All @@ -46,6 +54,7 @@ public boolean authenticate(String username, PublicKey publicKey, ServerSession
if (Objects.equals(storedPublicKey, publicKey)) {
log.debug("Found user {} for public key authentication", user.get().getLogin());
session.setAttribute(SshConstants.USER_KEY, user.get());
session.setAttribute(SshConstants.IS_BUILD_AGENT_KEY, false);
return true;
}
else {
Expand All @@ -56,6 +65,29 @@ public boolean authenticate(String username, PublicKey publicKey, ServerSession
log.error("Failed to convert stored public key string to PublicKey object", e);
}
}
else if (localCIBuildJobQueueService.isPresent()
&& localCIBuildJobQueueService.get().getBuildAgentInformation().stream().anyMatch(agent -> checkPublicKeyMatchesBuildAgentPublicKey(agent, publicKey))) {
log.info("Authenticating as build agent");
session.setAttribute(SshConstants.IS_BUILD_AGENT_KEY, true);
return true;
}
return false;
}

private boolean checkPublicKeyMatchesBuildAgentPublicKey(BuildAgentInformation agent, PublicKey publicKey) {
if (agent.publicSshKey() == null) {
return false;
}

AuthorizedKeyEntry agentKeyEntry = AuthorizedKeyEntry.parseAuthorizedKeyEntry(agent.publicSshKey());
PublicKey agentPublicKey;
try {
agentPublicKey = agentKeyEntry.resolvePublicKey(null, null, null);
}
catch (IOException | GeneralSecurityException e) {
return false;
}

return agentPublicKey.equals(publicKey);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -71,13 +71,19 @@ public Path resolveRootDirectory(String command, String[] args, ServerSession se

// git-upload-pack means fetch (read operation), git-receive-pack means push (write operation)
final var repositoryAction = gitCommand.equals("git-upload-pack") ? RepositoryActionType.READ : gitCommand.equals("git-receive-pack") ? RepositoryActionType.WRITE : null;
final var user = session.getAttribute(SshConstants.USER_KEY);
try {
localVCServletService.authorizeUser(repositoryTypeOrUserName, user, exercise, repositoryAction, localVCRepositoryUri.isPracticeRepository());

if (session.getAttribute(SshConstants.IS_BUILD_AGENT_KEY) && repositoryAction == RepositoryActionType.READ) {
// We already checked for build agent authenticity
}
bensofficial marked this conversation as resolved.
Show resolved Hide resolved
catch (LocalVCForbiddenException e) {
log.error("User {} does not have access to the repository {}", user.getLogin(), repositoryPath);
throw new AccessDeniedException("User does not have access to this repository", e);
else {
final var user = session.getAttribute(SshConstants.USER_KEY);
try {
localVCServletService.authorizeUser(repositoryTypeOrUserName, user, exercise, repositoryAction, localVCRepositoryUri.isPracticeRepository());
}
catch (LocalVCForbiddenException e) {
log.error("User {} does not have access to the repository {}", user.getLogin(), repositoryPath);
throw new AccessDeniedException("User does not have access to this repository", e);
}
}

// we cannot trust unvalidated user input
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@
"artemis.continuous-integration.specify-concurrent-builds=true", "artemis.continuous-integration.concurrent-build-size=1",
"artemis.continuous-integration.asynchronous=false", "artemis.continuous-integration.build.images.java.default=dummy-docker-image",
"artemis.continuous-integration.image-cleanup.enabled=true", "artemis.continuous-integration.image-cleanup.disk-space-threshold-mb=1000000000",
"spring.liquibase.enabled=true", "artemis.iris.health-ttl=500" })
"spring.liquibase.enabled=true", "artemis.iris.health-ttl=500", "artemis.version-control.ssh-private-key-folder-path=${java.io.tmpdir}",
"artemis.version-control.build-agent-use-ssh=true", "info.contact=test@localhost", "artemis.version-control.ssh-template-clone-url=ssh://git@localhost:7921/" })
@ContextConfiguration(classes = TestBuildAgentConfiguration.class)
public abstract class AbstractSpringIntegrationLocalCILocalVCTest extends AbstractArtemisIntegrationTest {

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package de.tum.in.www1.artemis.localvcci;

import static org.assertj.core.api.Assertions.assertThat;

import java.nio.file.Files;
import java.nio.file.Path;

import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.beans.factory.annotation.Value;

import com.hazelcast.core.HazelcastInstance;
import com.hazelcast.map.IMap;

import de.tum.in.www1.artemis.AbstractSpringIntegrationLocalCILocalVCTest;
import de.tum.in.www1.artemis.service.connectors.localci.buildagent.BuildAgentSshKeyService;
import de.tum.in.www1.artemis.service.connectors.localci.buildagent.SharedQueueProcessingService;
import de.tum.in.www1.artemis.service.connectors.localci.dto.BuildAgentInformation;

class BuildAgentSshAuthenticationIntegrationTest extends AbstractSpringIntegrationLocalCILocalVCTest {

@Autowired
@Qualifier("hazelcastInstance")
private HazelcastInstance hazelcastInstance;

@Autowired
private BuildAgentSshKeyService buildAgentSSHKeyService;

@Autowired
private SharedQueueProcessingService sharedQueueProcessingService;

@Value("${artemis.version-control.ssh-private-key-folder-path}")
protected String gitSshPrivateKeyPath;

@Test
void testWriteSSHKey() {
boolean sshPrivateKeyExists = Files.exists(Path.of(System.getProperty("java.io.tmpdir"), "id_rsa"));
assertThat(sshPrivateKeyExists).as("SSH private key written to tmp dir.").isTrue();
}

@Test
void testSSHInHazelcast() {
sharedQueueProcessingService.updateBuildAgentInformation();
IMap<String, BuildAgentInformation> buildAgentInformation = hazelcastInstance.getMap("buildAgentInformation");
assertThat(buildAgentInformation.values()).as("SSH public key available in hazelcast.")
.anyMatch(agent -> agent.publicSshKey().equals(buildAgentSSHKeyService.getPublicKeyAsString()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ void createJobs() {
job1 = new BuildJobQueueItem("1", "job1", "address1", 1, course.getId(), 1, 1, 1, BuildStatus.SUCCESSFUL, repositoryInfo, jobTimingInfo1, buildConfig, null);
job2 = new BuildJobQueueItem("2", "job2", "address1", 2, course.getId(), 1, 1, 1, BuildStatus.SUCCESSFUL, repositoryInfo, jobTimingInfo2, buildConfig, null);
String memberAddress = hazelcastInstance.getCluster().getLocalMember().getAddress().toString();
agent1 = new BuildAgentInformation(memberAddress, 1, 0, new ArrayList<>(List.of(job1)), false, new ArrayList<>(List.of(job2)));
agent1 = new BuildAgentInformation(memberAddress, 1, 0, new ArrayList<>(List.of(job1)), false, new ArrayList<>(List.of(job2)), null);
BuildJobQueueItem finishedJobQueueItem1 = new BuildJobQueueItem("3", "job3", "address1", 3, course.getId(), 1, 1, 1, BuildStatus.SUCCESSFUL, repositoryInfo, jobTimingInfo1,
buildConfig, null);
BuildJobQueueItem finishedJobQueueItem2 = new BuildJobQueueItem("4", "job4", "address1", 4, course.getId() + 1, 1, 1, 1, BuildStatus.FAILED, repositoryInfo, jobTimingInfo2,
Expand Down
Loading