From 518ad2f53b4a9ea9c649bd1980c32f886bfe33ce Mon Sep 17 00:00:00 2001 From: Gregory Cooke Date: Wed, 8 May 2024 16:47:41 +0000 Subject: [PATCH 1/5] clarifications on IdentityCertificateOptions --- security/advancedtls/advancedtls.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index 4becabc830de..e85098ac2913 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -153,18 +153,18 @@ func (o RootCertificateOptions) nonNilFieldCount() int { // IdentityCertificateOptions contains options to obtain identity certificates // for both the client and the server. -// At most one option could be set. +// At most one option should be set. Setting more than one option will result in undefined behavior. type IdentityCertificateOptions struct { // If Certificates is set, it will be used every time when needed to present - //identity certificates, without performing identity certificate reloading. + // identity certificates, without performing identity certificate reloading. Certificates []tls.Certificate // If GetIdentityCertificatesForClient is set, it will be invoked to obtain // identity certs for every new connection. - // This field MUST be set on client side. + // This field is only relevant when set on the client side. GetIdentityCertificatesForClient func(*tls.CertificateRequestInfo) (*tls.Certificate, error) // If GetIdentityCertificatesForServer is set, it will be invoked to obtain // identity certs for every new connection. - // This field MUST be set on server side. + // This field is only relevant when set on the server side. GetIdentityCertificatesForServer func(*tls.ClientHelloInfo) ([]*tls.Certificate, error) // If IdentityProvider is set, we will use the identity certs from the // Provider's KeyMaterial() call in the new connections. The Provider must From c759a1a2f7f66c2f4a7d8348042f51b174bb191b Mon Sep 17 00:00:00 2001 From: Gregory Cooke Date: Wed, 8 May 2024 16:49:15 +0000 Subject: [PATCH 2/5] RootCertificateOptions clarifications --- security/advancedtls/advancedtls.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index e85098ac2913..cbba89cec7b8 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -119,8 +119,8 @@ type GetRootCAsResults = RootCertificates // RootCertificateOptions contains options to obtain root trust certificates // for both the client and the server. -// At most one option could be set. If none of them are set, we -// use the system default trust certificates. +// At most one option should be set. If none of them are set, we +// use the system default trust certificates. Setting more than one will result in undefined behavior. type RootCertificateOptions struct { // If RootCertificates is set, it will be used every time when verifying // the peer certificates, without performing root certificate reloading. From 567175c89386eb24b4aa36ae7f0b63e85382dc01 Mon Sep 17 00:00:00 2001 From: Gregory Cooke Date: Wed, 8 May 2024 17:22:38 +0000 Subject: [PATCH 3/5] add documentation --- security/advancedtls/advancedtls.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index cbba89cec7b8..c482c063f220 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -16,9 +16,14 @@ * */ -// Package advancedtls is a utility library containing functions to construct -// credentials.TransportCredentials that can perform credential reloading and -// custom verification check. +// Package advancedtls is a library containing APIs for configuring grpc +// connetions with TLS. The APIs here give the user more customizable control to +// fit their security landscape, thus the "advanced" moniker. This package +// provides both interfaces and generally useful implementations of those +// interfaces, for example periodic credential reloading, support for +// certificate revocation lists, and customizable certificate verification +// behaviors. If the provided implementations do not fit a given use case, a +// custom implementation of the interface can be injected. package advancedtls import ( From 2e37a5656bd4052f818241d6c655db00698c2448 Mon Sep 17 00:00:00 2001 From: Gregory Cooke Date: Wed, 8 May 2024 17:24:43 +0000 Subject: [PATCH 4/5] . --- security/advancedtls/advancedtls.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index c482c063f220..b612edd4a58e 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -124,8 +124,9 @@ type GetRootCAsResults = RootCertificates // RootCertificateOptions contains options to obtain root trust certificates // for both the client and the server. -// At most one option should be set. If none of them are set, we -// use the system default trust certificates. Setting more than one will result in undefined behavior. +// At most one option should be set. If none of them are set, we use the system +// default trust certificates. Setting more than one option will result in +// undefined behavior. type RootCertificateOptions struct { // If RootCertificates is set, it will be used every time when verifying // the peer certificates, without performing root certificate reloading. From d7aff4589caaf95418353c73fa740aec0ed810dc Mon Sep 17 00:00:00 2001 From: Gregory Cooke Date: Mon, 13 May 2024 17:42:25 +0000 Subject: [PATCH 5/5] address PR comments --- security/advancedtls/advancedtls.go | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/security/advancedtls/advancedtls.go b/security/advancedtls/advancedtls.go index b612edd4a58e..216cd97a2924 100644 --- a/security/advancedtls/advancedtls.go +++ b/security/advancedtls/advancedtls.go @@ -16,14 +16,15 @@ * */ -// Package advancedtls is a library containing APIs for configuring grpc -// connetions with TLS. The APIs here give the user more customizable control to -// fit their security landscape, thus the "advanced" moniker. This package -// provides both interfaces and generally useful implementations of those -// interfaces, for example periodic credential reloading, support for -// certificate revocation lists, and customizable certificate verification -// behaviors. If the provided implementations do not fit a given use case, a -// custom implementation of the interface can be injected. +// Package advancedtls provides gRPC transport credentials that allow easy +// configuration of advanced TLS features. The APIs here give the user more +// customizable control to fit their security landscape, thus the "advanced" +// moniker. This package provides both interfaces and generally useful +// implementations of those interfaces, for example periodic credential +// reloading, support for certificate revocation lists, and customizable +// certificate verification behaviors. If the provided implementations do not +// fit a given use case, a custom implementation of the interface can be +// injected. package advancedtls import ( @@ -124,8 +125,8 @@ type GetRootCAsResults = RootCertificates // RootCertificateOptions contains options to obtain root trust certificates // for both the client and the server. -// At most one option should be set. If none of them are set, we use the system -// default trust certificates. Setting more than one option will result in +// At most one field should be set. If none of them are set, we use the system +// default trust certificates. Setting more than one field will result in // undefined behavior. type RootCertificateOptions struct { // If RootCertificates is set, it will be used every time when verifying @@ -159,7 +160,7 @@ func (o RootCertificateOptions) nonNilFieldCount() int { // IdentityCertificateOptions contains options to obtain identity certificates // for both the client and the server. -// At most one option should be set. Setting more than one option will result in undefined behavior. +// At most one field should be set. Setting more than one field will result in undefined behavior. type IdentityCertificateOptions struct { // If Certificates is set, it will be used every time when needed to present // identity certificates, without performing identity certificate reloading.