Skip to content

Add settings for minimum key strength and allowed key type

username-removed-35056 requested to merge electrickite/gitlab-ce:min-key-strength into master

What does this MR do?

This merge request adds three new application settings:

  1. minimum_rsa_bits: The minimum allowed bit length of an uploaded RSA key
  2. minimum_ecdsa_bits: The minimum allowed curve size (in bits) of an uploaded ECDSA key
  3. allowed_key_types: The SSH key types accepted by the application (RSA, DSA, or ECDSA)

To support these settings, it adds validations to the Key model to check length/type and three fields to the application settings page to allow an administrator to configure them. API support is also added.

Are there points in the code the reviewer needs to double check?

Based on the validations in the Key model, it looks like Gitlab currently accepts only RSA, DSA, and ECDSA keys. This assumption should be verified.

db/migrations/20160727193009_add_minimum_key_length_to_application_settings.rb contains some Active Record code to set the default value for allowed_key_types after the column is created since it is a serialized array. I'm not sure if there is a different/preferred way to do this.

Why was this MR needed?

@andrewjamescollett sums it up very well:

"The admin does not have control over the key length or type for the ssh keys. This is something that could become a problem as the brute force cracking of keys becomes less difficult."

ssh-keygen allows users to create RSA keys with as few as 768 bits, which falls well below recommendations from certain standards groups (such as the US NIST). Some organizations deploying Gitlab will need to enforce minimum key strength, either to satisfy internal security policy or for regulatory compliance.

Similarly, certain standards groups recommend using RSA or ECDSA over the older DSA and administrators may need to limit the allowed SSH key algorithms.

What are the relevant issue numbers?

Feature request #17849 (closed) would be resolved by this MR.

Screenshots (if relevant)

Application settings screen before:
before

Applications settings screen after:
with_length_and_type

Does this MR meet the acceptance criteria?

Merge request reports