-
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,12 +2,22 @@ package securitypolicy | |
|
|
||
| import ( | ||
| "context" | ||
| "crypto/sha256" | ||
| "encoding/base64" | ||
| "fmt" | ||
| "os" | ||
| "path/filepath" | ||
| "syscall" | ||
| "time" | ||
|
|
||
| "github.com/Microsoft/cosesign1go/pkg/cosesign1" | ||
| didx509resolver "github.com/Microsoft/didx509go/pkg/did-x509-resolver" | ||
| "github.com/Microsoft/hcsshim/internal/log" | ||
| "github.com/Microsoft/hcsshim/internal/protocol/guestrequest" | ||
| "github.com/Microsoft/hcsshim/internal/protocol/guestresource" | ||
| oci "github.com/opencontainers/runtime-spec/specs-go" | ||
| "github.com/pkg/errors" | ||
| "github.com/sirupsen/logrus" | ||
| ) | ||
|
|
||
| type createEnforcerFunc func(base64EncodedPolicy string, criMounts, criPrivilegedMounts []oci.Mount, maxErrorMessageLength int) (SecurityPolicyEnforcer, error) | ||
|
|
@@ -121,7 +131,7 @@ type SecurityPolicyEnforcer interface { | |
| EnforceGetPropertiesPolicy(ctx context.Context) error | ||
| EnforceDumpStacksPolicy(ctx context.Context) error | ||
| EnforceRuntimeLoggingPolicy(ctx context.Context) (err error) | ||
| LoadFragment(ctx context.Context, issuer string, feed string, code string) error | ||
| LoadFragment(ctx context.Context, issuer string, feed string, rego string) error | ||
| EnforceScratchMountPolicy(ctx context.Context, scratchPath string, encrypted bool) (err error) | ||
| EnforceScratchUnmountPolicy(ctx context.Context, scratchPath string) (err error) | ||
| GetUserInfo(spec *oci.Process, rootPath string) (IDName, []IDName, string, error) | ||
|
|
@@ -142,6 +152,69 @@ func (s stringSet) contains(item string) bool { | |
| return contains | ||
| } | ||
|
|
||
| // Fragment extends current security policy with additional constraints | ||
| // from the incoming fragment. Note that it is base64 encoded over the bridge/ | ||
| // | ||
| // There are three checking steps: | ||
| // 1 - Unpack the cose document and check it was actually signed with the cert | ||
| // chain inside its header | ||
| // 2 - Check that the issuer field did:x509 identifier is for that cert chain | ||
| // (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) { | ||
| log.G(ctx).WithField("fragment", fmt.Sprintf("%+v", fragment)).Debug("VerifyAndExtractFragment") | ||
|
|
||
| raw, err := base64.StdEncoding.DecodeString(fragment.Fragment) | ||
| if err != nil { | ||
| return "", "", "", fmt.Errorf("failed to decode fragment: %w", err) | ||
| } | ||
| blob := []byte(fragment.Fragment) | ||
| // keep a copy of the fragment, so we can manually figure out what went wrong | ||
| // will be removed eventually. Give it a unique name to avoid any potential | ||
| // race conditions. | ||
| sha := sha256.New() | ||
| 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) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any reason we're ignoring this error?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
|
|
||
| unpacked, err := cosesign1.UnpackAndValidateCOSE1CertChain(raw) | ||
| if err != nil { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, as it is just for debug purpose. |
||
| return "", "", "", fmt.Errorf("InjectFragment failed COSE validation: %w", err) | ||
| } | ||
|
|
||
| payloadString = string(unpacked.Payload[:]) | ||
| issuer = unpacked.Issuer | ||
| feed = unpacked.Feed | ||
| chainPem := unpacked.ChainPem | ||
|
|
||
| log.G(ctx).WithFields(logrus.Fields{ | ||
| "issuer": issuer, // eg the DID:x509:blah.... | ||
| "feed": feed, | ||
| "cty": unpacked.ContentType, | ||
| "chainPem": chainPem, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we want to log chainPem?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
| }).Debugf("unpacked COSE1 cert chain") | ||
|
|
||
| log.G(ctx).WithFields(logrus.Fields{ | ||
| "payload": payloadString, | ||
| }).Tracef("unpacked COSE1 payload") | ||
|
|
||
| if len(issuer) == 0 || len(feed) == 0 { // must both be present | ||
| return "", "", "", fmt.Errorf("either issuer and feed must both be provided in the COSE_Sign1 protected header") | ||
| } | ||
|
|
||
| // Resolve returns a did doc that we don't need | ||
| // we only care if there was an error or not | ||
| _, err = didx509resolver.Resolve(unpacked.ChainPem, issuer, true) | ||
| if err != nil { | ||
| log.G(ctx).Printf("Badly formed fragment - did resolver failed to match fragment did:x509 from chain with purported issuer %s, feed %s - err %s", issuer, feed, err.Error()) | ||
| return "", "", "", err | ||
| } | ||
|
|
||
| return issuer, feed, payloadString, nil | ||
| } | ||
|
|
||
| // CreateSecurityPolicyEnforcer returns an appropriate enforcer for input | ||
| // parameters. Returns an error if the requested `enforcer` implementation | ||
| // isn't registered. | ||
|
|
||
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.
Uh oh!
There was an error while loading. Please reload this page.
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.
@anmaxvl Actually, I have an upcoming PR that does move this along with some other common functions into a different file #2544 - (I haven't pushed all my changes into that PR yet)