Encode Join Attributes in Bot Certificates #49426
Conversation
| subject.ExtraNames = append(subject.ExtraNames, | ||
| pkix.AttributeTypeAndValue{ | ||
| Type: JoinAttributesASN1ExtensionOID, | ||
| Value: string(encoded), |
There was a problem hiding this comment.
Can/Should we store this as bytes instead?
There was a problem hiding this comment.
I think the internal encoding implementation is identical (https://github.com/golang/go/blob/master/src/encoding/asn1/marshal.go#L41-L63) but I believe the values are explicitly typed. That doesn't matter much for us since we'll support whichever datatype we use, but I think tools (openssl, etc) can decide to render plaintext or hex depending on type. That is to say, I think strings are probably better due to the type that will be embedded alongside the value itself.
Technically I think ASN.1 has some native struct encoding we could use, and the go encoder supports it via field tags. That might be technically be "more" "correct" but probably adds too much complexity to be worth it.
There was a problem hiding this comment.
Yeah - I have a feeling that encoding as a string may just be the best bet. I think my biggest fear here is overly inflating certificate sizes - I'm yet to really determine the point at which it would become problematic. If testing does indicate this, then maybe we'll need to switch to encoding as binary rather than protojson.
There was a problem hiding this comment.
After looking into this, it looks like storing Proto bytes instead actually causes some issues in the Subject DN due to the way Go's implementation of TLS handles parsing X509 certs. So we'll have to stick with string for now.
There was a problem hiding this comment.
Looks like this is rendering nicely with openssl x509, I see: 1.3.9999.2.21={"meta":{"join_method":"token"}}
strideynet
force-pushed
the
strideynet/persist-join-attributes-in-x509-cert
branch
3 times, most recently
from
d51e36a to
bbb99df
Compare
strideynet
added
no-changelog
|
Since this PR has sat around for a hot second, I'll manually retest this before merging. |
strideynet
force-pushed
the
strideynet/persist-join-attributes-in-x509-cert
branch
2 times, most recently
from
1c7805e to
58b98e4
Compare
strideynet
changed the base branch from
master
to
strideynet/protos-for-encoding-join-attrs
strideynet
force-pushed
the
strideynet/persist-join-attributes-in-x509-cert
branch
from
2d8677b to
b8c5014
Compare
strideynet
changed the title
github-actions
bot
added
kubernetes-access
size/lg
tctl
| subject.ExtraNames = append(subject.ExtraNames, | ||
| pkix.AttributeTypeAndValue{ | ||
| Type: JoinAttributesASN1ExtensionOID, | ||
| Value: string(encoded), |
There was a problem hiding this comment.
Looks like this is rendering nicely with openssl x509, I see: 1.3.9999.2.21={"meta":{"join_method":"token"}}
As per RFD191: #49133
Closes #49571
Depends on #50680
This PR encodes attributes produced by the join process into the certificates issued to Bots, so that these attributes can be used in authorization and templating decisions made when issuing Workload Identities.
In the v16/v17 backports, a disabled-by-default environment variable flag will be added. In a later release, we may enable this by default.