Skip to content
Merged
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
46 changes: 30 additions & 16 deletions integration-tests/ssh_cert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,20 +10,34 @@ import (
)

func TestSSHCerts(t *testing.T) {
authServer := mockapi.NewAuthServer(t)
defer authServer.Close()

myUserID := "my-user-id"

apiHandler := mockapi.NewHandler(t)
apiHandler.SetMyUser(&mockapi.User{ID: myUserID})
apiServer := httptest.NewServer(apiHandler)
defer apiServer.Close()

f := newCommandFactory(t, apiServer.URL, authServer.URL)

output := f.Run("ssh-cert:info")
assert.Regexp(t, `(?m)^filename: .+?id_ed25519-cert\.pub$`, output)
assert.Contains(t, output, "key_id: test-key-id\n")
assert.Contains(t, output, "key_type: ssh-ed25519-cert-v01@openssh.com\n")
cases := []struct {
name string
algorithm string
filename string
keyType string
}{
{"ed25519", "ed25519", "id_ed25519", "ssh-ed25519-cert-v01@openssh.com"},
{"rsa", "rsa", "id_rsa", "ssh-rsa-cert-v01@openssh.com"},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
authServer := mockapi.NewAuthServer(t)
defer authServer.Close()

myUserID := "my-user-id"

apiHandler := mockapi.NewHandler(t)
apiHandler.SetMyUser(&mockapi.User{ID: myUserID})
apiServer := httptest.NewServer(apiHandler)
defer apiServer.Close()

f := newCommandFactory(t, apiServer.URL, authServer.URL)
f.extraEnv = []string{EnvPrefix + "SSH_CERT_KEY_ALGORITHM=" + c.algorithm}

output := f.Run("ssh-cert:info")
assert.Regexp(t, `(?m)^filename: .+?`+c.filename+`-cert\.pub$`, output)
assert.Contains(t, output, "key_id: test-key-id\n")
assert.Contains(t, output, "key_type: "+c.keyType+"\n")
})
}
}
9 changes: 9 additions & 0 deletions legacy/config-defaults.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,15 @@ ssh:
# often provides a weak security benefit.
cert_key_ttl: 86400

# The algorithm of the automatically generated SSH key pair.
#
# Valid values: "auto", "rsa" or "ed25519". The "auto" value means "rsa" if
# FIPS mode is enabled (detected on Linux via /proc/sys/crypto/fips_enabled),
# and "ed25519" otherwise. An invalid value is treated as "auto", with a
# warning.
# Overridden by the {application.env_prefix}SSH_CERT_KEY_ALGORITHM env var.
cert_key_algorithm: auto

# How the CLI detects and configures Git repositories as projects.
detection:
## Required keys that must be defined elsewhere:
Expand Down
58 changes: 51 additions & 7 deletions legacy/src/SshCert/Certifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,15 @@

