-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat: Add CEL kw.crypto.verifyCert() function #18
Merged
Merged
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,138 @@ | ||
| package library | ||
|
|
||
| import ( | ||
| "reflect" | ||
|
|
||
| "github.com/google/cel-go/cel" | ||
| "github.com/google/cel-go/common/types" | ||
| "github.com/google/cel-go/common/types/ref" | ||
| "github.com/google/cel-go/common/types/traits" | ||
| "github.com/kubewarden/policy-sdk-go/pkg/capabilities/crypto" | ||
| ) | ||
|
|
||
| // Crypto returns a cel.EnvOption to configure namespaced crypto host-callback | ||
| // Kubewarden functions. | ||
| // | ||
| // # Crypto.VerifyCert | ||
| // | ||
| // This CEL function accepts a certificate, a certificate chain, and an | ||
| // expiration date. | ||
| // It returns a bool on whether the provided CertificateVerificationRequest | ||
| // (containing a cert to be verified, a cert chain, and an expiration date) | ||
| // passes certificate verification. | ||
| // | ||
| // Accepts 3 arguments: | ||
| // - string, of PEM-encoded certificate to verify. | ||
| // - list of strings, of PEM-encoded certs, ordered by trust usage | ||
| // (intermediates first, root last). If empty, certificate is assumed trusted. | ||
| // - string in RFC 3339 time format, to check expiration against. | ||
| // If empty, certificate is assumed never expired. | ||
| // | ||
| // Returns a map(<string>) with 2 fields: | ||
| // - "trusted": <bool> informing if certificate passed verification or not | ||
| // - "reason": <string> with reason, in case "Trusted" is false | ||
| // | ||
| // Usage in CEL: | ||
| // | ||
| // crypto.verifyCert(<string>, list(<string>), <string>) -> map(<string>, value) | ||
| // | ||
| // Example: | ||
| // | ||
| // kw.crypto.verifyCert( | ||
| // '---BEGIN CERTIFICATE---foo---END CERTIFICATE---', | ||
| // [ | ||
| // '---BEGIN CERTIFICATE---bar---END CERTIFICATE---' | ||
| // ], | ||
| // '2030-08-15T16:23:42+00:00' | ||
| // )" | ||
| func Crypto() cel.EnvOption { | ||
| return cel.Lib(cryptoLib{}) | ||
| } | ||
|
|
||
| type cryptoLib struct{} | ||
|
|
||
| // LibraryName implements the SingletonLibrary interface method. | ||
| func (cryptoLib) LibraryName() string { | ||
| return "kw.crypto" | ||
| } | ||
|
|
||
| // CompileOptions implements the Library interface method. | ||
| func (cryptoLib) CompileOptions() []cel.EnvOption { | ||
| return []cel.EnvOption{ | ||
| // group every binding under a container to simplify usage | ||
| cel.Container("crypto"), | ||
|
|
||
| cel.Function("kw.crypto.verifyCert", | ||
| cel.Overload("kw_crypto_verify_cert", | ||
| []*cel.Type{ | ||
| cel.StringType, | ||
| cel.ListType(cel.StringType), | ||
| cel.StringType, | ||
| }, | ||
| cel.MapType(cel.StringType, cel.DynType), | ||
| cel.FunctionBinding(verifyCert), | ||
| ), | ||
| ), | ||
| } | ||
| } | ||
|
|
||
| // ProgramOptions implements the Library interface method. | ||
| func (cryptoLib) ProgramOptions() []cel.ProgramOption { | ||
| return []cel.ProgramOption{} | ||
| } | ||
|
|
||
| func verifyCert(args ...ref.Val) ref.Val { | ||
| cert, ok1 := args[0].Value().(string) | ||
| if !ok1 { | ||
| return types.MaybeNoSuchOverloadErr(args[0]) | ||
| } | ||
|
|
||
| certChain, ok2 := args[1].(traits.Lister) | ||
| if !ok2 { | ||
| return types.MaybeNoSuchOverloadErr(args[1]) | ||
| } | ||
|
|
||
| notAfter, ok3 := args[2].Value().(string) | ||
| if !ok3 { | ||
| return types.MaybeNoSuchOverloadErr(args[2]) | ||
| } | ||
|
|
||
| // convert all cert.Data from string to []rune | ||
| cryptoCert := crypto.Certificate{ | ||
| Encoding: crypto.Pem, | ||
| Data: []rune(cert), | ||
| } | ||
| certChainLength, ok := certChain.Size().(types.Int) | ||
| if !ok { | ||
| return types.NewErr("cannot convert certChain length to int") | ||
| } | ||
| cryptoCertChain := make([]crypto.Certificate, 0, certChainLength) | ||
| for i := types.Int(0); i < certChainLength; i++ { | ||
| certElem, err := certChain.Get(i).ConvertToNative(reflect.TypeOf("")) | ||
| if err != nil { | ||
| return types.NewErr("cannot convert certChain: %s", err) | ||
| } | ||
| certString, ok := certElem.(string) | ||
| if !ok { | ||
| return types.NewErr("cannot convert cert into string") | ||
| } | ||
|
|
||
| cryptoCertChain = append(cryptoCertChain, | ||
| crypto.Certificate{ | ||
| Encoding: crypto.Pem, | ||
| Data: []rune(certString), | ||
| }, | ||
| ) | ||
| } | ||
|
|
||
| response, err := crypto.VerifyCert(&host, cryptoCert, cryptoCertChain, notAfter) | ||
| if err != nil { | ||
| return types.NewErr("cannot verify certificate: %s", err) | ||
| } | ||
|
|
||
| return types.NewStringInterfaceMap(types.DefaultTypeAdapter, | ||
| map[string]any{ | ||
| "trusted": response.Trusted, | ||
| "reason": response.Reason, | ||
| }) | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,128 @@ | ||
| package library | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "reflect" | ||
| "testing" | ||
|
|
||
| "github.com/google/cel-go/cel" | ||
| "github.com/kubewarden/policy-sdk-go/pkg/capabilities" | ||
| "github.com/kubewarden/policy-sdk-go/pkg/capabilities/crypto" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| func TestCrypto(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| expression string | ||
| responseTrusted bool | ||
| responseReason string | ||
| expectedResult any | ||
| }{ | ||
| { | ||
| "kw.crypto.verifyCert", | ||
| "kw.crypto.verifyCert(" + | ||
| "'---BEGIN CERTIFICATE---foo---END CERTIFICATE---'," + | ||
| "[ '---BEGIN CERTIFICATE---bar---END CERTIFICATE---' ]," + | ||
| "'2030-08-15T16:23:42+00:00'" + | ||
| ")", | ||
| false, | ||
| "the certificate is expired", | ||
| map[string]any{ | ||
| "trusted": false, | ||
| "reason": "the certificate is expired", | ||
| }, | ||
| }, | ||
| { | ||
| "kw.crypto.verifyCert with empty CertChain", | ||
| "kw.crypto.verifyCert( " + | ||
| "'---BEGIN CERTIFICATE---foo2---END CERTIFICATE---'," + | ||
| "[]," + | ||
| "'0004-08-15T16:23:42+00:00'" + | ||
| ")", | ||
| true, // e.g: cert is past expiration date, yet is trusted (empty CertChain) | ||
| "", | ||
| map[string]any{ | ||
| "trusted": true, | ||
| "reason": "", | ||
| }, | ||
| }, | ||
| { | ||
| "kw.crypto.verifyCert return type", | ||
| "kw.crypto.verifyCert( " + | ||
| "'---BEGIN CERTIFICATE---foo2---END CERTIFICATE---'," + | ||
| "[]," + | ||
| "'0004-08-15T16:23:42+00:00'" + | ||
| ").trusted", | ||
| true, // e.g: cert is past expiration date, yet is trusted (empty CertChain) | ||
| "", | ||
| true, | ||
| }, | ||
| } | ||
| for _, test := range tests { | ||
| t.Run(test.name, func(t *testing.T) { | ||
| var err error | ||
| host.Client, err = capabilities.NewSuccessfulMockWapcClient(crypto.CertificateVerificationResponse{ | ||
| Trusted: test.responseTrusted, | ||
| Reason: test.responseReason, | ||
| }) | ||
| require.NoError(t, err) | ||
|
|
||
| env, err := cel.NewEnv( | ||
| Crypto(), | ||
| ) | ||
| require.NoError(t, err) | ||
|
|
||
| ast, issues := env.Compile(test.expression) | ||
| require.Empty(t, issues) | ||
|
|
||
| prog, err := env.Program(ast) | ||
| require.NoError(t, err) | ||
|
|
||
| val, _, err := prog.Eval(map[string]interface{}{}) | ||
| require.NoError(t, err) | ||
|
|
||
| result, err := val.ConvertToNative(reflect.TypeOf(test.expectedResult)) | ||
| require.NoError(t, err) | ||
|
|
||
| require.Equal(t, test.expectedResult, result) | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestCryptoHostFailure(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| expression string | ||
| }{ | ||
| { | ||
| "kw.crypto.verifyCert host failure", | ||
| "kw.crypto.verifyCert( " + | ||
| "'---BEGIN CERTIFICATE---foo3---END CERTIFICATE---'," + | ||
| "[ '---BEGIN CERTIFICATE---bar3---END CERTIFICATE---' ]," + | ||
| "'2030-08-15T16:23:42+00:00'" + | ||
| ")", | ||
| }, | ||
| } | ||
| for _, test := range tests { | ||
| t.Run(test.name, func(t *testing.T) { | ||
| var err error | ||
| host.Client = capabilities.NewFailingMockWapcClient(fmt.Errorf("hostcallback error")) | ||
|
|
||
| env, err := cel.NewEnv( | ||
| Crypto(), | ||
| ) | ||
| require.NoError(t, err) | ||
|
|
||
| ast, issues := env.Compile(test.expression) | ||
| require.Empty(t, issues) | ||
|
|
||
| prog, err := env.Program(ast) | ||
| require.NoError(t, err) | ||
|
|
||
| _, _, err = prog.Eval(map[string]interface{}{}) | ||
| require.Error(t, err) | ||
| require.Equal(t, "cannot verify certificate: hostcallback error", err.Error()) | ||
| }) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 are not using the request/response struct native types.
also, what about DER certificates?
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.
Since Der is a binary data format that would not be fit for a string, it seemed a good idea to not provide it.
If we don't need a type
Certificate{}with anEncodingfield for Der, we don't need the request types.Also, we couldn't use the Request type as-is, as the struct type
capabilities/crypto/Certificate{}expects a []rune (and by extension the struct typeCertificateVerificationRequest{}). CEL only knows about strings, bytes, and []int, and provides stringToBytes (not fit) and stringToInt64 (should be int8). We would need intermediate helper types.For the Response type, it could be done, but at that point I preferred to return a map than a CertificateVerificationResponse{} that needs to be documented. It may also seem simpler to CEL users (but I'm open to changing it).
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.
@fabriziosestito it's my fault, I told Victor this could have been dropped because it would not be possible to write binary data into the definition of a policy.
However, @fabriziosestito reminded me CEL policies can also fetch data from external sources, like a ConfigMap. Do you think this could be a way to make DER data appear into a policy? I suspect you would still have to take the DER data, encode it using base64 and add that into a Secrets/ConfigMap. The CEL policy would have to work in reverse order: base64 decode (is there a way to do that) -> DER -> our function.
Is that something that can realistically happen? Should we just focus on the most common use case right now (PEM) and expand to support DER later on?
I'm a bit conflicted, because if we were to add DER support later on, we would have to break our CEL API; which would be super painful...
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 provide 2 different CEL functions, one for Pem and one for Der. I could rename the current function.
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.
Opened #23.
I'm conflicted if to subtly change the current
kw.crypto.verifyCert()signature to make it more easier to add DER certs later on.