Skip to content
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

TrustStore: Purge CacheOnly flag from network requests #3510

Merged
merged 1 commit into from
Dec 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions go/lib/infra/modules/trust/v2/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,14 @@ var (
)

// newTestCryptoProvider returns a new crypto provider for testing.
func newTestCryptoProvider(db DBRead, recurser Recurser, resolver Resolver, router Router,
alwaysCacheOnly bool) CryptoProvider {
func newTestCryptoProvider(db DBRead, recurser Recurser, resolver Resolver,
router Router) CryptoProvider {

return &cryptoProvider{
db: db,
recurser: recurser,
resolver: resolver,
router: router,
alwaysCacheOnly: alwaysCacheOnly,
db: db,
recurser: recurser,
resolver: resolver,
router: router,
}
}

Expand Down
17 changes: 2 additions & 15 deletions go/lib/infra/modules/trust/v2/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,16 +66,9 @@ func (h *chainReqHandler) Handle() *infra.HandlerResult {
return infra.MetricsErrInternal
}
sendAck := messenger.SendAckHelper(ctx, rw)
opts := infra.ChainOpts{
TrustStoreOpts: infra.TrustStoreOpts{
LocalOnly: chainReq.CacheOnly,
},
AllowInactiveTRC: true,
}
raw, err := h.provider.GetRawChain(ctx, chainReq.IA(), chainReq.Version,
opts, h.request.Peer)
infra.ChainOpts{AllowInactiveTRC: true}, h.request.Peer)
if err != nil {
// FIXME(roosd): We should send a negative response.
logger.Error("[TrustStore:chainReqHandler] Unable to retrieve chain", "err", err)
sendAck(proto.Ack_ErrCode_reject, AckNotFound)
return infra.MetricsErrTrustStore(err)
Expand Down Expand Up @@ -121,14 +114,8 @@ func (h *trcReqHandler) Handle() *infra.HandlerResult {
return infra.MetricsErrInternal
}
sendAck := messenger.SendAckHelper(ctx, rw)
opts := infra.TRCOpts{
TrustStoreOpts: infra.TrustStoreOpts{
LocalOnly: trcReq.CacheOnly,
},
AllowInactive: true,
}
raw, err := h.provider.GetRawTRC(ctx, trcReq.ISD, trcReq.Version,
opts, h.request.Peer)
infra.TRCOpts{AllowInactive: true}, h.request.Peer)
if err != nil {
logger.Error("[TrustStore:trcReqHandler] Unable to retrieve TRC", "err", err)
sendAck(proto.Ack_ErrCode_reject, AckNotFound)
Expand Down
10 changes: 2 additions & 8 deletions go/lib/infra/modules/trust/v2/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,6 @@ type cryptoProvider struct {
recurser Recurser
resolver Resolver
router Router
// alwaysCacheOnly forces the cryptoProvider to always send cache-only
// requests. This should be set in the CS.
alwaysCacheOnly bool
}

func (p *cryptoProvider) GetTRC(ctx context.Context, isd addr.ISD, version scrypto.Version,
Expand Down Expand Up @@ -153,12 +150,9 @@ func (p *cryptoProvider) fetchTRC(ctx context.Context, isd addr.ISD, version scr
if err := p.recurser.AllowRecursion(client); err != nil {
return decoded.TRC{}, err
}
// In case the server is provided, cache-only should be set.
cacheOnly := server != nil || p.alwaysCacheOnly
req := TRCReq{
ISD: isd,
Version: version,
CacheOnly: cacheOnly,
ISD: isd,
Version: version,
}
// Choose remote server, if not set.
if server == nil {
Expand Down
42 changes: 16 additions & 26 deletions go/lib/infra/modules/trust/v2/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ func TestCryptoProviderGetTRC(t *testing.T) {
Expect func(m *mocks, dec *decoded.TRC)
Opts infra.TRCOpts
ExpectedErr error
CacheOnly bool
}{
"TRC in database, allow inactive": {
Expect: func(m *mocks, dec *decoded.TRC) {
Expand Down Expand Up @@ -90,9 +89,8 @@ func TestCryptoProviderGetTRC(t *testing.T) {
)
m.Recurser.EXPECT().AllowRecursion(gomock.Any()).Return(nil)
req := trust.TRCReq{
ISD: dec.TRC.ISD,
Version: dec.TRC.Version,
CacheOnly: true,
ISD: dec.TRC.ISD,
Version: dec.TRC.Version,
}
m.Resolver.EXPECT().TRC(gomock.Any(), req, ip).Return(*dec, nil)
},
Expand Down Expand Up @@ -202,9 +200,8 @@ func TestCryptoProviderGetTRC(t *testing.T) {
m.Recurser.EXPECT().AllowRecursion(gomock.Any()).Return(nil)
m.Router.EXPECT().ChooseServer(gomock.Any(), dec.TRC.ISD).Return(ip, nil)
req := trust.TRCReq{
ISD: dec.TRC.ISD,
Version: dec.TRC.Version,
CacheOnly: false,
ISD: dec.TRC.ISD,
Version: dec.TRC.Version,
}
m.Resolver.EXPECT().TRC(gomock.Any(), req, ip).Return(decoded.TRC{}, internal)
},
Expand All @@ -218,9 +215,8 @@ func TestCryptoProviderGetTRC(t *testing.T) {
)
m.Recurser.EXPECT().AllowRecursion(gomock.Any()).Return(nil)
req := trust.TRCReq{
ISD: dec.TRC.ISD,
Version: dec.TRC.Version,
CacheOnly: true,
ISD: dec.TRC.ISD,
Version: dec.TRC.Version,
}
m.Resolver.EXPECT().TRC(gomock.Any(), req, ip).Return(decoded.TRC{}, internal)
},
Expand All @@ -242,7 +238,7 @@ func TestCryptoProviderGetTRC(t *testing.T) {
}
decoded := loadTRC(t, trc1v1)
test.Expect(&m, &decoded)
provider := trust.NewCryptoProvider(m.DB, m.Recurser, m.Resolver, m.Router, false)
provider := trust.NewCryptoProvider(m.DB, m.Recurser, m.Resolver, m.Router)
ptrc, err := provider.GetTRC(nil, trc1v1.ISD, trc1v1.Version, test.Opts)
if test.ExpectedErr != nil {
require.Error(t, err)
Expand All @@ -268,7 +264,6 @@ func TestCryptoProviderGetTRCLatest(t *testing.T) {
Expect func(m *mocks, dec *decoded.TRC) decoded.TRC
Opts infra.TRCOpts
ExpectedErr error
CacheOnly bool
}{
"TRC in database, allow inactive": {
Expect: func(m *mocks, dec *decoded.TRC) decoded.TRC {
Expand All @@ -288,9 +283,8 @@ func TestCryptoProviderGetTRCLatest(t *testing.T) {
ip := &net.IPAddr{IP: []byte{127, 0, 0, 1}}
m.Router.EXPECT().ChooseServer(gomock.Any(), dec.TRC.ISD).Return(ip, nil)
req := trust.TRCReq{
ISD: dec.TRC.ISD,
Version: scrypto.Version(scrypto.LatestVer),
CacheOnly: false,
ISD: dec.TRC.ISD,
Version: scrypto.Version(scrypto.LatestVer),
}
m.Resolver.EXPECT().TRC(gomock.Any(), req, ip).Return(*dec, nil)
return *dec
Expand Down Expand Up @@ -330,9 +324,8 @@ func TestCryptoProviderGetTRCLatest(t *testing.T) {
ip := &net.IPAddr{IP: []byte{127, 0, 0, 1}}
m.Router.EXPECT().ChooseServer(gomock.Any(), dec.TRC.ISD).Return(ip, nil)
req := trust.TRCReq{
ISD: dec.TRC.ISD,
Version: scrypto.Version(scrypto.LatestVer),
CacheOnly: false,
ISD: dec.TRC.ISD,
Version: scrypto.Version(scrypto.LatestVer),
}
m.Resolver.EXPECT().TRC(gomock.Any(), req, ip).Return(*dec, nil)
return decoded.TRC{}
Expand All @@ -354,9 +347,8 @@ func TestCryptoProviderGetTRCLatest(t *testing.T) {
ip := &net.IPAddr{IP: []byte{127, 0, 0, 1}}
m.Router.EXPECT().ChooseServer(gomock.Any(), dec.TRC.ISD).Return(ip, nil)
req := trust.TRCReq{
ISD: dec.TRC.ISD,
Version: scrypto.Version(scrypto.LatestVer),
CacheOnly: false,
ISD: dec.TRC.ISD,
Version: scrypto.Version(scrypto.LatestVer),
}
newer := decoded.TRC{TRC: &(*dec.TRC)}
newer.TRC.Version += 1
Expand All @@ -382,9 +374,8 @@ func TestCryptoProviderGetTRCLatest(t *testing.T) {
ip := &net.IPAddr{IP: []byte{127, 0, 0, 1}}
m.Router.EXPECT().ChooseServer(gomock.Any(), dec.TRC.ISD).Return(ip, nil)
req := trust.TRCReq{
ISD: dec.TRC.ISD,
Version: scrypto.Version(scrypto.LatestVer),
CacheOnly: false,
ISD: dec.TRC.ISD,
Version: scrypto.Version(scrypto.LatestVer),
}
newer := decoded.TRC{TRC: &(*dec.TRC)}
newer.TRC.Version += 1
Expand All @@ -410,8 +401,7 @@ func TestCryptoProviderGetTRCLatest(t *testing.T) {
}
decoded := loadTRC(t, trc1v1)
expected := test.Expect(&m, &decoded)
provider := trust.NewCryptoProvider(m.DB, m.Recurser, m.Resolver,
m.Router, test.CacheOnly)
provider := trust.NewCryptoProvider(m.DB, m.Recurser, m.Resolver, m.Router)
trcObj, err := provider.GetTRC(nil, trc1v1.ISD, scrypto.LatestVer, test.Opts)
assert.Equal(t, expected.TRC, trcObj)
if test.ExpectedErr != nil {
Expand Down
15 changes: 6 additions & 9 deletions go/lib/infra/modules/trust/v2/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,8 @@ func (r *resolver) Chain(ctx context.Context, req ChainReq,
return decoded.Chain{}, serrors.Wrap(ErrInvalidResponse, err)
}
w := resolveWrap{
resolver: r,
server: server,
cacheOnly: req.CacheOnly,
resolver: r,
server: server,
}
if err := r.inserter.InsertChain(ctx, dec, w.TRC); err != nil {
return decoded.Chain{}, serrors.WrapStr("unable to insert certificate chain", err,
Expand Down Expand Up @@ -218,9 +217,8 @@ func (w *prevWrap) TRC(_ context.Context, isd addr.ISD, version scrypto.Version)
// resolverWrap provides TRCs that are backed by the resolver. If a TRC is
// missing in the DB, network requests are allowed.
type resolveWrap struct {
resolver *resolver
server net.Addr
cacheOnly bool
resolver *resolver
server net.Addr
}

func (w resolveWrap) TRC(ctx context.Context, isd addr.ISD,
Expand All @@ -234,9 +232,8 @@ func (w resolveWrap) TRC(ctx context.Context, isd addr.ISD,
return nil, serrors.WrapStr("error querying DB for TRC", err)
}
req := TRCReq{
ISD: isd,
Version: version,
CacheOnly: w.cacheOnly,
ISD: isd,
Version: version,
}
decoded, err := w.resolver.TRC(ctx, req, w.server)
if err != nil {
Expand Down
10 changes: 4 additions & 6 deletions go/lib/infra/modules/trust/v2/rpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,8 @@ type RPC interface {

// TRCReq holds the values of a TRC request.
type TRCReq struct {
ISD addr.ISD
Version scrypto.Version
CacheOnly bool
ISD addr.ISD
Version scrypto.Version
}

func (r TRCReq) withVersion(version scrypto.Version) TRCReq {
Expand All @@ -46,7 +45,6 @@ func (r TRCReq) withVersion(version scrypto.Version) TRCReq {

// ChainReq holds the values of a certificate chain request.
type ChainReq struct {
IA addr.IA
Version scrypto.Version
CacheOnly bool
IA addr.IA
Version scrypto.Version
}