class Certifier
{
public const KEY_ALGORITHM = 'ed25519';
public const PRIVATE_KEY_FILENAME = 'id_ed25519';
private const FIPS_ENABLED_FILE = '/proc/sys/crypto/fips_enabled';
// Algorithms supported by the certificate parser.
private const KEY_ALGORITHMS = ['rsa', 'ed25519'];
private readonly OutputInterface $stdErr;

private static bool $disableAutoLoad = false;

private string $keyAlgorithm;

public function __construct(private readonly Api $api, private readonly Config $config, private readonly Shell $shell, private readonly Filesystem $fs, OutputInterface $output, private readonly FileLock $fileLock)
{
$this->stdErr = $output instanceof ConsoleOutputInterface ? $output->getErrorOutput() : $output;
Expand Down Expand Up @@ -83,7 +86,8 @@ private function doGenerateCertificate(bool $forceNewKey = false): Certificate
$dir = $this->config->getSessionDir(true) . DIRECTORY_SEPARATOR . 'ssh';
$this->fs->mkdir($dir, 0o700);

$privateKeyFilename = $dir . DIRECTORY_SEPARATOR . self::PRIVATE_KEY_FILENAME;
$keyAlgorithm = $this->keyAlgorithm();
$privateKeyFilename = $dir . DIRECTORY_SEPARATOR . 'id_' . $keyAlgorithm;
$certificateFilename = $privateKeyFilename . '-cert.pub';
$publicKeyFilename = $privateKeyFilename . '.pub';
$tempPrivateKeyFilename = $privateKeyFilename . '_tmp';
Expand All @@ -102,7 +106,7 @@ private function doGenerateCertificate(bool $forceNewKey = false): Certificate
|| ($keyTtl !== 0 && ($mtime = filemtime($privateKeyFilename)) && time() - $mtime > $keyTtl);

if ($regenerateKey) {
$this->generateSshKey($tempPrivateKeyFilename);
$this->generateSshKey($tempPrivateKeyFilename, $keyAlgorithm);

$publicContents = file_get_contents($tempPublicKeyFilename);
if (!$publicContents) {
Expand Down Expand Up @@ -156,7 +160,7 @@ private function doGenerateCertificate(bool $forceNewKey = false): Certificate
public function getExistingCertificate(): ?Certificate
{
$dir = $this->config->getSessionDir(true) . DIRECTORY_SEPARATOR . 'ssh';
$private = $dir . DIRECTORY_SEPARATOR . self::PRIVATE_KEY_FILENAME;
$private = $dir . DIRECTORY_SEPARATOR . 'id_' . $this->keyAlgorithm();
$cert = $private . '-cert.pub';

$exists = file_exists($private) && file_exists($cert);
Expand Down Expand Up @@ -243,19 +247,59 @@ public function certificateConflictsWithJwt(Certificate $certificate, ?string $j
return false;
}

/**
* Returns the algorithm for the temporary SSH key pair.
*/
private function keyAlgorithm(): string
{
if (isset($this->keyAlgorithm)) {
return $this->keyAlgorithm;
}
try {
$algorithm = self::resolveKeyAlgorithm($this->config->getStr('ssh.cert_key_algorithm'), self::FIPS_ENABLED_FILE);
} catch (\InvalidArgumentException $e) {
$this->stdErr->writeln('<comment>Warning:</comment> ' . $e->getMessage());
$algorithm = self::resolveKeyAlgorithm('auto', self::FIPS_ENABLED_FILE);
}
return $this->keyAlgorithm = $algorithm;
}

/**
* Resolves the configured key algorithm, detecting FIPS mode for "auto".
*
* @param string $configured
* The configured algorithm: "auto", "rsa" or "ed25519".
* @param string $fipsEnabledFile
* The file indicating whether FIPS mode is enabled (Linux only).
*/
public static function resolveKeyAlgorithm(string $configured, string $fipsEnabledFile): string
{
$configured = strtolower(trim($configured));
if (in_array($configured, self::KEY_ALGORITHMS, true)) {
return $configured;
}
if ($configured !== '' && $configured !== 'auto') {
throw new \InvalidArgumentException(sprintf('Invalid configuration value for ssh.cert_key_algorithm: %s (expected "auto", "rsa" or "ed25519")', $configured));
}
$fipsEnabled = is_readable($fipsEnabledFile) && trim((string) file_get_contents($fipsEnabledFile)) === '1';
return $fipsEnabled ? 'rsa' : 'ed25519';
}

/**
* Generate an SSH key pair to request a new certificate.
*
* @param string $filename
* The private key filename.
* @param string $algorithm
* The key algorithm (passed to ssh-keygen -t).
*/
private function generateSshKey(string $filename): void
private function generateSshKey(string $filename, string $algorithm): void
{
$this->stdErr->writeln('Generating local key pair', OutputInterface::VERBOSITY_VERBOSE);

$args = [
'ssh-keygen',
'-t', self::KEY_ALGORITHM,
'-t', $algorithm,
'-f', $filename,
'-N', '', // No passphrase
'-C', $this->config->getStr('application.slug') . '-temporary-cert', // Key comment
Expand Down
51 changes: 51 additions & 0 deletions legacy/tests/SshCert/CertifierTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

declare(strict_types=1);

namespace Platformsh\Cli\Tests\SshCert;

use PHPUnit\Framework\TestCase;
use Platformsh\Cli\SshCert\Certifier;
use Platformsh\Cli\Tests\HasTempDirTrait;

class CertifierTest extends TestCase
{
use HasTempDirTrait;

public function testResolveKeyAlgorithm(): void
{
$this->tempDirSetUp();
assert($this->tempDir !== null);
$fipsOn = $this->tempDir . '/fips-on';
$fipsOff = $this->tempDir . '/fips-off';
$missing = $this->tempDir . '/missing';
file_put_contents($fipsOn, "1\n");
file_put_contents($fipsOff, "0\n");

$cases = [
['auto', $fipsOn, 'rsa'],
['auto', $fipsOff, 'ed25519'],
['auto', $missing, 'ed25519'],
['', $fipsOn, 'rsa'],
['', $missing, 'ed25519'],
['ed25519', $fipsOn, 'ed25519'],
['rsa', $fipsOff, 'rsa'],
[' RSA ', $fipsOff, 'rsa'],
];
foreach ($cases as [$configured, $fipsFile, $expected]) {
$this->assertSame($expected, Certifier::resolveKeyAlgorithm($configured, $fipsFile), sprintf('configured "%s" with %s', $configured, basename($fipsFile)));
}
}

public function testResolveKeyAlgorithmRejectsUnsupported(): void
{
foreach (['ecdsa', '../rsa'] as $value) {
try {
Certifier::resolveKeyAlgorithm($value, '/nonexistent');
$this->fail('Expected exception for: ' . $value);
} catch (\InvalidArgumentException) {
$this->addToAssertionCount(1);
}
}
}
}
Loading