Resolve most Gosec suggestions #338

jsirianni · 2021-06-23T23:44:08Z

Description of Changes

This PR resolves most suggestions from gosec ./....

Potentially breaking changes in behavior

File Permissions

Gosec suggests that files should be restricted to the user, 0600 for example will give read and write to the user running the process and nobody else. I personally agree but I don't have a strong stance on how Stanza should behave. It is possible that the Database, Buffers, and Stanza Log Files will contain sensitive data.

None of the files generated by stanza (database, buffers, log file file, file output) require execute permission so I changed anything with 07xx to 06xx.

Please check that the PR fulfills these requirements

Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)
Add a changelog entry (for non-trivial bug fixes / features)
CI passes

…ror log can expose sensative information

…g files

…atabase can contain sensative information

…ing stanza. handle errors from closing files

djaglowski · 2021-06-23T23:50:05Z

Log Files	Logs / Second	CPU Avg (%)	CPU Avg Δ (%)	Memory Avg (MB)	Memory Avg Δ (MB)
1	1000	1.5517654	+0.103422046	128.64777	-0.22776794
1	5000	4.913927	+0.20685148	137.99905	+3.8657074
1	10000	9.500093	-0.46544266	142.36247	-4.3743286
1	50000	50.518597	+2.2252579	175.84833	+3.441269
1	100000	91.360535	-6.9166794	239.5726	+2.6201477
10	100	1.9655418	+0.18963242	135.27277	+1.422409
10	500	5.4829364	+8.6307526e-05	139.87971	-0.2316742
10	1000	11.241725	-0.9637909	146.8086	-2.8468475
10	5000	59.82815	+3.3960686	174.96498	-2.510498
10	10000	94.99771	-23.659126	218.13861	-15.270599

…e should be 0600

djaglowski · 2021-06-24T00:03:36Z

Log Files	Logs / Second	CPU Avg (%)	CPU Avg Δ (%)	Memory Avg (MB)	Memory Avg Δ (MB)
1	1000	1.5517534	+0.103410006	128.3036	-0.57192993
1	5000	4.879511	+0.17243528	138.37082	+4.2374725
1	10000	10.000203	+0.034667015	145.87056	-0.86624146
1	50000	46.3632	-1.9301376	175.19707	+2.7900085
1	100000	98.11804	-0.15917206	237.0272	+0.07475281
10	100	1.8621333	+0.08622384	132.22952	-1.6208344
10	500	5.931189	+0.448339	137.9911	-2.120285
10	1000	11.776102	-0.4294138	143.24637	-6.409073
10	5000	50.62089	-5.8111916	174.23343	-3.2420502
10	10000	106.51048	-12.146355	230.79027	-2.6189423

…or is never checked. Instead, log the error because we want to continue

…ction

…e old versions of TLS, so we will support 1.0 and newer. An issue has been filed to make the min version a configuratin option for environments that require a higher min version

djaglowski · 2021-06-30T23:24:45Z

Log Files	Logs / Second	CPU Avg (%)	CPU Avg Δ (%)	Memory Avg (MB)	Memory Avg Δ (MB)
1	1000	1.3965567	-3.9339066e-06	130.72076	+1.967804
1	5000	5.103608	-0.22409534	136.12958	-1.2580872
1	10000	10.0504465	+0.136199	142.33311	-1.9172821
1	50000	51.016155	+4.133812	177.40195	+1.1764679
1	100000	100.755066	+12.452103	240.94221	+14.62204
10	100	1.7586685	+0.03453505	130.61356	-2.3701477
10	500	5.2242293	-0.12062836	139.85628	-0.7117462
10	1000	11.896795	+1.1381569	148.85991	+3.1363068
10	5000	49.03485	-5.1209183	175.9414	-1.4108276
10	10000	111.09935	+3.5727081	227.7046	+1.4008636

jsirianni · 2021-07-01T22:17:05Z

Holding off on merging, as Windows has issues with the new permissions

--- FAIL: TestPersisterLoad (0.01s)
    util.go:27: remove C:\Users\circleci\AppData\Local\Temp\837055480\test.db: The process cannot access the file because it is being used by another process.

