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
authz: Rbac engine audit logging #6225
Changes from 96 commits
8b9f59a
56ab1cd
0f3b6e7
6bff58b
9b3ab47
bbafb89
5dad66e
73b1390
27cfe85
c8751d3
441fb5c
2326e9f
abcb93f
3f57e69
b7863ee
48a875e
edf40f1
551ced9
1718c45
dad7293
7b9e96e
1f4c0c0
2eb4512
fd97b2c
ca8f66c
bf571d6
49f1ff3
066adbb
472d752
66fa61a
95a5253
67a71ff
1a4e978
1a5b03d
5490046
1c4b097
0747d62
fa5e894
b95b6f1
9ef033b
4896f7e
6fb2f46
760f946
d426edb
5aef84f
1b60ec1
6287772
90a1306
bc236d9
60338b3
c4e6067
4fec2a8
60088e3
50f2673
26e48ce
67613d3
468126a
4a9e3f8
16ea27e
89bc833
938b6b9
752d9e6
844216c
a46cd2e
f86c660
cab68ba
21a3788
5e5f9e9
724e066
89aca52
1685b62
2f51981
8e20f7f
c5798a9
d35a865
9e16b15
697ad75
2bead31
5ce64e9
1402f64
ac17f03
488c09d
9fa7542
5392f60
421acff
0960c8b
2cb5de1
361dcba
4dd9061
a81070c
50d1699
1637d73
4c2b1bb
218ba4b
1ec90ed
85696a7
d8777da
cfaf71d
dd6ff8d
4362f75
d582f2c
6908c86
ea8f50e
dd2f2a6
52f77b8
2f0a376
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 |
---|---|---|
@@ -0,0 +1,94 @@ | ||
/* | ||
* Copyright 2023 gRPC authors. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
package rbac | ||
|
||
import ( | ||
"encoding/json" | ||
"fmt" | ||
"strings" | ||
|
||
v1xdsudpatypepb "github.com/cncf/xds/go/udpa/type/v1" | ||
v3xdsxdstypepb "github.com/cncf/xds/go/xds/type/v3" | ||
v3rbacpb "github.com/envoyproxy/go-control-plane/envoy/config/rbac/v3" | ||
"google.golang.org/grpc/authz/audit" | ||
"google.golang.org/protobuf/types/known/anypb" | ||
"google.golang.org/protobuf/types/known/structpb" | ||
) | ||
|
||
const udpaTypedStuctType = "type.googleapis.com/udpa.type.v1.TypedStruct" | ||
const xdsTypedStuctType = "type.googleapis.com/xds.type.v3.TypedStruct" | ||
|
||
func buildLogger(loggerConfig *v3rbacpb.RBAC_AuditLoggingOptions_AuditLoggerConfig) (audit.Logger, error) { | ||
gtcooke94 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if loggerConfig.GetAuditLogger().GetTypedConfig() == nil { | ||
return nil, fmt.Errorf("AuditLogger TypedConfig cannot be nil") | ||
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. Does this get plumbed back to the user somehow? I traced it back a couple levels and it's never augmented or changed. I don't think this would be a useful error message for a user. Similar with all the other errors in this function / PR - please make sure that the failure modes are understood and that the errors will make sense where they appear. 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. It ends up bubbling up through the I think these errors make sense in the realm of the user getting them back from calling `authz.NewStatic(.), what do you think? 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. The authz policy is a string and you're giving them errors about nil pointers. That doesn't seem right. 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. That makes sense, I'll reword the errors to match that. Will comment back once I push that commit 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. Reworded some, I think the others left still make sense? PTAL 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. When the user calls 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 - the way the golang was already structured it all pushes into these protos in the end - the 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. Does that mean this error won't ever go back out to users of 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. In the So checks that happen in |
||
} | ||
customConfig, loggerName, err := getCustomConfig(loggerConfig.AuditLogger.TypedConfig) | ||
if err != nil { | ||
return nil, err | ||
} | ||
if loggerName == "" { | ||
return nil, fmt.Errorf("AuditLogger TypedConfig.TypeURL cannot be an empty string") | ||
} | ||
factory := audit.GetLoggerBuilder(loggerName) | ||
if factory == nil { | ||
if loggerConfig.IsOptional { | ||
return nil, nil | ||
} | ||
return nil, fmt.Errorf("no builder registered for %v", loggerName) | ||
} | ||
auditLoggerConfig, err := factory.ParseLoggerConfig(customConfig) | ||
if err != nil { | ||
return nil, fmt.Errorf("AuditLogger custom config could not be parsed: %v", err) | ||
} | ||
auditLogger := factory.Build(auditLoggerConfig) | ||
return auditLogger, nil | ||
} | ||
|
||
func getCustomConfig(config *anypb.Any) (json.RawMessage, string, error) { | ||
switch config.GetTypeUrl() { | ||
case udpaTypedStuctType: | ||
typedStruct := &v1xdsudpatypepb.TypedStruct{} | ||
if err := config.UnmarshalTo(typedStruct); err != nil { | ||
return nil, "", fmt.Errorf("failed to unmarshal resource: %v", err) | ||
} | ||
return convertCustomConfig(typedStruct.TypeUrl, typedStruct.Value) | ||
case xdsTypedStuctType: | ||
typedStruct := &v3xdsxdstypepb.TypedStruct{} | ||
if err := config.UnmarshalTo(typedStruct); err != nil { | ||
return nil, "", fmt.Errorf("failed to unmarshal resource: %v", err) | ||
} | ||
return convertCustomConfig(typedStruct.TypeUrl, typedStruct.Value) | ||
} | ||
return nil, "", fmt.Errorf("custom config not implemented for type [%v]", config.GetTypeUrl()) | ||
} | ||
|
||
func convertCustomConfig(typeURL string, s *structpb.Struct) (json.RawMessage, string, error) { | ||
// The gRPC policy name will be the "type name" part of the value of the | ||
// type_url field in the TypedStruct. We get this by using the part after | ||
// the last / character. Can assume a valid type_url from the control plane. | ||
urls := strings.Split(typeURL, "/") | ||
if len(urls) == 0 { | ||
return nil, "", fmt.Errorf("error converting custom lb policy %v for %v: typeURL must have a url-like format with the typeName being the value after the last /", typeURL, s) | ||
} | ||
name := urls[len(urls)-1] | ||
|
||
rawJSON, err := json.Marshal(s) | ||
rockspore marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if err != nil { | ||
return nil, "", fmt.Errorf("error converting custom lb policy %v for %v: %v", typeURL, s, err) | ||
} | ||
return rawJSON, name, 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.
for _, config := range options.AuditLoggers
?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.
Changed, I think this was a holdover from when this was structured differently.
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.
Looks like this is why?
Should this be
[]*auditLogger
then?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.
Oh yes, I remember now that is why I had done that - changed to
[]*auditLogger