Add support for TLS client authentication
Allow gitlab-ci to connect to a gitlab host using TLS client authentication (mutual authentication). Adds configuration and support for using TLS client certificates when using go's TLS transport layer and also sets git enviromental variables for runners.
See also #1291 (closed) and !86 (closed)
Merge request reports
Activity
@sfnelson Thanks for this MR and your contribution to GitLab Runner. @tmaczukin @ayufan can you take a look at this MR?
Hmm, this looks like a complete implementation that should work covering everything :)
I have only concern with exposing client certs to build container. Maybe we should remove them after cloning sources?
Other then that Awesome work @sfnelson. I like your changes and I think that we should merge them
- Last reply by Kamil Trzcińśki
@sfnelson We should also support client certificates by this command helpers: https://gitlab.com/gitlab-org/gitlab-ci-multi-runner/tree/master/commands/helpers (they are used to upload/download artifacts).
It means setting:
CI_SERVER_TLS_CLIENT_CERT_FILEandCI_SERVER_TLS_CLIENT_KEY_FILEbefore executing this commands.Edited by Kamil TrzcińśkiMentioned in merge request !340 (closed)
mentioned in issue #1736
added 991 commits
- b3a7af96...cdb231f2 - 990 commits from branch
gitlab-org:master - b2d13f06 - Add support for TLS client authentication
- b3a7af96...cdb231f2 - 990 commits from branch
I've updated this merge request. @ayufan would you take a look please? It seems like a commonly requested feature, would be great to see it merged.
@tmaczukin Could you review it?
added 75 commits
- 12529a6b...ef0ca338 - 74 commits from branch
gitlab-org:master - 51bb37bc - Add support for TLS client authentication
- 12529a6b...ef0ca338 - 74 commits from branch
assigned to @tmaczukin
@tmaczukin I'm currently debugging a potential problem from porting my changes to master. The code is ready for review, but please don't merge it until I confirm that it's ready.
@tmaczukin this issue is extremely important to our business. What can I do to keep the process moving?
@sfnelson I'll review this today :)
@tmaczukin did you get a chance to review this MR? Do you have any comments?
Hi @tmaczukin, have you had a chance to look at this yet?
@sfnelson Looking on this right now :)
@sfnelson Looks really nice :).
I left few comments. Please fix these and resolve conflicts. Meanwhile I'll try to perform some manual tests using client authentication and this MR :)
mentioned in merge request !86 (closed)
changed milestone to %v9.1
added improvement label
@tmaczukin ready for review
@sfnelson I left two comments
@tmaczukin oops, done
@sfnelson I think all is good now.
However we didn't have time to install this with RC version on our shared runners to check if this doesn't introduce any regression. And since this MR touches places where communication with GitLab is made, any regression may affect all users - even if they are not using TLS Client Authentication. I don't think that merging this a day before release will be a good idea.
I'm moving this MR to 9.2. It will be merged at Monday so anyone will be able to install this with Bleeding Edge version which will be basically 9.2 with this one change.
Thank you for your awesome work on this MR!
changed milestone to %v9.2
mentioned in issue #1291 (closed)
@tmaczukin awesome, thanks
mentioned in issue #2570 (closed)