djaglowski · 2021-07-08T02:02:26Z

Log Files	Logs / Second	CPU Avg (%)	CPU Avg Δ (%)	Memory Avg (MB)	Memory Avg Δ (MB)
1	1000	1.4137814	+0.017193675	126.6382	-2.355194
1	5000	4.775944	-0.086218834	135.17564	-0.11126709
1	10000	9.689806	-1.0514202	144.21431	+0.6961212
1	50000	50.37477	+5.114975	173.2399	+2.8207245
1	100000	93.63939	-3.0081253	230.3459	-0.99812317
10	100	1.8276143	+4.351139e-05	134.09631	-0.041748047
10	500	5.827616	-0.12072897	139.96309	+1.8591003
10	1000	11.344997	-0.3273468	149.08594	+0.76091003
10	5000	50.310246	-2.6447449	170.49124	-0.53286743
10	10000	102.002106	-2.1593094	221.23909	-8.632538

djaglowski · 2021-07-08T02:20:09Z

Log Files	Logs / Second	CPU Avg (%)	CPU Avg Δ (%)	Memory Avg (MB)	Memory Avg Δ (MB)
1	1000	1.4827673	+0.086179614	128.49043	-0.5029602
1	5000	4.9138565	+0.05169344	137.89938	+2.6124725
1	10000	9.810335	-0.93089104	144.48465	+0.9664612
1	50000	51.104256	+5.8444595	178.91136	+8.4921875
1	100000	96.49438	-0.1531372	234.54579	+3.201767
10	100	1.7930857	-0.0344851	132.16595	-1.9721069
10	500	5.8620706	-0.086274624	138.63954	+0.535553
10	1000	11.500226	-0.17211819	147.32974	-0.99528503
10	5000	53.897736	+0.9427452	177.23047	+6.20636
10	10000	102.99433	-1.1670837	227.57611	-2.295517

djaglowski · 2021-07-08T03:06:40Z

Log Files	Logs / Second	CPU Avg (%)	CPU Avg Δ (%)	Memory Avg (MB)	Memory Avg Δ (MB)
1	1000	1.5690066	+0.17241883	127.7799	-1.2134933
1	5000	4.793154	-0.069009304	138.16406	+2.8771515
1	10000	10.086335	-0.654891	145.86975	+2.3515625
1	50000	49.105	+3.8452034	168.31842	-2.1007538
1	100000	97.345276	+0.69776154	222.39143	-8.952591
10	100	2.08623	+0.25865924	133.7197	-0.41836548
10	500	5.8622885	-0.08605671	136.3812	-1.7227936
10	1000	13.241648	+1.5693035	146.84886	-1.4761658
10	5000	55.24232	+2.2873306	175.60452	+4.580414
10	10000	106.17302	+2.0116043	228.61261	-1.259018

codecov · 2021-07-08T03:13:39Z

Codecov Report

Merging #338 (e0ca031) into master (a6780d3) will decrease coverage by 0.12%.
The diff coverage is 33.33%.

@@            Coverage Diff             @@
##           master     #338      +/-   ##
==========================================
- Coverage   69.61%   69.49%   -0.12%     
==========================================
  Files         123      123              
  Lines        6508     6519      +11     
==========================================
  Hits         4530     4530              
- Misses       1499     1509      +10     
- Partials      479      480       +1

Impacted Files	Coverage Δ
cmd/stanza/logging.go	`46.15% <0.00%> (ø)`
cmd/stanza/root.go	`39.42% <0.00%> (-1.58%)`	⬇️
operator/buffer/disk.go	`65.27% <0.00%> (ø)`
...perator/builtin/input/aws/cloudwatch/cloudwatch.go	`28.34% <0.00%> (+0.15%)`	⬆️
operator/builtin/input/journald/journald.go	`56.12% <0.00%> (ø)`
operator/builtin/input/udp/udp.go	`70.18% <0.00%> (-3.04%)`	⬇️
operator/builtin/output/forward/forward.go	`55.07% <0.00%> (ø)`
operator/builtin/parser/time/time.go	`66.67% <0.00%> (+4.17%)`	⬆️
cmd/stanza/offsets.go	`59.70% <25.00%> (-2.20%)`	⬇️
operator/builtin/input/file/file.go	`76.43% <33.33%> (+0.79%)`	⬆️
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a6780d3...e0ca031. Read the comment docs.

