feat: disable grpc reflection configuration #3550
Conversation
dosubot
bot
added
the
size:S
BandlSkyler
force-pushed
the
reflection-register-refactor
branch
from
d186ea6 to
3cf50f7
Compare
|
This change should not be made as it affects forward compatibility. It should be registered by default and optional not to register |
I see, but we should also ensure that users are not unknowingly enabling this feature and potentially exposing themselves to security risks without their consent. |
BandlSkyler
force-pushed
the
reflection-register-refactor
branch
from
3cf50f7 to
7995a04
Compare
transport/grpc/server.go
Outdated
| @@ -113,6 +113,13 @@ func StreamInterceptor(in ...grpc.StreamServerInterceptor) ServerOption { | |||
| } | |||
| } | |||
|
|
|||
| // DisuseReflection disuse reflection. | |||
| func DisuseReflection(enable bool) ServerOption { | |||
There was a problem hiding this comment.
Why do we still need to pass the enable parameter when the method is called Disuse Also, usually disable is used
transport/grpc/server.go
Outdated
| } | ||
|
|
||
| // NewServer creates a gRPC server by options. | ||
| // By default, it will register gRPC reflection and gRPC admin. |
There was a problem hiding this comment.
This annotation should not be added here
There was a problem hiding this comment.
I'm not sure where to add annotations properly, please guide me.
There was a problem hiding this comment.
Can be placed on the newly added method or in the document
There was a problem hiding this comment.
I think we should add an annotation in kratos-layout about that grpc admin and grpc reflection are registered.
BandlSkyler
force-pushed
the
reflection-register-refactor
branch
from
7995a04 to
84eb06b
Compare
transport/grpc/server.go
Outdated
| @@ -113,6 +113,13 @@ func StreamInterceptor(in ...grpc.StreamServerInterceptor) ServerOption { | |||
| } | |||
| } | |||
|
|
|||
| // DisuseReflection disuse reflection. | |||
| func DisuseReflection() ServerOption { | |||
shenqidebaozi
changed the title
shenqidebaozi
changed the title
BandlSkyler
force-pushed
the
reflection-register-refactor
branch
from
84eb06b to
b4fed5e
Compare
dosubot
bot
added
size:M
BandlSkyler
force-pushed
the
reflection-register-refactor
branch
from
b4fed5e to
90a5088
Compare
BandlSkyler
force-pushed
the
reflection-register-refactor
branch
from
90a5088 to
77ff1c1
Compare
Description (what this PR does / why we need it):
For security reasons, grpc reflection should not be registered directly by default