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

Jump to bottom

Implement Ingress Secure Keys #420

Closed
wants to merge 4 commits into from
Closed

Implement Ingress Secure Keys #420

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
54d12c7
Implement Ingress Secure Keys
P-EB Sep 25, 2023
5af7e55
Escape dots in regex patterns for ssh algorithms
P-EB Sep 25, 2023
b9b4899
Factor out in a generic function the helper listing allowed ssh key a…
P-EB Sep 25, 2023
72be788
Drop an unused variable in print_accepted_key_algorithms
P-EB Sep 25, 2023
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
15 changes: 1 addition & 14 deletions bin/plugin/open/selfAddIngressKey
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,21 +61,8 @@ if (!OVH::Bastion::has_piv_helper()) {
}

if (not defined $pubKey) {
$fnret = OVH::Bastion::get_supported_ssh_algorithms_list(way => 'ingress');
$fnret or osh_exit $fnret;
my @algoList = @{$fnret->value};
my $algos = join(' ', @algoList);
osh_info "Please paste the SSH key you want to add. This bastion supports the following algorithms:\n";

if (grep { 'ed25519' eq $_ } @algoList) {
osh_info "ED25519: strongness[#####] speed[#####], use `ssh-keygen -t ed25519' to generate one";
}
if (grep { 'ecdsa' eq $_ } @algoList) {
osh_info "ECDSA : strongness[####.] speed[#####], use `ssh-keygen -t ecdsa -b 521' to generate one";
}
if (grep { 'rsa' eq $_ } @algoList) {
osh_info "RSA : strongness[###..] speed[#....], use `ssh-keygen -t rsa -b 4096' to generate one";
}
OVH::Bastion::print_accepted_key_algorithms(way => "ingress");
osh_info "\nIn any case, don't save it without a passphrase.";

if (OVH::Bastion::config('ingressKeysFromAllowOverride')->value) {
Expand Down
14 changes: 1 addition & 13 deletions bin/plugin/restricted/accountCreate
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,20 +103,8 @@ if (defined $maxInactiveDays && $maxInactiveDays < 0) {
}

if (!$pubKey && !$noKey) {
$fnret = OVH::Bastion::get_supported_ssh_algorithms_list(way => 'ingress');
$fnret or osh_exit $fnret;
my @algoList = @{$fnret->value};
my $algos = join(' ', @algoList);
osh_info "Please paste the SSH key you want to add. This bastion supports the following algorithms:\n";
if (grep { 'ed25519' eq $_ } @algoList) {
osh_info "ED25519: strongness[#####] speed[#####], use `ssh-keygen -t ed25519' to generate one";
}
if (grep { 'ecdsa' eq $_ } @algoList) {
osh_info "ECDSA : strongness[####.] speed[#####], use `ssh-keygen -t ecdsa -b 521' to generate one";
}
if (grep { 'rsa' eq $_ } @algoList) {
osh_info "RSA : strongness[###..] speed[#....], use `ssh-keygen -t rsa -b 4096' to generate one";
}
OVH::Bastion::print_accepted_key_algorithms(way => "ingress");
osh_info "\nIn any case, don't save it without a passphrase (your paste won't be echoed).";
$pubKey = <STDIN>;
}
Expand Down
2 changes: 1 addition & 1 deletion etc/bastion/bastion.conf.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@
# allowedIngressSshAlgorithms (array of strings (algorithm names))
# DESC: The algorithms authorized for ingress ssh public keys added to this bastion. Possible values: ``dsa``, ``rsa``, ``ecdsa``, ``ed25519``, note that some of those might not be supported by your current version of ``OpenSSH``: unsupported algorithms are automatically omitted at runtime.
# DEFAULT: [ "rsa", "ecdsa", "ed25519" ]
"allowedIngressSshAlgorithms": [ "rsa", "ecdsa", "ed25519" ],
"allowedIngressSshAlgorithms": [ "rsa", "ecdsa", "ed25519", "ecdsa-sk", "ed25519-sk" ],
#
# allowedEgressSshAlgorithms (array of strings (algorithm names))
# DESC: The algorithms authorized for egress ssh public keys generated on this bastion. Possible values: ``dsa``, ``rsa``, ``ecdsa``, ``ed25519``, note that some of those might not be supported by your current version of ``OpenSSH``, unsupported algorithms are automatically omitted at runtime.
Expand Down
2 changes: 1 addition & 1 deletion lib/perl/OVH/Bastion.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ my %_autoload_files = (
],
password => [qw{ get_hashes_from_password get_password_file get_hashes_list is_valid_hash }],
ssh => [
qw{ has_piv_helper verify_piv get_authorized_keys_from_file add_key_to_authorized_keys_file put_authorized_keys_to_file get_ssh_pub_key_info is_valid_public_key get_from_for_user_key generate_ssh_key get_bastion_ips get_supported_ssh_algorithms_list is_allowed_algo_and_size is_valid_fingerprint print_public_key account_ssh_config_get account_ssh_config_set ssh_ingress_keys_piv_apply is_effective_piv_account_policy_enabled }
qw{ has_piv_helper verify_piv get_authorized_keys_from_file add_key_to_authorized_keys_file put_authorized_keys_to_file get_ssh_pub_key_info is_valid_public_key get_from_for_user_key generate_ssh_key get_bastion_ips get_supported_ssh_algorithms_list is_allowed_algo_and_size is_valid_fingerprint print_public_key account_ssh_config_get account_ssh_config_set ssh_ingress_keys_piv_apply is_effective_piv_account_policy_enabled print_accepted_key_algorithms }
],
);

Expand Down
4 changes: 2 additions & 2 deletions lib/perl/OVH/Bastion/configuration.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,8 @@ sub load_configuration {
## no critic(RegularExpressions::ProhibitFixedStringMatches)
{
name => 'allowedIngressSshAlgorithms',
default => [qw{ rsa ecdsa ed25519 }],
validre => qr/^(rsa|ecdsa|ed25519)$/
default => [qw{ rsa ecdsa ed25519 edcsa-sk ed25519-sk }],
validre => qr/^(rsa|ecdsa|ed25519|ecdsa-sk|ed25519-sk)$/
},
## no critic(RegularExpressions::ProhibitFixedStringMatches)
{
Expand Down
36 changes: 33 additions & 3 deletions lib/perl/OVH/Bastion/ssh.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,7 @@ sub get_ssh_pub_key_info {

my ($prefix, $typecode, $base64, $comment);
if ($pubKey =~
m{^\s*((\S+)\s+)?(ssh-dss|ssh-rsa|ecdsa-sha\d+-nistp\d+|ssh-ed\d+)\s+([a-zA-Z0-9/=+]+)(\s+(.{1,128})?)?$}
m{^\s*((\S+)\s+)?(ssh-dss|ssh-rsa|ecdsa-sha\d+-nistp\d+|ssh-ed\d+|sk-ssh-ed25519\@openssh\.com|sk-ecdsa-sha2-nistp256\@openssh\.com)\s+([a-zA-Z0-9/=+]+)(\s+(.{1,128})?)?$}
&& length($pubKey) <= 3000)
{
($prefix, $typecode, $base64, $comment) = ($2, $3, $4, $6);
Expand Down Expand Up @@ -360,7 +360,7 @@ sub get_ssh_pub_key_info {
256 SHA256:Yggd7VRRbbivxkdVwrdt0HpqKNylMK91nNIU+RxndTI john@doe (ED25519)
=cut

if (defined $sshkeygen and $sshkeygen =~ /^(\d+)\s+(\S+)\s+(.+)\s+\(([A-Z0-9]+)\)$/) {
if (defined $sshkeygen and $sshkeygen =~ /^(\d+)\s+(\S+)\s+(.+)\s+\(([A-Z0-9-]+)\)$/) {
my ($size, $fingerprint, $comment2, $family) = ($1, $2, $3, $4);
$return{'size'} = $size + 0;
$return{'fingerprint'} = $fingerprint;
Expand Down Expand Up @@ -436,7 +436,7 @@ EOS
$fnret->{'msg'} = "Unknown error (" . $fnret->msg . "), please report to your sysadmin.";
}
else {
if (not grep { $fnret->value->{'family'} eq $_ } qw{ RSA ECDSA ED25519 }) {
if (not grep { $fnret->value->{'family'} eq $_ } qw{ RSA ECDSA ED25519 ECDSA-SK ED25519-SK }) {
$fnret->{'err'} = 'ERR_UNKNOWN_TYPE';
$fnret->{'msg'} =
"Unknown family type (" . $fnret->value->{'family'} . "), please report to your sysadmin.";
Expand Down Expand Up @@ -634,6 +634,8 @@ sub get_supported_ssh_algorithms_list {
my $version = $1;
push @supportedList, 'ecdsa' if ($version gt "5.7");
push @supportedList, 'ed25519' if ($version gt "6.5");
push @supportedList, 'ecdsa-sk' if ($version gt "8.2");
push @supportedList, 'ed25519-sk' if ($version gt "8.2");
@cached_runtime_list = @supportedList;
last;
}
Expand Down Expand Up @@ -1046,4 +1048,32 @@ sub is_effective_piv_account_policy_enabled {
: R('KO_DISABLED', msg => "inherits the globally disabled policy");
}

# Deduces from the bastion config what algorithms are accepted.
sub print_accepted_key_algorithms {
my %params = @_;
my $way = $params{'way'};
my $fnret;

$fnret = OVH::Bastion::get_supported_ssh_algorithms_list(way => $way);
$fnret or osh_exit $fnret;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
$fnret or osh_exit $fnret;
$fnret or return $fnret;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@speed47 I kept the osh_exit because if the function crashes I guess the underlying command should crash, shouldn't it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey, sorry, the end of the week was kinda busy :(

All functions that can be found in .inc files are expected to return, even if they return an error (where $fnret is false), so you should indeed return and not exit here.
It's up to the caller to decide whether it should exit or do something else when you func returns an error.

osh_exit is only to be used in plugins (found in bin/plugins/*)
HEXIT stands for "helper exit" and is only to be used in helpers (called under sudo, found in bin/helpers/*)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack, willdo.

my @algoList = @{$fnret->value};
my $algos = join(' ', @algoList);
P-EB marked this conversation as resolved.
Show resolved Hide resolved

if (grep { 'ed25519-sk' eq $_ } @algoList) {
osh_info "FIDO2 Ed25519: strongness[######] speed[#####], use `ssh-keygen -t ed25519-sk' to generate one";
}
if (grep { 'ed25519' eq $_ } @algoList) {
osh_info "ED25519 : strongness[#####.] speed[#####], use `ssh-keygen -t ed25519' to generate one";
}
if (grep { 'ecdsa-sk' eq $_ } @algoList) {
osh_info "FIDO2 ECDSA : strongness[#####.] speed[#####], use `ssh-keygen -t ecdsa-sk -b 521' to generate one";
}
if (grep { 'ecdsa' eq $_ } @algoList) {
osh_info "ECDSA : strongness[####..] speed[#####], use `ssh-keygen -t ecdsa -b 521' to generate one";
}
if (grep { 'rsa' eq $_ } @algoList) {
osh_info "RSA : strongness[###...] speed[#....], use `ssh-keygen -t rsa -b 4096' to generate one";
}
}

1;
Loading