-
Notifications
You must be signed in to change notification settings - Fork 275
Security Policy: Move fragment extraction #2542
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
57a463d to
37ea7f4
Compare
37ea7f4 to
4b55375
Compare
| sha.Write(blob) | ||
| timestamp := time.Now() | ||
| fragmentPath := fmt.Sprintf("fragment-%x-%d.blob", sha.Sum(nil), timestamp.UnixMilli()) | ||
| _ = os.WriteFile(filepath.Join(os.TempDir(), fragmentPath), blob, 0644) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason we're ignoring this error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just purely for internal debug purposes, so we don't want to throw any errors on failure to write this.
| _ = os.WriteFile(filepath.Join(os.TempDir(), fragmentPath), blob, 0644) | ||
|
|
||
| unpacked, err := cosesign1.UnpackAndValidateCOSE1CertChain(raw) | ||
| if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it expected that we always keep the fragment file in temp directory regardless of the validation result?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, as it is just for debug purpose.
| "issuer": issuer, // eg the DID:x509:blah.... | ||
| "feed": feed, | ||
| "cty": unpacked.ContentType, | ||
| "chainPem": chainPem, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to log chainPem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a CA cert pem which contains public certs - used for troubleshooting
| // (ie fingerprint of a non leaf cert and the subject matches the leaf cert) | ||
| // 3 - Check that this issuer/feed match the requirement of the user provided | ||
| // security policy (done in the regoby LoadFragment) | ||
| func ExtractAndVerifyFragment(ctx context.Context, fragment *guestresource.LCOWSecurityPolicyFragment) (issuer string, feed string, payloadString string, err error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we could also put this into its own file, e.g. pkg/securitypolicy/fragments.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point, it's just a helper function, so leaving it here seems appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move inject and load fragment into the securitypolicy pkg Signed-off-by: Mahati Chamarthy <[email protected]>
4b55375 to
27ea7d4
Compare
Fragment extraction and validation is a common operation across C-LCOW and C-WCOW. This PR moves that functionality into the SecurityPolicy package, so that gcs and gcs-sidecar can call directly into it.