Let's Encrypt certificates for pages shouldn't be downloadable
In https://gitlab.com/gitlab-org/gitlab-ce/issues/28996
To reduce security risks we should never make the let's encrypt private key visible to any users at any time since they do not need to know it. Right now it would appear that private keys for certificates are visible when editing a pages domain. This should not be visible for let's encrypt certificates, so we should not give users ability to see uploaded key on domain edit page.
But current implementation of !26438 (merged) actually allows this.
Approach 1: newer show private key and certificate on edit page
We already have "summary" on show page:
We can add button "replace ssl certificate" to domain edit form. This button would just add certificate and private keys fields as they currently exist. Saving this form with empty fields would delete ssl certificates though.
To prevent users from accidentally removing their ssl certificate we can open modal window instead of adding fields to current form.
This approach is also match our current API: we don't show private_key
in API. And form can be rewritten in Vue without API changes.
Approach 2: Only obtain Let's Encrypt certificates for verified domains if verification is enabled
This way users would be able to see all certificates and private keys, but they won't be able to get ssl certificates from Let's Encrypt though GitLab for domains they haven't proved ownership of.
Approach 3: store in database the fact that particular certificate was obtained through Let's Encrypt
The following discussion from !26438 (merged) should be addressed:
-
@vshushlin started a discussion: (+4 comments) @nfriend I see a potential volnerability here.
- obtain Let's Encrypt certificate
- open edit domain page
- submit form with some invalid field(e.g. make add certificate field from browser inspector and put
abc
there), and turn off lets encrypt - get
key
value from this field
I don't remember how exactly it was avoided in previous version with 2 files, but I think I checked this and decided it was fine...
Since Let's Encrypt isn't yet fully implemented and enabled this issue is public.