bloomberg / vault-auth-spire

vault-auth-spire is an authentication plugin for Hashicorp Vault which allows logging into Vault using a Spire provided SVID.
Apache License 2.0
41 stars 8 forks source link

SPIFFE ID extracted from SVID is not guaranteed to be for the workload #7

Closed dennisgove closed 4 years ago

dennisgove commented 4 years ago

Describe the bug The assumption in this line is that of all the URIs found in the SVID SAN blocks the first one will be for the workload. This is not necessarily true if the SVID contains multiple PEM blocks.

SVIDs will contain multiple PEM blocks if SPIRE service option upstream_bundle = true is set. Those additional PEM blocks contain the full chain of trust and while important for verification are not the appropriate SpiffeIDs to use when generating Vault policy IDs (discussed in #1). I'm unsure if SPIFFE guarantees the order of PEM blocks within an SVID (see https://github.com/spiffe/spiffe/issues/32 for discussion about the order of SPIFFE IDs in a certificate).

As such, this assumption that the first PEM block contains the workload's SpiffeID is wrong.

uris := []string{}
for _, svidCert := range svidCerts {
  for _, uri := range svidCert.URIs {
    logrus.Info("Found URI: " + uri.String())
    uris = append(uris, uri.String())
  }
}

...

Metadata: map[string]string{
  "spiffeId": uris[0],
}

To Reproduce Steps to reproduce the behavior:

  1. Start SPIRE Server with upstream_bundle = true option.
  2. Register a workload
  3. Export/view SVID for workload
  4. See that the SVID contains multiple PEM blocks.

Expected behavior The extracted SpiffeID from the SVID should always be for the workload and not for another entry in the trust chain.

dennisgove commented 4 years ago

According to SPIFFE_Workload_API the leaf certificate must come first if there is a list of intermediaries. This reflects and reiterates what is in the TLS RFC, so we are fine to assume the first certificate represents the workload itself.

// ASN.1 DER encoded certificate chain. MAY include intermediates, // the leaf certificate (or SVID itself) MUST come first.

Will use this ticket to ensure we document this expectation in the method(s) used to read PEM blocks and the importance of order in the parsed certificates.