Skip to content

mtls support#720

Merged
ldemailly merged 13 commits intomasterfrom
mtls
Feb 26, 2023
Merged

mtls support#720
ldemailly merged 13 commits intomasterfrom
mtls

Conversation

@ldemailly
Copy link
Copy Markdown
Member

@ldemailly ldemailly commented Feb 25, 2023 •
edited
Loading

fixes #719

  • mtls option
  • generate test client cert and ca and use in tests for mtls
  • use log.LogRequest and log.TLSInfo updated there in 1.3.0 for showing peer cert in debug endpoint and log
  • mtls for grpc too
  • prep for 1.52
  • fix URLSchemeCheck called twice both with initDone:true #695 while at it : incompatible change: you can't reuse and HTTPOptions and change the scheme in the URL.
  • fix fortio server -h continuing instead of showing flags.

also used/depended on https://github.com/fortio/log/pull/5/files / cleaned up duplicate LogRequest and TLSInfo

In addition to the unit tests:

% go run . server -mtls -cacert cert-tmp/clica.crt -cert cert-tmp/server.crt -key cert-tmp/server.key &

client not providing a cert:

% curl --cacert cert-tmp/ca.crt https://localhost:8080/debug
18:16:01 http: TLS handshake error from 127.0.0.1:63245: tls: client didn't provide a certificate
curl: (56) OpenSSL SSL_read: error:14094412:SSL routines:ssl3_read_bytes:sslv3 alert bad certificate, errno 0

vs - when providing client cert:

% curl --cacert cert-tmp/ca.crt --cert cert-tmp/client.crt --key cert-tmp/client.key https://localhost:8080/debug 
18:17:11 Debug: GET /debug HTTP/2.0 127.0.0.1:63318 ()  "curl/7.86.0" https TLS_AES_128_GCM_SHA256 "CN=fake-client"
Φορτίο version dev  go1.19.6 arm64 darwin echo debug server up for 1m52.6s on MacBook-Air.local - request from 127.0.0.1:63318 https TLS_AES_128_GCM_SHA256 "CN=fake-client"

GET /debug HTTP/2.0

headers:

Host: localhost:8080
Accept: */*
User-Agent: curl/7.86.0

body:

(note the CommonName in the output)

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 25, 2023 •
edited
Loading

Codecov Report

Base: 89.1% // Head: 89.2% // Increases project coverage by +0.2% 🎉

Coverage data is based on head (38dd76a) compared to base (c044bbd).
Patch coverage: 92.5% of modified lines in pull request are covered.

Additional details and impacted files 
@@           Coverage Diff            @@
##           master    #720     +/-   ##
========================================
+ Coverage    89.1%   89.2%   +0.2%     
========================================
  Files          16      16             
  Lines        3921    3917      -4     
========================================
+ Hits         3492    3495      +3     
+ Misses        276     271      -5     
+ Partials      153     151      -2
Impacted Files Coverage Δ
fhttp/http_forwarder.go 90.3% <0.0%> (ø)
fgrpc/pingsrv.go 88.6% <80.0%> (-1.3%) ⬇️
fgrpc/grpcrunner.go 90.7% <100.0%> (ø)
fhttp/http_client.go 84.7% <100.0%> (+0.5%) ⬆️
fhttp/http_server.go 81.8% <100.0%> (-1.3%) ⬇️
fhttp/http_utils.go 99.1% <100.0%> (+1.8%) ⬆️
rapi/data.go 92.9% <100.0%> (ø)
rapi/restHandler.go 87.2% <100.0%> (ø)
fnet/network.go 90.1% <0.0%> (+0.5%) ⬆️
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ldemailly ldemailly mentioned this pull request Feb 25, 2023
Merged
wiardvanrij
wiardvanrij reviewed Feb 26, 2023
View reviewed changes
@ldemailly ldemailly merged commit 142cf8c into master Feb 26, 2023
@ldemailly ldemailly deleted the mtls branch February 26, 2023 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wiardvanrij wiardvanrij wiardvanrij approved these changes

Assignees

@wiardvanrij wiardvanrij

@bseenu bseenu

@wuhaoyujerry wuhaoyujerry

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

option to require client cert (mtls) URLSchemeCheck called twice both with initDone:true

4 participants

@ldemailly @wiardvanrij @bseenu @wuhaoyujerry