Skip to content

Commit d881241

Browse files
committed
fix: Updated MissingMinVersionTLS query to check go version
1 parent 9773e8a commit d881241

File tree

6 files changed

+135
-27
lines changed

6 files changed

+135
-27
lines changed

go/src/docs/security/MissingMinVersionTLS/MissingMinVersionTLS.md

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,20 @@
11
# Missing MinVersion in tls.Config
2-
Golang's `tls.Config` struct accepts `MinVersion` parameter that sets minimum accepted TLS version. If the parameter is not provided, default value is used: TLS1.2 for clients, and TLS1.0 for servers. TLS1.0 is considered deprecated and should not be used.
2+
Golang's `tls.Config` struct accepts `MinVersion` parameter that sets minimum accepted TLS version. If the parameter is not provided, the default depends on the Go version in use:
33

4+
- Since **Go 1.18**, clients default to TLS 1.2 (previously TLS 1.0)
5+
- Since **Go 1.22**, servers also default to TLS 1.2 (previously TLS 1.0)
6+
7+
For projects that support older Go versions, leaving `MinVersion` unset may still permit TLS 1.0 or 1.1, which are deprecated and should not be used.
8+
9+
This query flags `tls.Config` values where `MinVersion` is never set explicitly and the project's `go.mod` declares support for:
10+
- **Go < 1.18** for client-side configs (when client default is TLS 1.0)
11+
- **Go < 1.22** for server-side configs (when server default is TLS 1.0)
412

513
## Recommendation
6-
Explicitly set tls version to an up-to-date one.
14+
Explicitly set the TLS version to TLS 1.2 or higher:
15+
- For projects using Go < 1.18: Set `MinVersion` for both clients and servers
16+
- For projects using Go 1.18-1.21: Set `MinVersion` for servers
17+
- For projects using Go >= 1.22: Defaults are secure, but explicit setting is still recommended
718

819

920
## Example
@@ -50,8 +61,15 @@ func main() {
5061
}
5162