jsirianni added 15 commits June 23, 2021 19:32

gosec: allow loading user defined file

4d20bd3

gosec: limit stanza log to the user that is running stanza. stanza er…

0e59836

…ror log can expose sensative information

handle errors returned by stderr / stdout

b55c96c

gosec: allow net/http/pprof's http webhook. handle errors from closin…

a957851

…g files

gosec: limit stanza database to user that is running stanza. stanza d…

c9e3b01

…atabase can contain sensative information

gosec: allow user to pass in files. restrict buffer file to user runn…

71a511a

…ing stanza. handle errors from closing files

handle error from closing connection

316448c

gosec: limit permissions for file output file to the user running stanza

12f3f51

handle errors from closing body

3072f52

handle errors from closing body

c0ee888

return the error from t.ProcessWith

1e53f35

handle error from base.Set

d25c788

gosec: allow loading user defined file

018a561

gosec: allow loading user defined file

559bd13

gosec: handle errors

39b0e1e

database directory is fine with 0755 permissions but the database fil…

df00c72

…e should be 0600

jsirianni added 7 commits June 30, 2021 18:46

Merge branch 'master' into resolve-gosec

722a259

remove return type error from handleEvent and handleEvents as the err…

623a6e3

…or is never checked. Instead, log the error because we want to continue

return file.Close() error

7e0396b

log errors returned by file.Close()

db20e4a

nosec, user defines the input file

955dab0

journalctl is an executable that is required for this operator to fun…

67c0ce1

…ction

gosec wants TLS 1.2/1.3 but Go defaults to 1.0. Some users may requir…

79df13f

…e old versions of TLS, so we will support 1.0 and newer. An issue has been filed to make the min version a configuratin option for environments that require a higher min version

djaglowski previously approved these changes Jul 1, 2021

View reviewed changes

jsirianni added the do not merge label Jul 1, 2021

Merge branch 'master' into resolve-gosec

f972beb

Merge branch 'master' into resolve-gosec

8245996

remove inappropriate use of failnow

53954d1

jsirianni dismissed djaglowski’s stale review via 53954d1 July 8, 2021 02:07

close open files before test finishes

e0ca031

jsirianni requested a review from djaglowski July 8, 2021 13:19

jsirianni removed the do not merge label Jul 8, 2021

djaglowski approved these changes Jul 8, 2021

View reviewed changes

jsirianni merged commit 2113838 into master Jul 8, 2021

jsirianni deleted the resolve-gosec branch July 8, 2021 14:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resolve most Gosec suggestions #338

Resolve most Gosec suggestions #338

jsirianni commented Jun 23, 2021 •

edited

Loading

djaglowski commented Jun 23, 2021

djaglowski commented Jun 24, 2021

djaglowski commented Jun 30, 2021

jsirianni commented Jul 1, 2021

djaglowski commented Jul 8, 2021

djaglowski commented Jul 8, 2021

djaglowski commented Jul 8, 2021

codecov bot commented Jul 8, 2021 •

edited

Loading

Resolve most Gosec suggestions #338

Resolve most Gosec suggestions #338

Conversation

jsirianni commented Jun 23, 2021 • edited Loading

Description of Changes

Potentially breaking changes in behavior

File Permissions

Please check that the PR fulfills these requirements

djaglowski commented Jun 23, 2021

djaglowski commented Jun 24, 2021

djaglowski commented Jun 30, 2021

jsirianni commented Jul 1, 2021

djaglowski commented Jul 8, 2021

djaglowski commented Jul 8, 2021

djaglowski commented Jul 8, 2021

codecov bot commented Jul 8, 2021 • edited Loading

Codecov Report

jsirianni commented Jun 23, 2021 •

edited

Loading

codecov bot commented Jul 8, 2021 •

edited

Loading