Repository navigation
Commit cd4ee03
authored
sec(iac/aws): reject the all-zeros OIDC thumbprint in the aws-target bundles (#1678)
* sec(iac/aws): reject the all-zeros OIDC thumbprint in aws-target CloudFormation
`OIDCThumbprint` defaulted to the all-zeros placeholder and was validated only
as 40 hexadecimal characters, which that placeholder satisfies.
Current AWS behaviour makes that value wrong in both directions. AWS verifies
the JWKS endpoint's TLS certificate against its own library of trusted root
CAs and consults the configured thumbprint only when that certificate does not
chain to one of them, when AWS cannot retrieve it, or when the endpoint
requires TLS 1.3. For both issuers this template documents
(login.microsoftonline.com and accounts.google.com) the thumbprint is
therefore never read at all. On the fallback path the all-zeros string is not
the SHA-1 of any certificate, so it matches nothing and every
AssumeRoleWithWebIdentity call fails. It does not bypass chain verification --
it nulls out the only verification that path has, while reading like a
deliberate configuration.
`ThumbprintList` is optional on AWS::IAM::OIDCProvider: when omitted, IAM
retrieves and uses the issuer's real top intermediate CA thumbprint, which is
what the IAM console does by default. That is now the template's default.
- Default `OIDCThumbprint` to empty and omit `ThumbprintList` via
`AWS::NoValue` when it is empty, so IAM retrieves the real thumbprint.
- Reject the placeholder in two independent layers: an `AllowedPattern`
requiring at least one non-zero hex digit, and an unconditional `Rules`
assertion naming the literal. Neither a regex simplification nor a
submission path that skips one layer can readmit it alone.
- Replace the parameter documentation, which described the placeholder as safe
for providers "where AWS auto-validates the chain".
- Add regression coverage that reads the `AllowedPattern`, `MaxLength` and
`Default` back out of the committed template and evaluates them the way
CloudFormation does. Both tests fail against the pre-fix template.
`ThumbprintList` updates with no interruption, so existing stacks switch
between the two forms without replacing the OIDC provider or changing its ARN.
Stacks currently holding the placeholder will fail their next update until the
parameter is cleared or set to a real thumbprint; that is deliberate, and such
a stack cannot be authenticating on the fallback path today in any case.
Closes #1615
* sec(iac/aws): align aws-target Terraform thumbprint guard with AWS behaviour
The `thumbprint_list` guard rejected the all-zeros placeholder for issuers
other than `login.microsoftonline.com` and `accounts.google.com`, justified in
the code as "the all-zeros value bypasses the CA-chain check entirely". That
justification does not match how AWS handles thumbprints, and the CloudFormation
sibling now encodes the opposite rule, so the two would otherwise contradict
each other on the same defect.
AWS verifies the JWKS endpoint's TLS certificate against its own library of
trusted root CAs and falls back to the configured thumbprints only when that
certificate does not chain to one of them, when AWS cannot retrieve it, or when
the endpoint requires TLS 1.3. So all-zeros bypasses nothing: on the primary
path AWS never reads it, and on the fallback path it is not the SHA-1 of any
certificate and matches nothing, failing role assumption outright. The issuer
allowlist was beside the point in both directions -- the thumbprint is unread
for every publicly-trusted issuer, not only those two, and the placeholder
breaks any private-CA issuer regardless of which one it is.
- Default `thumbprint_list` to `[]` and pass `null` when empty, so IAM
retrieves and uses the issuer's real top intermediate CA thumbprint.
- Reject the all-zeros entry unconditionally instead of per-issuer, and drop
the non-empty-list validation, since empty is now the correct default.
- Record the finding, the corrected reasoning and the upgrade path in
`known_issues/13_iac_aws_target.md`, and mark the superseded entry.
Verified with `terraform validate` plus `terraform plan` over four inputs: the
all-zeros list is rejected even for `accounts.google.com` (previously allowed),
a short value is rejected, and both an empty list and a real thumbprint are
accepted.
Refs #1615
* docs(iac/aws): qualify the thumbprint fallback claim and tighten the guard test
Review follow-up to the two commits above, no behaviour change.
The parameter and variable documentation said the thumbprint is "never
consulted" for the two documented issuers. AWS also falls back to thumbprint
verification when it cannot retrieve the TLS certificate or when the endpoint
requires TLS 1.3, and neither of those is issuer-specific, so the value can be
read even for a publicly trusted issuer. Reworded to say it is read only when
one of the listed fallback conditions applies.
Also assert that the all-zeros placeholder appears exactly once in the
template, so it can live in the Rules assertion that rejects it and nowhere
else; a `Contains` check would have stayed green if it were reintroduced as a
default alongside the assertion.
Refs #16151 parent f6bb36a commit cd4ee03
5 files changed
Lines changed: 350 additions & 35 deletions
File tree
- iac/federation/aws-target
- cloudformation
- terraform
- internal/api
- known_issues
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
53 | 53 | | |
54 | 54 | | |
55 | 55 | | |
56 | | - | |
57 | | - | |
58 | | - | |
59 | | - | |
60 | | - | |
61 | | - | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
62 | 96 | | |
63 | 97 | | |
64 | 98 | | |
| |||
99 | 133 | | |
100 | 134 | | |
101 | 135 | | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
102 | 156 | | |
103 | 157 | | |
104 | 158 | | |
| |||
107 | 161 | | |
108 | 162 | | |
109 | 163 | | |
110 | | - | |
111 | | - | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
112 | 173 | | |
113 | 174 | | |
114 | 175 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
26 | | - | |
27 | | - | |
28 | | - | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
29 | 40 | | |
30 | 41 | | |
31 | 42 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
86 | 86 | | |
87 | 87 | | |
88 | 88 | | |
89 | | - | |
90 | | - | |
91 | | - | |
92 | | - | |
93 | | - | |
94 | | - | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
95 | 106 | | |
96 | 107 | | |
97 | | - | |
98 | | - | |
99 | | - | |
100 | | - | |
101 | | - | |
102 | | - | |
| 108 | + | |
103 | 109 | | |
104 | 110 | | |
105 | 111 | | |
| |||
108 | 114 | | |
109 | 115 | | |
110 | 116 | | |
111 | | - | |
112 | | - | |
113 | | - | |
114 | | - | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
115 | 128 | | |
116 | | - | |
117 | | - | |
118 | | - | |
119 | | - | |
120 | | - | |
121 | | - | |
122 | | - | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
123 | 133 | | |
124 | 134 | | |
125 | 135 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
8 | 8 | | |
9 | 9 | | |
10 | 10 | | |
| 11 | + | |
| 12 | + | |
11 | 13 | | |
12 | 14 | | |
13 | 15 | | |
14 | 16 | | |
15 | 17 | | |
16 | 18 | | |
17 | 19 | | |
| 20 | + | |
18 | 21 | | |
19 | 22 | | |
20 | 23 | | |
| |||
1499 | 1502 | | |
1500 | 1503 | | |
1501 | 1504 | | |
| 1505 | + | |
| 1506 | + | |
| 1507 | + | |
| 1508 | + | |
| 1509 | + | |
| 1510 | + | |
| 1511 | + | |
| 1512 | + | |
| 1513 | + | |
| 1514 | + | |
| 1515 | + | |
| 1516 | + | |
| 1517 | + | |
| 1518 | + | |
| 1519 | + | |
| 1520 | + | |
| 1521 | + | |
| 1522 | + | |
| 1523 | + | |
| 1524 | + | |
| 1525 | + | |
| 1526 | + | |
| 1527 | + | |
| 1528 | + | |
| 1529 | + | |
| 1530 | + | |
| 1531 | + | |
| 1532 | + | |
| 1533 | + | |
| 1534 | + | |
| 1535 | + | |
| 1536 | + | |
| 1537 | + | |
| 1538 | + | |
| 1539 | + | |
| 1540 | + | |
| 1541 | + | |
| 1542 | + | |
| 1543 | + | |
| 1544 | + | |
| 1545 | + | |
| 1546 | + | |
| 1547 | + | |
| 1548 | + | |
| 1549 | + | |
| 1550 | + | |
| 1551 | + | |
| 1552 | + | |
| 1553 | + | |
| 1554 | + | |
| 1555 | + | |
| 1556 | + | |
| 1557 | + | |
| 1558 | + | |
| 1559 | + | |
| 1560 | + | |
| 1561 | + | |
| 1562 | + | |
| 1563 | + | |
| 1564 | + | |
| 1565 | + | |
| 1566 | + | |
| 1567 | + | |
| 1568 | + | |
| 1569 | + | |
| 1570 | + | |
| 1571 | + | |
| 1572 | + | |
| 1573 | + | |
| 1574 | + | |
| 1575 | + | |
| 1576 | + | |
| 1577 | + | |
| 1578 | + | |
| 1579 | + | |
| 1580 | + | |
| 1581 | + | |
| 1582 | + | |
| 1583 | + | |
| 1584 | + | |
| 1585 | + | |
| 1586 | + | |
| 1587 | + | |
| 1588 | + | |
| 1589 | + | |
| 1590 | + | |
| 1591 | + | |
| 1592 | + | |
| 1593 | + | |
| 1594 | + | |
| 1595 | + | |
| 1596 | + | |
| 1597 | + | |
| 1598 | + | |
| 1599 | + | |
| 1600 | + | |
| 1601 | + | |
| 1602 | + | |
| 1603 | + | |
| 1604 | + | |
| 1605 | + | |
| 1606 | + | |
| 1607 | + | |
| 1608 | + | |
| 1609 | + | |
| 1610 | + | |
| 1611 | + | |
| 1612 | + | |
| 1613 | + | |
| 1614 | + | |
| 1615 | + | |
| 1616 | + | |
| 1617 | + | |
| 1618 | + | |
| 1619 | + | |
| 1620 | + | |
| 1621 | + | |
| 1622 | + | |
| 1623 | + | |
| 1624 | + | |
| 1625 | + | |
| 1626 | + | |
| 1627 | + | |
| 1628 | + | |
| 1629 | + | |
| 1630 | + | |
| 1631 | + | |
| 1632 | + | |
| 1633 | + | |
| 1634 | + | |
| 1635 | + | |
| 1636 | + | |
| 1637 | + | |
| 1638 | + | |
| 1639 | + | |
| 1640 | + | |
| 1641 | + | |
| 1642 | + | |
| 1643 | + | |
| 1644 | + | |
| 1645 | + | |
| 1646 | + | |
| 1647 | + | |
| 1648 | + | |
| 1649 | + | |
| 1650 | + | |
| 1651 | + | |
| 1652 | + | |
| 1653 | + | |
| 1654 | + | |
| 1655 | + | |
0 commit comments