TLS: Tls.vhost is not validating the common name(CN/HostName) of the server #7682
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…in server certificate While using the TLS, we set verify to true if we want to verify server certificate. What fluent-bit verifies here is mostly only the validitiy. If vhost is configured and verify is set to true, it does not validate hostname/common-name field of server certificate. As part of this commit, if verify is set and vhost is configured, the server will be validated against the configuerd hostname. If it does not match then TLS handshake fails with invalid certificate error.
muttanna2972
requested review from
edsiper,
leonardo-albertovich,
fujimotos and
koleini
as code owners
muttanna2972
changed the title
muttanna2972
changed the title
muttanna2972
changed the title
leonardo-albertovich
requested changes
Jul 19, 2023
|
@muttanna2972 pls check the comments provided |
|
Currently addressing the review comments. Once testing is done on the new changes, will commit the same |
|
@muttanna2972 ping |
|
Hi @edsiper, I have addressed the comments, in my local repo, I just need to test it and push the changes. Apologies for the delay. |
|
@leonardo-albertovich , @edsiper I have tried to addressed all the comments and pushed one more commit. Please review. |
|
please sign-off your commits, otherwise we get a DCO error |
…server fluent#7682 signed-off-by: Muttanna Hosur muttanna2972@gmail.com
|
@leonardo-albertovich could you please review the changes? |
|
This is superseded by: #8934 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While using the TLS, we set verify to true if we want to verify server certificate. What fluent-bit verifies here is mostly only the validity and authenticity (signed by known CA or not) or not. But If vhost is configured and verify is set to true, it does not validate hostname/common-name field of server certificate.
As part of this commit, if verify is set and vhost is configured, the server will be validated against the configuerd hostname. If it does not match then TLS handshake fails with invalid certificate error.
Code change details:
Using
X509_VERIFY_PARAM_set1_hostto set verification of host parameter so that server certificate is validated during the handshakeSigned-off-by: Muttanna Hosur muttanna2972@gmail.com
#7178
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
Configuration at Client:
Name Tail
Tag local3_2.3.4.5
Path /var/log/firewall
DB /tmp/local3_2.3.4.5
Buffer_Max_Size 64k
Buffer_Chunk_Size 64k
Mem_Buf_Limit 512
Refresh_Interval 30
[OUTPUT]
Name Syslog
Match local3_2.3.4.5
Host 10.10.10.30
Port 38867
Mode TCP
Syslog_Format rfc3164
Syslog_Message_Key log
net.connect_timeout 50
tls on
tls.verify true
tls.vhost devserver.com
tls.ca_file /home/mhosur/keys/ca.crt
tls.crt_file /home/mhosur/keys/flb_client.crt
tls.key_file /home/mhosur/keys/flb_client.key
[INPUT]
Name Tail
Tag local3_2.3.4.5
Path /var/log/firewall
DB /tmp/local3_2.3.4.5
Buffer_Max_Size 64k
Buffer_Chunk_Size 64k
Mem_Buf_Limit 512
Refresh_Interval 30
[INPUT]
Name syslog
#Tag local3_2.3.4.5
Listen 10.10.10.30
Port 38867
Parser syslog-rfc3164
Mode TCP
Buffer_Chunk_Size 64KB
tls on
tls.ca_file /home/mhosur/keys/ca.crt
tls.crt_file /home/mhosur/keys/server.crt
tls.key_file /home/mhosur/keys/server.key
-->
[[1m[^[[0m2023/06/28 16:41:07^[[1m]^[[0m [^[[92m info^[[0m] [sp] stream processor started
^[[1m[^[[0m2023/06/28 16:41:07^[[1m]^[[0m [^[[93mdebug^[[0m] [input:tail:tail.0] inode=2270112 file=/var/log/firewall promote to TAIL_EVENT
^[[1m[^[[0m2023/06/28 16:41:07^[[1m]^[[0m [^[[92m info^[[0m] [input:tail:tail.0] inotify_fs_add(): inode=2270112 watch_fd=1 name=/var/log/firewall
^[[1m[^[[0m2023/06/28 16:41:07^[[1m]^[[0m [^[[93mdebug^[[0m] [input:tail:tail.0] [static files] processed 0b, done
^[[1m[^[[0m2023/06/28 16:41:07^[[1m]^[[0m [^[[92m info^[[0m] [output:stdout:stdout.1] worker #0 started
^[[1m[^[[0m2023/06/28 16:41:09^[[1m]^[[0m [^[[93mdebug^[[0m] [input:tail:tail.0] inode=2270112 events: IN_MODIFY
^[[1m[^[[0m2023/06/28 16:41:09^[[1m]^[[0m [^[[93mdebug^[[0m] [input chunk] update output instances with new chunk size diff=28
^[[1m[^[[0m2023/06/28 16:41:09^[[1m]^[[0m [^[[93mdebug^[[0m] [task] created task=0x7f56f412f020 id=0 OK
^[[1m[^[[0m2023/06/28 16:41:09^[[1m]^[[0m [^[[93mdebug^[[0m] [output:stdout:stdout.1] task_id=0 assigned to thread #0
^[[1m[^[[0m2023/06/28 16:41:09^[[1m]^[[0m [^[[91merror^[[0m] XXX the value of tls vhost devserver.com and vhost (null)
^[[1m[^[[0m2023/06/28 16:41:09^[[1m]^[[0m [^[[93mdebug^[[0m] [out flush] cb_destroy coro_id=0
^[[1m[^[[0m2023/06/28 16:41:09^[[1m]^[[0m [^[[93mdebug^[[0m] [out flush] cb_destroy coro_id=0
^[[1m[^[[0m2023/06/28 16:41:09^[[1m]^[[0m [^[[93mdebug^[[0m] [task] destroy task=0x7f56f412f020 (task_id=0)
^[[1m[^[[0m2023/06/28 16:41:09^[[1m]^[[0m [^[[93mdebug^[[0m] [socket] could not validate socket status for #56 (don't worry)
^[[1m[^[[0m2023/06/28 16:41:21^[[1m]^[[0m [^[[93mdebug^[[0m] [input:tail:tail.0] inode=2270112 events: IN_MODIFY
^[[1m[^[[0m2023/06/28 16:41:21^[[1m]^[[0m [^[[93mdebug^[[0m] [input chunk] update output instances with new chunk size diff=28
^[[1m[^[[0m2023/06/28 16:41:21^[[1m]^[[0m [^[[93mdebug^[[0m] [task] created task=0x7f56f4158e40 id=0 OK
^[[1m[^[[0m2023/06/28 16:41:21^[[1m]^[[0m [^[[93mdebug^[[0m] [output:stdout:stdout.1] task_id=0 assigned to thread #0
^[[1m[^[[0m2023/06/28 16:41:21^[[1m]^[[0m [^[[93mdebug^[[0m] [out flush] cb_destroy coro_id=1
^[[1m[^[[0m2023/06/28 16:41:21^[[1m]^[[0m [^[[91merror^[[0m] XXX the value of tls vhost devserver.com and vhost (null)
^[[1m[^[[0m2023/06/28 16:41:21^[[1m]^[[0m [^[[93mdebug^[[0m] [input:tail:tail.0] inode=2270112 events: IN_MODIFY
^[[1m[^[[0m2023/06/28 16:41:21^[[1m]^[[0m [^[[93mdebug^[[0m] [input chunk] update output instances with new chunk size diff=28
^[[1m[^[[0m2023/06/28 16:41:21^[[1m]^[[0m [^[[93mdebug^[[0m] [out flush] cb_destroy coro_id=1
^[[1m[^[[0m2023/06/28 16:41:21^[[1m]^[[0m [^[[93mdebug^[[0m] [task] destroy task=0x7f56f4158e40 (task_id=0)
^[[1m[^[[0m2023/06/28 16:41:21^[[1m]^[[0m [^[[93mdebug^[[0m] [socket] could not validate socket status for #56 (don't worry)
^[[1m[^[[0m2023/06/28 16:41:22^[[1m]^[[0m [^[[93mdebug^[[0m] [task] created task=0x7f56f4156d10 id=0 OK
^[[1m[^[[0m2023/06/28 16:41:22^[[1m]^[[0m [^[[93mdebug^[[0m] [output:stdout:stdout.1] task_id=0 assigned to thread #0
^[[1m[^[[0m2023/06/28 16:41:22^[[1m]^[[0m [^[[93mdebug^[[0m] [out flush] cb_destroy coro_id=2
^[[1m[^[[0m2023/06/28 16:41:22^[[1m]^[[0m [^[[91merror^[[0m] XXX the value of tls vhost devserver.com and vhost (null)
^[[1m[^[[0m2023/06/28 16:41:22^[[1m]^[[0m [^[[93mdebug^[[0m] [out flush] cb_destroy coro_id=2
[[1m[^[[0m2023/06/28 16:41:22^[[1m]^[[0m [^[[91merror^[[0m] XXX the value of tls vhost devserver.com and vhost (null)
^[[1m[^[[0m2023/06/28 16:41:22^[[1m]^[[0m [^[[93mdebug^[[0m] [out flush] cb_destroy coro_id=2
^[[1m[^[[0m2023/06/28 16:41:22^[[1m]^[[0m [^[[93mdebug^[[0m] [task] destroy task=0x7f56f4156d10 (task_id=0)
^[[1m[^[[0m2023/06/28 16:41:22^[[1m]^[[0m [^[[93mdebug^[[0m] [socket] could not validate socket status for #56 (don't worry)
^[[1m[^[[0m2023/06/28 16:41:37^[[1m]^[[0m [^[[93mdebug^[[0m] [input:tail:tail.0] scanning path /var/log/firewall
^[[1m[^[[0m2023/06/28 16:41:37^[[1m]^[[0m [^[[93mdebug^[[0m] [input:tail:tail.0] scan_blog add(): dismissed: /var/log/firewall, inode 2270112
^[[1m[^[[0m2023/06/28 16:41:37^[[1m]^[[0m [^[[93mdebug^[[0m] [input:tail:tail.0] 0 new files found on path '/var/log/firewall'
^[[1m[^[[0m2023/06/28 16:42:07^[[1m]^[[0m [^[[93mdebug^[[0m] [input:tail:tail.0] scanning path /var/log/firewall
^[[1m[^[[0m2023/06/28 16:42:07^[[1m]^[[0m [^[[93mdebug^[[0m] [input:tail:tail.0] scan_blog add(): dismissed: /var/log/firewall, inode 2270112
^[[1m[^[[0m2023/06/28 16:42:07^[[1m]^[[0m [^[[93mdebug^[[0m] [input:tail:tail.0] 0 new files found on path '/var/log/firewall'
root@mhosur:/home/mhosur/flb_dev/fork/fluent-bit# valgrind ./build/bin/fluent-bit -f1 -c /usr/local/etc/fluent-bit/fluent-bit.conf
==61949== Memcheck, a memory error detector
==61949== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==61949== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==61949== Command: ./build/bin/fluent-bit -f1 -c /usr/local/etc/fluent-bit/fluent-bit.conf
==61949==
Fluent Bit v2.1.7
[2023/07/11 10:25:08] [ info] Configuration:
[2023/07/11 10:25:08] [ info] flush time | 1.000000 seconds
[2023/07/11 10:25:08] [ info] grace | 5 seconds
[2023/07/11 10:25:08] [ info] daemon | 0
[2023/07/11 10:25:08] [ info] ___________
[2023/07/11 10:25:08] [ info] inputs:
[2023/07/11 10:25:08] [ info] tail
[2023/07/11 10:25:08] [ info] ___________
[2023/07/11 10:25:08] [ info] filters:
[2023/07/11 10:25:08] [ info] ___________
[2023/07/11 10:25:08] [ info] outputs:
[2023/07/11 10:25:08] [ info] syslog.0
[2023/07/11 10:25:08] [ info] stdout.1
[2023/07/11 10:25:08] [ info] ___________
[2023/07/11 10:25:08] [ info] collectors:
==61949== Warning: client switching stacks? SP change: 0x5fea148 --> 0x57cb550
==61949== to suppress, use: --max-stackframe=8514552 or greater
==61949== Warning: client switching stacks? SP change: 0x57cb4a8 --> 0x5fea148
==61949== to suppress, use: --max-stackframe=8514720 or greater
==61949== Warning: client switching stacks? SP change: 0x6ff42a8 --> 0x57d1900
==61949== to suppress, use: --max-stackframe=25307560 or greater
==61949== further instances of this message will not be shown.
[0] local3_2.3.4.5: [[1689071109.650585363, {}], {"log"=>"hello firewall 2"}]
[0] local3_2.3.4.5: [[1689071161.882649655, {}], {"log"=>"hello firewall 3"}]
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
] Set
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.