5263
```
53-
In this example, the `http.Server` may be set with TLS configuration created by either `test1` or `test2` functions. The `test1` result will be highlighted by this query, as it fails to explicitly set minimum supported TLS version. The `test2` result will not be marked, even that it also uses the default value for minimum version. That is because the `test2` is explicit, and this query assumes that developers knew what they are doing.
64+
In this example, the `http.Server` may be set with TLS configuration created by either `test1` or `test2` functions. For projects with `go` directive < 1.22, the `test1` result will be highlighted by this query, as it fails to explicitly set minimum supported TLS version. The `test2` result will not be marked, even though it also uses the default value for minimum version. That is because the `test2` is explicit, and this query assumes that developers knew what they are doing.
65+
66+
Note: The query behavior depends on the `go` directive in `go.mod`:
67+
- **Go < 1.18**: Both client and server configs without MinVersion are flagged
68+
- **Go 1.18-1.21**: Only server configs without MinVersion are flagged
69+
- **Go >= 1.22**: No configs are flagged (both defaults are secure)
5470

5571

5672
## References
5773
* [tls.Config specification](https://pkg.go.dev/crypto/tls#Config)
74+
* [Go 1.18 Release Notes - TLS 1.0 and 1.1 disabled by default client-side](https://tip.golang.org/doc/go1.18#tls10)
75+
* [Go 1.22 Release Notes - TLS 1.2 default for servers](https://tip.golang.org/doc/go1.22#minor_library_changes)

go/src/security/MissingMinVersionTLS/MissingMinVersionTLS.qhelp

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,29 +4,57 @@
44
<qhelp>
55
<overview>
66
<p>
7-
Golang's <code>tls.Config</code> struct accepts <code>MinVersion</code> parameter that sets minimum accepted TLS version.
8-
If the parameter is not provided, default value is used: TLS1.2 for clients, and TLS1.0 for servers.
9-
TLS1.0 is considered deprecated and should not be used.
7+
Golang's <code>tls.Config</code> struct accepts a <code>MinVersion</code> parameter that sets the minimum accepted TLS version.
8+
If the parameter is not provided, the default depends on the Go version in use. Since Go 1.18, <code>crypto/tls</code> clients default to TLS 1.2 (previously TLS 1.0).
9+
Since Go 1.22, <code>crypto/tls</code> servers also default to TLS 1.2 (previously TLS 1.0).
10+
</p>
11+
<p>
12+
This query flags <code>tls.Config</code> values where <code>MinVersion</code> is never set explicitly and the project's
13+
<code>go.mod</code> declares support for a Go version where the defaults are insecure:
14+
</p>
15+
<ul>
16+
<li>Go &lt; 1.18 for client-side configs (when client default is TLS 1.0)</li>
17+
<li>Go &lt; 1.22 for server-side configs (when server default is TLS 1.0)</li>
18+
</ul>
19+
<p>
20+
TLS 1.0 and 1.1 are deprecated and should not be used.
1021
</p>
1122

1223
</overview>
1324
<recommendation>
14-
<p>Explicitly set tls version to an up-to-date one.</p>
25+
<p>Explicitly set the TLS version to TLS 1.2 or higher:</p>
26+
<ul>
27+
<li>For projects using Go &lt; 1.18: Set <code>MinVersion</code> for both clients and servers</li>
28+
<li>For projects using Go 1.18-1.21: Set <code>MinVersion</code> for servers</li>
29+
<li>For projects using Go &gt;= 1.22: Defaults are secure, but explicit setting is still recommended</li>
30+
</ul>
1531

1632
</recommendation>
1733
<example>
1834
<sample src="MissingMinVersionTLS.go" />
1935

2036
<p>In this example, the <code>http.Server</code> may be set with TLS configuration created by either <code>test1</code> or <code>test2</code> functions.
21-
The <code>test1</code> result will be highlighted by this query, as it fails to explicitly set minimum supported TLS version.
22-
The <code>test2</code> result will not be marked, even that it also uses the default value for minimum version.
23-
That is because the <code>test2</code> is explicit, and this query assumes that developers knew what they are doing.
37+
For projects with a <code>go</code> directive &lt; 1.22, the <code>test1</code> result will be highlighted by this query, as it fails to explicitly set minimum supported TLS version.
38+
The <code>test2</code> result will not be marked, even though it also uses the default value for minimum version.
39+
That is because the <code>test2</code> is explicit, and this query assumes that developers knew what they are doing.
2440
</p>
41+
<p>Note: The query behavior depends on the <code>go</code> directive in <code>go.mod</code>:</p>
42+
<ul>
43+
<li>Go &lt; 1.18: Both client and server configs without MinVersion are flagged</li>
44+
<li>Go 1.18-1.21: Only server configs without MinVersion are flagged</li>
45+
<li>Go &gt;= 1.22: No configs are flagged (both defaults are secure)</li>
46+
</ul>
2547

2648
</example>
2749
<references>
2850
<li>
2951
<a href="https://pkg.go.dev/crypto/tls#Config">tls.Config specification</a>
3052
</li>
53+
<li>
54+
<a href="https://tip.golang.org/doc/go1.18#tls10">Go 1.18 Release Notes - TLS 1.0 and 1.1 disabled by default client-side</a>
55+
</li>
56+
<li>
57+
<a href="https://tip.golang.org/doc/go1.22#minor_library_changes">Go 1.22 Release Notes - TLS 1.2 default for servers</a>
58+
</li>
3159
</references>
3260
</qhelp>

go/src/security/MissingMinVersionTLS/MissingMinVersionTLS.ql

Lines changed: 74 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* @name Missing MinVersion in tls.Config
33
* @id tob/go/missing-min-version-tls
4-
* @description This rule finds cases when you do not set the `tls.Config.MinVersion` explicitly for servers. By default version 1.0 is used, which is considered insecure. This rule does not mark explicitly set insecure versions
4+
* @description Finds uses of tls.Config where MinVersion is not set and the project's minimum Go version (from go.mod) indicates insecure defaults: Go < 1.18 for clients (default TLS 1.0) or Go < 1.22 for servers (default TLS 1.0). Does not mark explicitly set insecure versions.
55
* @kind problem
66
* @tags security
77
* @problem.severity error
@@ -11,6 +11,7 @@
1111
*/
1212

1313
import go
14+
import semmle.go.GoMod as GoMod
1415

1516
/**
1617
* Flow of a `tls.Config` to a write to the `MinVersion` field.
@@ -103,6 +104,69 @@ predicate configOrConfigPointer(Type t) {
103104
)
104105
}
105106

107+
/**
108+
* Holds if v is a Go version string for Go 1.x that is >= 1.18.
109+
* Matches: "1.18", "1.19", "1.20", ..., "1.99", "1.100", "1.18.0", etc.
110+
*/
111+
predicate goVersionAtLeast_1_18(string v) {
112+
v.regexpMatch("1\\.(1[89]|[2-9][0-9]|[1-9][0-9]{2,})(\\.\\d+)?")
113+
}
114+
115+
/**
116+
* Holds if v is a Go version string for Go 1.x that is >= 1.22.
117+
* Matches: "1.22", "1.23", ..., "1.99", "1.100", "1.22.0", etc.
118+
*/
119+
predicate goVersionAtLeast_1_22(string v) {
120+
v.regexpMatch("1\\.(2[2-9]|[3-9][0-9]|[1-9][0-9]{2,})(\\.\\d+)?")
121+
}
122+
123+
/**
124+
* Holds if the project may be built with a Go version where a server with
125+
* an unset MinVersion still defaults to TLS 1.0/1.1 (Go < 1.22).
126+
*
127+
* - If there is no go.mod: assume YES (be conservative).
128+
* - Otherwise: if ANY go.mod has a `go` directive < 1.22: YES.
129+
*/
130+
predicate projectSupportsOldTlsDefaultsForServers() {
131+
not exists(GoMod::GoModGoLine _) or
132+
exists(GoMod::GoModGoLine l |
133+
not goVersionAtLeast_1_22(l.getVersion())
134+
)
135+
}
136+
137+
/**
138+
* Holds if the project may be built with a Go version where a client with
139+
* an unset MinVersion still defaults to TLS 1.0/1.1 (Go < 1.18).
140+
*
141+
* - If there is no go.mod: assume YES (be conservative).
142+
* - Otherwise: if ANY go.mod has a `go` directive < 1.18: YES.
143+
*/
144+
predicate projectSupportsOldTlsDefaultsForClients() {
145+
not exists(GoMod::GoModGoLine _) or
146+
exists(GoMod::GoModGoLine l |
147+
not goVersionAtLeast_1_18(l.getVersion())
148+
)
149+
}
150+
151+
/**
152+
* Holds if the config is used as a client config (TLSClientConfig).
153+
*/
154+
predicate isClientConfig(Variable v, StructLit configStruct) {
155+
exists(KeyValueExpr kv |
156+
kv.getKey().(VariableName).getTarget().getName() = "TLSClientConfig" and
157+
(
158+
kv.getValue().getAChild*() = v.getARead().asExpr()
159+
or
160+
kv.getValue().getAChild*() = configStruct
161+
)
162+
)
163+
or
164+
exists(Type t |
165+
t.hasQualifiedName("net/http", "Client") and
166+
v.getType() = t.getPointerType*()
167+
)
168+
}
169+
106170
// v - a variable holding any structure which is or contains the tls.Config
107171
from StructLit configStruct, Variable v, DataFlow::Node source, DataFlow::Node sink
108172
where
@@ -112,21 +176,6 @@ where
112176
sink.asExpr() = v.getAReference() and
113177
source.asExpr() = configStruct
114178
)
115-
116-
// exclude if tls.Config is used as TLSClientConfig, as default for clients is TLS 1.2
117-
and not exists(KeyValueExpr kv |
118-
kv.getKey().(VariableName).getTarget().getName() = "TLSClientConfig" and
119-
(
120-
kv.getValue().getAChild*() = v.getARead().asExpr()
121-
or
122-
kv.getValue().getAChild*() = configStruct
123-
)
124-
)
125-
126-
and not exists(Type t |
127-
t.hasQualifiedName("net/http", "Client") and
128-
v.getType() = t.getPointerType*()
129-
)
130179

131180
// only explicitely defined, e.g., skip function arguments
132181
and (
@@ -149,4 +198,13 @@ where
149198
) else
150199
any()
151200

201+
// Version-aware filtering based on client vs server usage:
202+
// - For clients: only flag if Go < 1.18 (when client default is TLS 1.0)
203+
// - For servers: only flag if Go < 1.22 (when server default is TLS 1.0)
204+
and (
205+
(isClientConfig(v, configStruct) and projectSupportsOldTlsDefaultsForClients())
206+
or
207+
(not isClientConfig(v, configStruct) and projectSupportsOldTlsDefaultsForServers())
208+
)
209+
152210
select configStruct, "TLS.Config.MinVersion is never set for variable $@ ", v, v.getName()

go/test/query-tests/security/MissingMinVersionTLS/MissingMinVersionTLS.expected

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
| MissingMinVersionTLS.go:135:10:135:49 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:136:3:136:3 | c | c |
1111
| MissingMinVersionTLS.go:142:23:142:62 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:142:3:142:3 | c | c |
1212
| MissingMinVersionTLS.go:149:23:149:62 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:149:3:149:3 | c | c |
13+
| MissingMinVersionTLS.go:159:21:161:5 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:157:3:157:8 | client | client |
1314
| MissingMinVersionTLS.go:168:16:168:55 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:167:3:167:5 | srv | srv |
1415
| MissingMinVersionTLS.go:171:16:171:55 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:170:3:170:6 | srv2 | srv2 |
1516
| MissingMinVersionTLS.go:176:9:176:48 | struct literal | TLS.Config.MinVersion is never set for variable $@ | MissingMinVersionTLS.go:182:3:182:6 | srv3 | srv3 |

go/test/query-tests/security/MissingMinVersionTLS/MissingMinVersionTLS.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,8 @@ func main() {
151151
c.Init3()
152152
}
153153
}
154-
// OK: config used only for a client
154+
// BAD for Go < 1.18: config used for a client (clients default to TLS 1.0)
155+
// OK for Go >= 1.18: clients default to TLS 1.2
155156
{
156157
client := &http.Client{
157158
Transport: &http.Transport{
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
11
module codeql-go-tests/query/MissingMinVersionTLS
22

3+
// Using Go 1.15 (< 1.18) to test the case where both clients and servers
4+
// default to TLS 1.0, so both should be flagged when MinVersion is not set
35
go 1.15

0 commit comments

Comments
 (0)