diff --git a/Gopkg.lock b/Gopkg.lock index 422554699..62a5aea36 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -28,11 +28,17 @@ revision = "346938d642f2ec3594ed81d874461961cd0faa76" version = "v1.1.0" +[[projects]] + name = "github.com/gofrs/flock" + packages = ["."] + revision = "392e7fae8f1b0bdbd67dad7237d23f618feb6dbb" + version = "v0.7.1" + [[projects]] name = "github.com/golang/protobuf" packages = ["proto"] - revision = "b4deda0973fb4c70b50d226b1af49f3da59f5265" - version = "v1.1.0" + revision = "c823c79ea1570fb5ff454033735a8e68575d1d0f" + version = "v1.3.0" [[projects]] name = "github.com/google/go-cmp" @@ -46,12 +52,6 @@ revision = "3af367b6b30c263d47e8895973edcca9a49cf029" version = "v0.2.0" -[[projects]] - name = "github.com/julienschmidt/httprouter" - packages = ["."] - revision = "8c199fb6259ffc1af525cc3ad52ee60ba8359669" - version = "v1.1" - [[projects]] name = "github.com/pelletier/go-toml" packages = ["."] @@ -89,12 +89,6 @@ revision = "12b6f73e6084dad08a7c6e575284b177ecafbc71" version = "v1.2.1" -[[projects]] - name = "github.com/theckman/go-flock" - packages = ["."] - revision = "dd99ba217509348e899885ee977b6e2acb18179c" - version = "v0.5.0" - [[projects]] branch = "master" name = "golang.org/x/crypto" @@ -135,6 +129,6 @@ [solve-meta] analyzer-name = "dep" analyzer-version = 1 - inputs-digest = "57ada0db71d59cf41009c5fed7eca533452a7ffa7af346db64b6244a63cbbab7" + inputs-digest = "10bd66ddd0a417265606dbeca1aa951d8bc4040b79b1bc00c4b9a107d8b2fdf7" solver-name = "gps-cdcl" solver-version = 1 diff --git a/Gopkg.toml b/Gopkg.toml index 725f74047..c21e72dbb 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -25,6 +25,8 @@ # unused-packages = true +ignored = ["google.golang.org/grpc"] + [prune] go-tests = true unused-packages = true diff --git a/README.md b/README.md index 94f37e6ec..a7393765b 100644 --- a/README.md +++ b/README.md @@ -161,6 +161,21 @@ that: possible for attackers to exploit it without intercepting the network path, for up to 7 days. +#### Testing productionization without a valid certificate + +It is possible to test an otherwise fully production configuration without +obtaining a certificate with the `CanSignHttpExchanges` extension. `amppkg` +still needs to perform OCSP verification, so the Issuer CA must be valid (i.e. no +self-signed certificates). e.g. You can use a certificate from [Let's Encrypt](https://letsencrypt.org/). + +Running `amppkg` with the `-invalidcert` flag will skip the check for +`CanSignHttpExchanges`. This flag is not necessary when using the +`-development` flag. + +Chrome can be configured to allow these invalid certificates with the +*Allow Signed HTTP Exchange certificates without extension* experiment: +chrome://flags/#allow-sxg-certs-without-extension + #### Redundancy If you need to load balance across multiple instances of `amppkg`, you'll want diff --git a/amppkg.example.toml b/amppkg.example.toml index b6127bdf9..2ac6f8872 100644 --- a/amppkg.example.toml +++ b/amppkg.example.toml @@ -52,10 +52,16 @@ CertFile = './pems/cert.pem' KeyFile = './pems/privkey.pem' # The path to a file where the OCSP response will be cached. The parent -# directory should exist, but the file need not. If this is a network-mounted -# file, it should support shared/exclusive locking. +# directory should exist, but the file need not. A dedicated lock file will be +# created in the same directory as this file, sharing the same name but with +# extension .lock appended. The filesystem must support shared and exclusive +# locking; consider this especially when utilizing network-mounted storage. OCSPCache = '/tmp/amppkg-ocsp' +# The list of request header names to be forwarded in a fetch request. +# Hop-by-hop headers, conditional request headers and Via cannot be included. +ForwardedRequestHeaders = [] + # This is a simple level of validation, to guard against accidental # misconfiguration of the reverse proxy that sits in front of the packager. # diff --git a/cmd/amppkg/main.go b/cmd/amppkg/main.go index 365aeea38..2208dc97c 100644 --- a/cmd/amppkg/main.go +++ b/cmd/amppkg/main.go @@ -24,22 +24,22 @@ import ( "log" "net/http" "net/url" - "path" "time" "github.com/WICG/webpackage/go/signedexchange" - "github.com/julienschmidt/httprouter" "github.com/pkg/errors" "github.com/ampproject/amppackager/packager/certcache" + "github.com/ampproject/amppackager/packager/mux" + "github.com/ampproject/amppackager/packager/rtv" "github.com/ampproject/amppackager/packager/signer" "github.com/ampproject/amppackager/packager/util" "github.com/ampproject/amppackager/packager/validitymap" - "github.com/ampproject/amppackager/packager/rtv" ) var flagConfig = flag.String("config", "amppkg.toml", "Path to the config toml file.") var flagDevelopment = flag.Bool("development", false, "True if this is a development server.") +var flagInvalidCert = flag.Bool("invalidcert", false, "True if invalid certificate intentionally used in production.") // Prints errors returned by pkg/errors with stack traces. func die(err interface{}) { log.Fatalf("%+v", err) } @@ -90,16 +90,25 @@ func main() { if certs == nil || len(certs) == 0 { die(fmt.Sprintf("no cert found in %s", config.CertFile)) } - if !*flagDevelopment && !util.CanSignHttpExchanges(certs[0]) { - die("cert is missing CanSignHttpExchanges extension") + if err := util.CanSignHttpExchanges(certs[0], time.Now()); err != nil { + if *flagDevelopment || *flagInvalidCert { + log.Println("WARNING:", err) + } else { + die(err) + } } - // TODO(twifkak): Verify that certs[0] covers all the signing domains in the config. key, err := util.ParsePrivateKey(keyPem) if err != nil { die(errors.Wrapf(err, "parsing %s", config.KeyFile)) } - // TODO(twifkak): Verify that key matches certs[0]. + + for _, urlSet := range config.URLSet { + domain := urlSet.Sign.Domain + if err := util.CertificateMatches(certs[0], key, domain); err != nil { + die(errors.Wrapf(err, "checking %s", config.CertFile)) + } + } validityMap, err := validitymap.New() if err != nil { @@ -125,20 +134,14 @@ func main() { } } - packager, err := signer.New(certs[0], key, config.URLSet, rtvCache, certCache.IsHealthy, - overrideBaseURL, /*requireHeaders=*/!*flagDevelopment) + signer, err := signer.New(certs[0], key, config.URLSet, rtvCache, certCache.IsHealthy, + overrideBaseURL, /*requireHeaders=*/!*flagDevelopment, config.ForwardedRequestHeaders) if err != nil { - die(errors.Wrap(err, "building packager")) + die(errors.Wrap(err, "building signer")) } // TODO(twifkak): Make log output configurable. - mux := httprouter.New() - mux.RedirectTrailingSlash = false - mux.RedirectFixedPath = false - mux.GET(util.ValidityMapPath, validityMap.ServeHTTP) - mux.GET("/priv/doc", packager.ServeHTTP) - mux.GET("/priv/doc/*signURL", packager.ServeHTTP) - mux.GET(path.Join(util.CertURLPrefix, ":certName"), certCache.ServeHTTP) + addr := "" if config.LocalOnly { addr = "localhost" @@ -148,7 +151,7 @@ func main() { Addr: addr, // Don't use DefaultServeMux, per // https://blog.cloudflare.com/exposing-go-on-the-internet/. - Handler: logIntercept{mux}, + Handler: logIntercept{mux.New(certCache, signer, validityMap)}, ReadTimeout: 10 * time.Second, ReadHeaderTimeout: 5 * time.Second, // If needing to stream the response, disable WriteTimeout and @@ -170,6 +173,9 @@ func main() { if *flagDevelopment { log.Println("WARNING: Running in development, using SXG key for TLS. This won't work in production.") log.Fatal(server.ListenAndServeTLS(config.CertFile, config.KeyFile)) + } else if *flagInvalidCert { + log.Println("WARNING: Running in production without valid signing certificate. Signed exchanges will not be valid.") + log.Fatal(server.ListenAndServe()) } else { log.Fatal(server.ListenAndServe()) } diff --git a/cmd/gateway_server/gateway/compile_proto.sh b/cmd/gateway_server/gateway/compile_proto.sh new file mode 100755 index 000000000..182d37eb2 --- /dev/null +++ b/cmd/gateway_server/gateway/compile_proto.sh @@ -0,0 +1,5 @@ +#!/bin/sh + +export PATH=$PATH:$HOME/go/bin/ + +protoc -I . ./gateway.proto --go_out=plugins=grpc:. --proto_path . diff --git a/cmd/gateway_server/gateway/gateway.pb.go b/cmd/gateway_server/gateway/gateway.pb.go new file mode 100644 index 000000000..0aef63651 --- /dev/null +++ b/cmd/gateway_server/gateway/gateway.pb.go @@ -0,0 +1,285 @@ +// Code generated by protoc-gen-go. DO NOT EDIT. +// source: gateway.proto + +package gateway + +import ( + context "context" + fmt "fmt" + proto "github.com/golang/protobuf/proto" + grpc "google.golang.org/grpc" + math "math" +) + +// Reference imports to suppress errors if they are not otherwise used. +var _ = proto.Marshal +var _ = fmt.Errorf +var _ = math.Inf + +// This is a compile-time assertion to ensure that this generated file +// is compatible with the proto package it is being compiled against. +// A compilation error at this line likely means your copy of the +// proto package needs to be updated. +const _ = proto.ProtoPackageIsVersion3 // please upgrade the proto package + +type SXGRequest struct { + // URL to fetch from publisher's internal server. ie. PublisherServer. + FetchUrl string `protobuf:"bytes,1,opt,name=fetch_url,json=fetchUrl,proto3" json:"fetch_url,omitempty"` + // URL to sign in sxg package. + SignUrl string `protobuf:"bytes,2,opt,name=sign_url,json=signUrl,proto3" json:"sign_url,omitempty"` + // ECC private key. + PrivateKey []byte `protobuf:"bytes,3,opt,name=private_key,json=privateKey,proto3" json:"private_key,omitempty"` + // X509 certificate which may or may not contain CanSignHTTPExchange + // extension. + PublicCert []byte `protobuf:"bytes,4,opt,name=public_cert,json=publicCert,proto3" json:"public_cert,omitempty"` + // Extra request headers to pass to Packager. singer.go->ServeHTTP. + PackagerServerRequestHeaders map[string]string `protobuf:"bytes,5,rep,name=packager_server_request_headers,json=packagerServerRequestHeaders,proto3" json:"packager_server_request_headers,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"` + XXX_NoUnkeyedLiteral struct{} `json:"-"` + XXX_unrecognized []byte `json:"-"` + XXX_sizecache int32 `json:"-"` +} + +func (m *SXGRequest) Reset() { *m = SXGRequest{} } +func (m *SXGRequest) String() string { return proto.CompactTextString(m) } +func (*SXGRequest) ProtoMessage() {} +func (*SXGRequest) Descriptor() ([]byte, []int) { + return fileDescriptor_f1a937782ebbded5, []int{0} +} + +func (m *SXGRequest) XXX_Unmarshal(b []byte) error { + return xxx_messageInfo_SXGRequest.Unmarshal(m, b) +} +func (m *SXGRequest) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { + return xxx_messageInfo_SXGRequest.Marshal(b, m, deterministic) +} +func (m *SXGRequest) XXX_Merge(src proto.Message) { + xxx_messageInfo_SXGRequest.Merge(m, src) +} +func (m *SXGRequest) XXX_Size() int { + return xxx_messageInfo_SXGRequest.Size(m) +} +func (m *SXGRequest) XXX_DiscardUnknown() { + xxx_messageInfo_SXGRequest.DiscardUnknown(m) +} + +var xxx_messageInfo_SXGRequest proto.InternalMessageInfo + +func (m *SXGRequest) GetFetchUrl() string { + if m != nil { + return m.FetchUrl + } + return "" +} + +func (m *SXGRequest) GetSignUrl() string { + if m != nil { + return m.SignUrl + } + return "" +} + +func (m *SXGRequest) GetPrivateKey() []byte { + if m != nil { + return m.PrivateKey + } + return nil +} + +func (m *SXGRequest) GetPublicCert() []byte { + if m != nil { + return m.PublicCert + } + return nil +} + +func (m *SXGRequest) GetPackagerServerRequestHeaders() map[string]string { + if m != nil { + return m.PackagerServerRequestHeaders + } + return nil +} + +type SXGResponse struct { + Sxg []byte `protobuf:"bytes,1,opt,name=sxg,proto3" json:"sxg,omitempty"` + Error bool `protobuf:"varint,2,opt,name=error,proto3" json:"error,omitempty"` + ErrorDescription string `protobuf:"bytes,3,opt,name=error_description,json=errorDescription,proto3" json:"error_description,omitempty"` + // application/cert-chain+cbor format certificate response. + Cbor []byte `protobuf:"bytes,4,opt,name=cbor,proto3" json:"cbor,omitempty"` + // HTTP headers returned by the packager. + HttpHeaders map[string]string `protobuf:"bytes,5,rep,name=http_headers,json=httpHeaders,proto3" json:"http_headers,omitempty" protobuf_key:"bytes,1,opt,name=key,proto3" protobuf_val:"bytes,2,opt,name=value,proto3"` + XXX_NoUnkeyedLiteral struct{} `json:"-"` + XXX_unrecognized []byte `json:"-"` + XXX_sizecache int32 `json:"-"` +} + +func (m *SXGResponse) Reset() { *m = SXGResponse{} } +func (m *SXGResponse) String() string { return proto.CompactTextString(m) } +func (*SXGResponse) ProtoMessage() {} +func (*SXGResponse) Descriptor() ([]byte, []int) { + return fileDescriptor_f1a937782ebbded5, []int{1} +} + +func (m *SXGResponse) XXX_Unmarshal(b []byte) error { + return xxx_messageInfo_SXGResponse.Unmarshal(m, b) +} +func (m *SXGResponse) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { + return xxx_messageInfo_SXGResponse.Marshal(b, m, deterministic) +} +func (m *SXGResponse) XXX_Merge(src proto.Message) { + xxx_messageInfo_SXGResponse.Merge(m, src) +} +func (m *SXGResponse) XXX_Size() int { + return xxx_messageInfo_SXGResponse.Size(m) +} +func (m *SXGResponse) XXX_DiscardUnknown() { + xxx_messageInfo_SXGResponse.DiscardUnknown(m) +} + +var xxx_messageInfo_SXGResponse proto.InternalMessageInfo + +func (m *SXGResponse) GetSxg() []byte { + if m != nil { + return m.Sxg + } + return nil +} + +func (m *SXGResponse) GetError() bool { + if m != nil { + return m.Error + } + return false +} + +func (m *SXGResponse) GetErrorDescription() string { + if m != nil { + return m.ErrorDescription + } + return "" +} + +func (m *SXGResponse) GetCbor() []byte { + if m != nil { + return m.Cbor + } + return nil +} + +func (m *SXGResponse) GetHttpHeaders() map[string]string { + if m != nil { + return m.HttpHeaders + } + return nil +} + +func init() { + proto.RegisterType((*SXGRequest)(nil), "gateway.SXGRequest") + proto.RegisterMapType((map[string]string)(nil), "gateway.SXGRequest.PackagerServerRequestHeadersEntry") + proto.RegisterType((*SXGResponse)(nil), "gateway.SXGResponse") + proto.RegisterMapType((map[string]string)(nil), "gateway.SXGResponse.HttpHeadersEntry") +} + +func init() { proto.RegisterFile("gateway.proto", fileDescriptor_f1a937782ebbded5) } + +var fileDescriptor_f1a937782ebbded5 = []byte{ + // 391 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x94, 0x92, 0x4f, 0xcb, 0xd3, 0x40, + 0x10, 0xc6, 0x4d, 0xd2, 0xda, 0x76, 0x52, 0xa5, 0xae, 0x3d, 0xc4, 0x2a, 0xb4, 0x16, 0x84, 0x82, + 0x90, 0x43, 0x45, 0x90, 0x1e, 0x3c, 0xf8, 0x87, 0x14, 0x14, 0x94, 0x14, 0xa1, 0xb7, 0xb0, 0x4d, + 0xc7, 0x24, 0x34, 0x24, 0xeb, 0xec, 0xa6, 0x1a, 0xf0, 0xe0, 0x97, 0xf4, 0xbb, 0x78, 0x94, 0xdd, + 0x44, 0x4a, 0x4b, 0xf1, 0xe5, 0xbd, 0xcd, 0x3c, 0x33, 0x4f, 0xf2, 0xcc, 0x2f, 0x81, 0x7b, 0x09, + 0x57, 0xf8, 0x9d, 0xd7, 0xbe, 0xa0, 0x52, 0x95, 0xac, 0xd7, 0xb6, 0xf3, 0xdf, 0x36, 0xc0, 0x66, + 0x1b, 0x84, 0xf8, 0xad, 0x42, 0xa9, 0xd8, 0x63, 0x18, 0x7c, 0x45, 0x15, 0xa7, 0x51, 0x45, 0xb9, + 0x67, 0xcd, 0xac, 0xc5, 0x20, 0xec, 0x1b, 0xe1, 0x0b, 0xe5, 0xec, 0x11, 0xf4, 0x65, 0x96, 0x14, + 0x66, 0x66, 0x9b, 0x59, 0x4f, 0xf7, 0x7a, 0x34, 0x05, 0x57, 0x50, 0x76, 0xe4, 0x0a, 0xa3, 0x03, + 0xd6, 0x9e, 0x33, 0xb3, 0x16, 0xc3, 0x10, 0x5a, 0xe9, 0x03, 0xd6, 0x66, 0xa1, 0xda, 0xe5, 0x59, + 0x1c, 0xc5, 0x48, 0xca, 0xeb, 0xb4, 0x0b, 0x46, 0x7a, 0x8b, 0xa4, 0xd8, 0x4f, 0x98, 0x0a, 0x1e, + 0x1f, 0x78, 0x82, 0x14, 0x49, 0xa4, 0x23, 0x52, 0x44, 0x4d, 0xa8, 0x28, 0x45, 0xbe, 0x47, 0x92, + 0x5e, 0x77, 0xe6, 0x2c, 0xdc, 0xe5, 0x4b, 0xff, 0xdf, 0x29, 0xa7, 0xdc, 0xfe, 0xe7, 0xd6, 0xba, + 0x31, 0xce, 0x56, 0x5d, 0x37, 0xbe, 0xf7, 0x85, 0xa2, 0x3a, 0x7c, 0x22, 0xfe, 0xb3, 0x32, 0xf9, + 0x04, 0x4f, 0x6f, 0x7c, 0x04, 0x1b, 0x81, 0xa3, 0x8f, 0x6b, 0xb0, 0xe8, 0x92, 0x8d, 0xa1, 0x7b, + 0xe4, 0x79, 0x85, 0x2d, 0x8e, 0xa6, 0x59, 0xd9, 0xaf, 0xac, 0xf9, 0x2f, 0x1b, 0x5c, 0x93, 0x4f, + 0x8a, 0xb2, 0x90, 0xa8, 0xbd, 0xf2, 0x47, 0x62, 0xbc, 0xc3, 0x50, 0x97, 0xda, 0x8b, 0x44, 0x25, + 0x19, 0x6f, 0x3f, 0x6c, 0x1a, 0xf6, 0x1c, 0x1e, 0x98, 0x22, 0xda, 0xa3, 0x8c, 0x29, 0x13, 0x2a, + 0x2b, 0x0b, 0x83, 0x73, 0x10, 0x8e, 0xcc, 0xe0, 0xdd, 0x49, 0x67, 0x0c, 0x3a, 0xf1, 0xae, 0xa4, + 0x96, 0xa6, 0xa9, 0xd9, 0x1a, 0x86, 0xa9, 0x52, 0xe2, 0x02, 0xda, 0xb3, 0x73, 0x68, 0x4d, 0x28, + 0x7f, 0xad, 0x94, 0x38, 0x83, 0xe4, 0xa6, 0x27, 0x65, 0xf2, 0x1a, 0x46, 0x97, 0x0b, 0xb7, 0x41, + 0xb0, 0xfc, 0x08, 0xf7, 0x83, 0xe6, 0xa5, 0x1a, 0x69, 0x16, 0x23, 0x5b, 0x81, 0x1b, 0x60, 0x81, + 0xc4, 0x15, 0x6e, 0xb6, 0x01, 0x7b, 0x78, 0xe5, 0x4b, 0x4e, 0xc6, 0xd7, 0x92, 0xce, 0xef, 0xbc, + 0x71, 0xfe, 0x58, 0xd6, 0xee, 0xae, 0xf9, 0x7b, 0x5f, 0xfc, 0x0d, 0x00, 0x00, 0xff, 0xff, 0x15, + 0x84, 0x4b, 0x32, 0xce, 0x02, 0x00, 0x00, +} + +// Reference imports to suppress errors if they are not otherwise used. +var _ context.Context +var _ grpc.ClientConn + +// This is a compile-time assertion to ensure that this generated file +// is compatible with the grpc package it is being compiled against. +const _ = grpc.SupportPackageIsVersion4 + +// GatewayServiceClient is the client API for GatewayService service. +// +// For semantics around ctx use and closing/ending streaming RPCs, please refer to https://godoc.org/google.golang.org/grpc#ClientConn.NewStream. +type GatewayServiceClient interface { + // Calls Signer.go->FetchUrl method. + GenerateSXG(ctx context.Context, in *SXGRequest, opts ...grpc.CallOption) (*SXGResponse, error) +} + +type gatewayServiceClient struct { + cc *grpc.ClientConn +} + +func NewGatewayServiceClient(cc *grpc.ClientConn) GatewayServiceClient { + return &gatewayServiceClient{cc} +} + +func (c *gatewayServiceClient) GenerateSXG(ctx context.Context, in *SXGRequest, opts ...grpc.CallOption) (*SXGResponse, error) { + out := new(SXGResponse) + err := c.cc.Invoke(ctx, "/gateway.GatewayService/GenerateSXG", in, out, opts...) + if err != nil { + return nil, err + } + return out, nil +} + +// GatewayServiceServer is the server API for GatewayService service. +type GatewayServiceServer interface { + // Calls Signer.go->FetchUrl method. + GenerateSXG(context.Context, *SXGRequest) (*SXGResponse, error) +} + +func RegisterGatewayServiceServer(s *grpc.Server, srv GatewayServiceServer) { + s.RegisterService(&_GatewayService_serviceDesc, srv) +} + +func _GatewayService_GenerateSXG_Handler(srv interface{}, ctx context.Context, dec func(interface{}) error, interceptor grpc.UnaryServerInterceptor) (interface{}, error) { + in := new(SXGRequest) + if err := dec(in); err != nil { + return nil, err + } + if interceptor == nil { + return srv.(GatewayServiceServer).GenerateSXG(ctx, in) + } + info := &grpc.UnaryServerInfo{ + Server: srv, + FullMethod: "/gateway.GatewayService/GenerateSXG", + } + handler := func(ctx context.Context, req interface{}) (interface{}, error) { + return srv.(GatewayServiceServer).GenerateSXG(ctx, req.(*SXGRequest)) + } + return interceptor(ctx, in, info, handler) +} + +var _GatewayService_serviceDesc = grpc.ServiceDesc{ + ServiceName: "gateway.GatewayService", + HandlerType: (*GatewayServiceServer)(nil), + Methods: []grpc.MethodDesc{ + { + MethodName: "GenerateSXG", + Handler: _GatewayService_GenerateSXG_Handler, + }, + }, + Streams: []grpc.StreamDesc{}, + Metadata: "gateway.proto", +} diff --git a/cmd/gateway_server/gateway/gateway.proto b/cmd/gateway_server/gateway/gateway.proto new file mode 100644 index 000000000..2893f3903 --- /dev/null +++ b/cmd/gateway_server/gateway/gateway.proto @@ -0,0 +1,40 @@ +// Gateway enables communication between C++ and Go environment. +// Using Gateway service c++ testing environment (client) can call go code +// (server). +// +// Gateway server uses public implementation of go gRPC library. + +syntax = "proto3"; + +package gateway; + +option cc_enable_arenas = true; + +message SXGRequest { + // URL to fetch from publisher's internal server. ie. PublisherServer. + string fetch_url = 1; + // URL to sign in sxg package. + string sign_url = 2; + // ECC private key. + bytes private_key = 3; + // X509 certificate which may or may not contain CanSignHTTPExchange + // extension. + bytes public_cert = 4; + // Extra request headers to pass to Packager. singer.go->ServeHTTP. + map packager_server_request_headers = 5; +} + +message SXGResponse { + bytes sxg = 1; + bool error = 2; + string error_description = 3; + // application/cert-chain+cbor format certificate response. + bytes cbor = 4; + // HTTP headers returned by the packager. + map http_headers = 5; +} + +service GatewayService { + // Calls Signer.go->FetchUrl method. + rpc GenerateSXG(SXGRequest) returns (SXGResponse) {} +} diff --git a/cmd/gateway_server/server.go b/cmd/gateway_server/server.go new file mode 100644 index 000000000..bf9c7355e --- /dev/null +++ b/cmd/gateway_server/server.go @@ -0,0 +1,213 @@ +// gRPC server to be used only for automated integration testing. + +package main + +import ( + "bytes" + "context" + "crypto" + "crypto/ecdsa" + "crypto/x509" + "flag" + "fmt" + "log" + "net" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "time" + + "github.com/WICG/webpackage/go/signedexchange" + "github.com/WICG/webpackage/go/signedexchange/certurl" + pb "github.com/ampproject/amppackager/cmd/gateway_server/gateway" + "github.com/ampproject/amppackager/packager/rtv" + "github.com/ampproject/amppackager/packager/signer" + "github.com/ampproject/amppackager/packager/util" + "golang.org/x/crypto/ocsp" + "google.golang.org/grpc" +) + +var ( + port = flag.Int("port", 9000, "Gateway server port") + publisherServerPort = flag.Int("publisher_server_port", 10000, + "Publisher server port.") +) + +type gatewayServer struct { + rtvCache *rtv.RTVCache +} + +func shouldPackage() bool { + return true +} + +func errorToSXGResponse(err error) *pb.SXGResponse { + response := &pb.SXGResponse{ + Error: true, + ErrorDescription: err.Error(), + } + return response +} + +func createOCSPResponse(cert *x509.Certificate, key crypto.Signer) ([]byte, error) { + thisUpdate := time.Now() + + // Construct args to ocsp.CreateResponse. + template := ocsp.Response{ + SerialNumber: cert.SerialNumber, + Status: ocsp.Good, + ThisUpdate: thisUpdate, + NextUpdate: thisUpdate.Add(time.Hour * 24 * 7), + IssuerHash: crypto.SHA256, + } + return ocsp.CreateResponse(cert /*issuer*/, cert /*responderCert*/, template, key) +} + +func (s *gatewayServer) GenerateSXG(ctx context.Context, request *pb.SXGRequest) (*pb.SXGResponse, error) { + log.Println("Handling request with fetchUrl =", request.FetchUrl, "; signUrl =", request.SignUrl) + + certs, err := signedexchange.ParseCertificates(request.PublicCert) + if err != nil { + return errorToSXGResponse(err), nil + } + + privateKey, err := util.ParsePrivateKey(request.PrivateKey) + if err != nil { + return errorToSXGResponse(err), nil + } + + signUrl, err := url.Parse(request.SignUrl) + if err != nil { + return errorToSXGResponse(err), nil + } + + var dotStarPattern = ".*" + signUrlPattern := util.URLPattern{ + Domain: signUrl.Host, + QueryRE: &dotStarPattern, + } + err = util.ValidateSignURLPattern(&signUrlPattern) + if err != nil { + return errorToSXGResponse(err), nil + } + + fetchUrlPattern := util.URLPattern{ + Scheme: []string{"http"}, + Domain: fmt.Sprintf("localhost:%d", *publisherServerPort), + ErrorOnStatefulHeaders: false, + QueryRE: &dotStarPattern, + SamePath: new(bool), + } + *fetchUrlPattern.SamePath = false + err = util.ValidateFetchURLPattern(&fetchUrlPattern) + if err != nil { + return errorToSXGResponse(err), nil + } + + urlSets := []util.URLSet{ + { + Sign: &signUrlPattern, + Fetch: &fetchUrlPattern, + }, + } + + packager, err := signer.New(certs[0], privateKey, urlSets, s.rtvCache, shouldPackage, signUrl, false, []string{}) + + if err != nil { + return errorToSXGResponse(err), nil + } + + if packager == nil { + return errorToSXGResponse(err), nil + } + + baseUrl, err := url.Parse(fmt.Sprintf("http://localhost:%d", *port)) + if err != nil { + return errorToSXGResponse(err), nil + } + q := baseUrl.Query() + q.Set("fetch", request.FetchUrl) + q.Set("sign", request.SignUrl) + baseUrl.RawQuery = q.Encode() + + httpreq, err := http.NewRequest("GET", baseUrl.String(), nil) + httpresp := httptest.NewRecorder() + packager.ServeHTTP(httpresp, httpreq) + + // TODO(amaltas): Capture error when signer returns unsigned document. + if httpresp.Code != 200 { + // TODO(amaltas): Add counter. + return &pb.SXGResponse{ + Error: true, + ErrorDescription: "Packager error.", + }, nil + } + + // Create cert-chain+cbor. + var ocspDer []byte + if len(certs) > 1 { + // Attach an OCSP response, signed with the second cert in the + // chain (assumed to be the issuer and using the same private + // key as the leaf cert). + var err error + ocspDer, err = createOCSPResponse(certs[1], privateKey.(*ecdsa.PrivateKey)) + if err != nil { + return errorToSXGResponse(err), nil + } + } else { + // Make up an invalid OCSP response. + ocspDer = []byte("ocsp") + } + var sctList []byte + certChain, err := certurl.NewCertChain(certs, ocspDer, sctList) + if err != nil { + return errorToSXGResponse(err), nil + } + + buf := &bytes.Buffer{} + err = certChain.Write(buf) + if err != nil { + return errorToSXGResponse(err), nil + } + + // Record http headers from the packager. + http_headers := map[string]string{} + for header_key, header_value := range httpresp.Header() { + // Ignores multiple header values. + http_headers[strings.ToLower(header_key)] = + string(header_value[0]) + } + + return &pb.SXGResponse{ + Sxg: httpresp.Body.Bytes(), + Cbor: buf.Bytes(), + HttpHeaders: http_headers}, nil +} + +func main() { + flag.Parse() + + if *port == -1 { + log.Fatalf("Set flag -port") + } + if *publisherServerPort == -1 { + log.Fatalf("Set flag -publisher-server-port") + } + + listener, err := net.Listen("tcp", fmt.Sprintf("127.0.0.1:%d", *port)) + if err != nil { + log.Fatalf("Failed to listen: %v", err) + } + + rtvCache, err := rtv.New() + if err != nil { + log.Fatalf("Error initializing RTVCache: %v", err) + } + + var opts []grpc.ServerOption + grpcServer := grpc.NewServer(opts...) + pb.RegisterGatewayServiceServer(grpcServer, &gatewayServer{rtvCache: rtvCache}) + log.Println("Starting server on port: ", *port) + grpcServer.Serve(listener) +} diff --git a/docs/cache_requirements.md b/docs/cache_requirements.md index 1af55be85..7213d9608 100644 --- a/docs/cache_requirements.md +++ b/docs/cache_requirements.md @@ -15,6 +15,9 @@ The Google AMP cache sets some requirements in addition to the ones set by the These include: * The signed `fallback URL` must equal the URL at which the SXG was delivered. + * The signature header must contain only: + * One parameterised identifier. + * Parameter values of type string, binary, or identifier. * The payload must be: * non-empty. * valid transformed AMP. The canonical definition of transformed AMP is the @@ -37,11 +40,15 @@ These include: `manifest-src`, `referrer`, and `upgrade-insecure-requests` may be omitted or have any value * all other directives are disallowed + * The signed `content-type` header must be present. Its media type must be + `text/html`. Its `charset` parameter, if present, must case-insensitively + equal `utf-8`. * The signed `link` header, if present, must look like [this](https://github.com/ampproject/amppackager/blob/e4bf0430ba152cfe82ccf063df92021dfc0f26a5/packager/signer/signer.go#L426) (the validation logic is currently very picky about its serialization); and have limits like [this](https://github.com/ampproject/amppackager/blob/e4bf0430ba152cfe82ccf063df92021dfc0f26a5/transformer/transformer.go#L177) (e.g. max 20 urls, rel=preload only, as=script|style only). URLs must be limited to `cdn.ampproject.org` and the allowlisted [font provider URLs](https://github.com/ampproject/amphtml/blob/b0ff92429923c86f3973009a84ff02f4f1868b4d/validator/validator-main.protoascii#L310). + * There must not be a signed `variant-key-04` or `variants-04` header. * The signature's duration (expiry minus date) must be >= 4 days. The above is an attempt at a complete list of SXG-related requirements, but it diff --git a/internal/url/url.go b/internal/url/url.go new file mode 100644 index 000000000..8f38239f3 --- /dev/null +++ b/internal/url/url.go @@ -0,0 +1,56 @@ +// Copyright 2019 Google LLC +// +// 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 +// +// https://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 url attempts to fix some of the issues related to Go's url.String(). +// This is a starting point for discrepancies between Go's implementation and +// the spec in https://tools.ietf.org/html/rfc3986. +// +// ABNF definitions: +// https://tools.ietf.org/html/rfc3986#appendix-A +// https://tools.ietf.org/html/rfc2234#section-6.1 +package url + +import ( + gourl "net/url" + "strings" +) + +// urlEncoder is used to encode the entire URL string and all its components - +// host, path, query, fragment. +// TODO(b/130250234): This is just a starting point for what may need to be +// encoded. Possibly encode more things. +var urlEncoder = strings.NewReplacer( + "<", "%3C", + ">", "%3E", + " ", "%20", +) + +// String post-processes Go's url.String() to encode characters that should be encoded. +// https://golang.org/issue/22907 +// https://golang.org/issue/30844 +// https://golang.org/issue/30922 +func String(input *gourl.URL) string { + // TODO(b/130234885): Handle relative URLs. + return urlEncoder.Replace(input.String()) +} + +// Splits the string on the separator (if it exists), retaining the separator. +// E.g. split("foo,bar", ",") returns "foo", ",bar" +func split(s string, sep string) (string, string) { + i := strings.Index(s, sep) + if i < 0 { + return s, "" + } + return s[:i], s[i:] +} diff --git a/internal/url/url_test.go b/internal/url/url_test.go new file mode 100644 index 000000000..51e48cfc3 --- /dev/null +++ b/internal/url/url_test.go @@ -0,0 +1,71 @@ +// Copyright 2019 Google LLC +// +// 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 +// +// https://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 url + +import ( + gourl "net/url" + "testing" +) + +func TestString(t *testing.T) { + tcs := []struct { + desc, in, out string + }{ + { + "<>", + "http://fo>o<.com/blm&wh>at#ev>ah", + "http://fo%3Eo%3C.com/bl%3Cah?zombo=co%3Em&wh%3Eat#ev%3Eah", + }, + { + "spaces encoded", + "https://foo.com/i haz spaces?q=i haz spaces", + "https://foo.com/i%20haz%20spaces?q=i%20haz%20spaces", + }, + { + // TODO(b/123017837): Go escapes only certain chars in fragments. + "fragment not entirely reescaped", // This is intrinsic Go URL behavior. + "https://example.com/amp.html#htmlURL=http%3A%2F%2Fbar.com%2Fbaz", + //does not produce: "https://example.com/amp.html#htmlURL=http%3A%2F%2Fbar.com%2Fbaz", + "https://example.com/amp.html#htmlURL=http://bar.com/baz", + }, + { + "fragment with space and quote reescaped", + "https://example.com/amp.html#fragment-\" ", + "https://example.com/amp.html#fragment-%22%20", + }, + { + "slashes in query, encoded or not, preserved", + "https://example.com/amp.html?URL=http://bar.com%2Fbaz", + "https://example.com/amp.html?URL=http://bar.com%2Fbaz", + }, + { + "slashes in path, encoded or not, preserved", + "https://example.com/foo%2Fbar/baz.html", + "https://example.com/foo%2Fbar/baz.html", + }, + } + for _, tt := range tcs { + t.Run(tt.desc, func(t *testing.T) { + u, err := gourl.Parse(tt.in) + if err != nil { + t.Fatalf("Error parsing %s", tt.in) + } + s := String(u) + if s != tt.out { + t.Errorf("got %q, want %q", s, tt.out) + } + }) + } +} diff --git a/packager/accept/accept.go b/packager/accept/accept.go index c82a7da02..77996be92 100644 --- a/packager/accept/accept.go +++ b/packager/accept/accept.go @@ -1,7 +1,9 @@ package accept import ( + "log" "mime" + "strings" "github.com/WICG/webpackage/go/signedexchange/version" "github.com/ampproject/amppackager/packager/util" @@ -18,23 +20,61 @@ const SxgContentType = "application/signed-exchange;v=" + AcceptedSxgVersion // signedexchange library. var SxgVersion = version.Version1b3 +// Tokenize a comma-separated string of accept patterns into a slice +func tokenize(accept string) []string { + var tokens []string + acceptLen := len(accept) + if acceptLen == 0 { + return tokens + } + + inQuotes := false + startIndex := 0 + for i := 0; i < acceptLen; i++ { + char := accept[i] + switch char { + case '"': + inQuotes = !inQuotes + case ',': + if !inQuotes { + tokens = append(tokens, util.TrimHeaderValue(accept[startIndex:i])) + startIndex = i + 1 + } + case '\\': + if !inQuotes { + log.Printf("unable to parse Accept header: %s", accept) + return []string{} + } + i++ + } + } + tokens = append(tokens, util.TrimHeaderValue(accept[startIndex:])) + return tokens +} + +// Determine whether a version specified by the accept header matches the +// version of signed exchange output by the packager +func hasMatchingSxgVersion(versions []string) bool { + for _, version := range versions { + if version == AcceptedSxgVersion { + return true + } + } + return false +} + // True if the given Accept header is one that the packager can satisfy. It // must contain application/signed-exchange;v=$V so that the packager knows // whether or not it can supply the correct version. "" and "*/*" are not // satisfiable, for this reason. func CanSatisfy(accept string) bool { - // There is an edge case on which this comma-splitting fails: - // Accept: application/signed-exchange;junk="some,thing";v=b2 - // However, in practice, browsers don't send media types with quoted - // commas in them: - // https://developer.mozilla.org/en-US/docs/Web/HTTP/Content_negotiation/List_of_default_Accept_values - // So we'll live with this deficiency for the sake of not forking - // mime.ParseMediaType. - types := util.Comma.Split(accept, -1) + types := tokenize(accept) for _, mediaRange := range types { mediatype, params, err := mime.ParseMediaType(mediaRange) - if err == nil && mediatype == "application/signed-exchange" && params["v"] == AcceptedSxgVersion { - return true + if err == nil && mediatype == "application/signed-exchange" { + if hasMatchingSxgVersion(strings.Split(params["v"], ",")) { + return true + } } } return false diff --git a/packager/accept/accept_test.go b/packager/accept/accept_test.go index a50c07b2e..296fba823 100644 --- a/packager/accept/accept_test.go +++ b/packager/accept/accept_test.go @@ -11,16 +11,17 @@ func TestCanSatisfy(t *testing.T) { assert.False(t, CanSatisfy("*/*")) assert.False(t, CanSatisfy("image/jpeg;v=b3")) assert.False(t, CanSatisfy(`application/signed-exchange;v=b2`)) - // This is a bug that will be triggered when a UA starts supporting multiple SXG versions: - assert.False(t, CanSatisfy(`application/signed-exchange;x="a,b";v="b3"`)) + assert.False(t, CanSatisfy(`application/signed-exchange;v="b1,b2"`)) + assert.False(t, CanSatisfy(`application/signed-exchange;x="y,application/signed-exchange;v=b3,z";v=b1`)) assert.True(t, CanSatisfy(`application/signed-exchange;v=b3`)) assert.True(t, CanSatisfy(`application/signed-exchange;v="b3"`)) + assert.True(t, CanSatisfy(`application/signed-exchange;v="b2,b3,b4"`)) assert.True(t, CanSatisfy(`application/signed-exchange;v=b3;q=0.8`)) + assert.True(t, CanSatisfy(`application/signed-exchange;v=b2;q=0.9,application/signed-exchange;v="b3,b4";q=0.8`)) assert.True(t, CanSatisfy(`application/signed-exchange;v=b1,application/signed-exchange;v=b3`)) assert.True(t, CanSatisfy(`application/signed-exchange;x="v=b1";v="b3"`)) assert.True(t, CanSatisfy("*/*, application/signed-exchange;v=b3")) assert.True(t, CanSatisfy("*/* \t,\t application/signed-exchange;v=b3")) - // This is the same bug, though one which won't occur in practice: - assert.True(t, CanSatisfy(`application/signed-exchange;x="y,application/signed-exchange;v=b3,z";v=b1`)) + assert.True(t, CanSatisfy(`application/signed-exchange;x="a,b";v="b3"`)) } diff --git a/packager/certcache/certcache.go b/packager/certcache/certcache.go index f07b93109..9f3850702 100644 --- a/packager/certcache/certcache.go +++ b/packager/certcache/certcache.go @@ -29,8 +29,8 @@ import ( "time" "github.com/WICG/webpackage/go/signedexchange/certurl" + "github.com/ampproject/amppackager/packager/mux" "github.com/ampproject/amppackager/packager/util" - "github.com/julienschmidt/httprouter" "github.com/pkg/errors" "github.com/pquerna/cachecontrol" "golang.org/x/crypto/ocsp" @@ -144,8 +144,9 @@ func (this *CertCache) ocspMidpoint(bytes []byte, issuer *x509.Certificate) (tim return resp.ThisUpdate.Add(resp.NextUpdate.Sub(resp.ThisUpdate) / 2), nil } -func (this *CertCache) ServeHTTP(resp http.ResponseWriter, req *http.Request, params httprouter.Params) { - if params.ByName("certName") == this.certName { +func (this *CertCache) ServeHTTP(resp http.ResponseWriter, req *http.Request) { + params := mux.Params(req) + if params["certName"] == this.certName { // https://tools.ietf.org/html/draft-yasskin-httpbis-origin-signed-exchanges-impl-00#section-3.3 // This content-type is not standard, but included to reduce // the chance that faulty user agents employ content sniffing. @@ -304,7 +305,7 @@ func (this *CertCache) shouldUpdateOCSP(bytes []byte) bool { return true } // TODO(twifkak): Use a logging framework with support for debug-only statements. - log.Println("No update necessary.") + log.Println("No OCSP update necessary.") return false } diff --git a/packager/certcache/certcache_test.go b/packager/certcache/certcache_test.go index d2323ff48..6d84cf988 100644 --- a/packager/certcache/certcache_test.go +++ b/packager/certcache/certcache_test.go @@ -29,9 +29,9 @@ import ( "github.com/WICG/webpackage/go/signedexchange" "github.com/WICG/webpackage/go/signedexchange/cbor" + "github.com/ampproject/amppackager/packager/mux" pkgt "github.com/ampproject/amppackager/packager/testing" "github.com/ampproject/amppackager/packager/util" - "github.com/julienschmidt/httprouter" "github.com/stretchr/testify/suite" "golang.org/x/crypto/ocsp" ) @@ -133,6 +133,10 @@ func (this *CertCacheSuite) TearDownTest() { } } +func (this *CertCacheSuite) mux() http.Handler { + return mux.New(this.handler, nil, nil) +} + func (this *CertCacheSuite) ocspServerCalled(f func()) bool { this.ocspServerWasCalled = false f() @@ -168,7 +172,7 @@ func (this *CertCacheSuite) DecodeCBOR(r io.Reader) map[string][]byte { } func (this *CertCacheSuite) TestServesCertificate() { - resp := pkgt.GetP(this.T(), this.handler, "/amppkg/cert/"+pkgt.CertName, httprouter.Params{httprouter.Param{"certName", pkgt.CertName}}) + resp := pkgt.Get(this.T(), this.mux(), "/amppkg/cert/"+pkgt.CertName) this.Assert().Equal(http.StatusOK, resp.StatusCode, "incorrect status: %#v", resp) this.Assert().Equal("nosniff", resp.Header.Get("X-Content-Type-Options")) cbor := this.DecodeCBOR(resp.Body) @@ -178,7 +182,7 @@ func (this *CertCacheSuite) TestServesCertificate() { } func (this *CertCacheSuite) TestServes404OnMissingCertificate() { - resp := pkgt.GetP(this.T(), this.handler, "/amppkg/cert/lalala", httprouter.Params{httprouter.Param{"certName", "lalala"}}) + resp := pkgt.Get(this.T(), this.mux(), "/amppkg/cert/lalala") this.Assert().Equal(http.StatusNotFound, resp.StatusCode, "incorrect status: %#v", resp) body, _ := ioutil.ReadAll(resp.Body) // Small enough not to fit a cert or key: @@ -187,7 +191,7 @@ func (this *CertCacheSuite) TestServes404OnMissingCertificate() { func (this *CertCacheSuite) TestOCSP() { // Verify it gets included in the cert-chain+cbor payload. - resp := pkgt.GetP(this.T(), this.handler, "/amppkg/cert/"+pkgt.CertName, httprouter.Params{httprouter.Param{"certName", pkgt.CertName}}) + resp := pkgt.Get(this.T(), this.mux(), "/amppkg/cert/"+pkgt.CertName) this.Assert().Equal(http.StatusOK, resp.StatusCode, "incorrect status: %#v", resp) // 302400 is 3.5 days. max-age is slightly less because of the time between fake OCSP generation and cert-chain response. // TODO(twifkak): Make this less flaky, by injecting a fake clock. @@ -222,7 +226,7 @@ func (this *CertCacheSuite) TestOCSPExpiry() { })) // Verify HTTP response expires immediately: - resp := pkgt.GetP(this.T(), this.handler, "/amppkg/cert/"+pkgt.CertName, httprouter.Params{httprouter.Param{"certName", pkgt.CertName}}) + resp := pkgt.Get(this.T(), this.mux(), "/amppkg/cert/"+pkgt.CertName) this.Assert().Equal("public, max-age=0", resp.Header.Get("Cache-Control")) // On update, verify network is called: diff --git a/packager/certcache/storage.go b/packager/certcache/storage.go index eb6aeb4d6..beba184eb 100644 --- a/packager/certcache/storage.go +++ b/packager/certcache/storage.go @@ -4,10 +4,12 @@ import ( "context" "io/ioutil" "log" + "os" + "runtime" "sync" "github.com/pkg/errors" - "github.com/theckman/go-flock" + "github.com/gofrs/flock" ) // This is an abstraction over a single file on a remote storage mechanism. It @@ -41,28 +43,60 @@ type LocalFile struct { path string } +// Check whether a file or directory exists. +func exists(path string) (bool, error) { + _, err := os.Stat(path) + if err == nil { + return true, nil + } + if os.IsNotExist(err) { + return false, nil + } + return true, err +} + func (this *LocalFile) Read(ctx context.Context, isExpired func([]byte) bool, update func([]byte) []byte) ([]byte, error) { - lock := flock.NewFlock(this.path) + // Use independent .lock file; necessary on Windows to avoid "The process cannot + // access the file because another process has locked a portion of the file." + lockPath := this.path + ".lock" + lock := flock.New(lockPath) locked, err := lock.TryRLock() if err != nil { - return nil, errors.Wrapf(err, "obtaining shared lock for %s", this.path) + return nil, errors.Wrapf(err, "obtaining shared lock for %s", lockPath) } if !locked { - return nil, errors.Errorf("unable to obtain shared lock for %s", this.path) + return nil, errors.Errorf("unable to obtain shared lock for %s", lockPath) } defer func() { if err = lock.Unlock(); err != nil { - log.Printf("Error unlocking %s; %+v", this.path, err) + log.Printf("Error unlocking %s; %+v", lockPath, err) } }() + + // Check whether OCSP cache file exists. + pathExists, err := exists(this.path) + if err != nil { + return nil, errors.Wrapf(err, "checking file exists %s", this.path) + } + + // Initialize empty contents. + var contents []byte + + // If cache file exists, read it and check freshness. Note that zero-length + // contents are considered "expired" by isExpired(). If an attempt is made + // to read the file before it exists on Windows, error "The system cannot + // find the file specified." is thrown. + if pathExists { + contents, err = ioutil.ReadFile(this.path) + if err != nil { + return nil, errors.Wrapf(err, "reading %s", this.path) + } + } + // At first glance, this looks like "broken" double-checked locking, as in // http://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html. // However, the difference is that a read lock is established first, so // that this shouldn't be looking at a partially-written file. - contents, err := ioutil.ReadFile(this.path) - if err != nil { - return nil, errors.Wrapf(err, "reading %s", this.path) - } select { case <-ctx.Done(): return nil, errors.Wrapf(ctx.Err(), "while reading %s", this.path) @@ -71,26 +105,36 @@ func (this *LocalFile) Read(ctx context.Context, isExpired func([]byte) bool, up return contents, nil } // Upgrade to a write-lock. It seems this may or may not be atomic, depending on the system. + // Windows does not handle a lock "upgrade", hence unlock before lock. + if runtime.GOOS == "windows" { + if err = lock.Unlock(); err != nil { + return nil, errors.Wrapf(err, "Error unlocking %s", lockPath) + } + } locked, err = lock.TryLock() if err != nil { - return nil, errors.Wrapf(err, "obtaining exclusive lock for %s", this.path) + return nil, errors.Wrapf(err, "obtaining exclusive lock for %s", lockPath) } if !locked { - return nil, errors.Errorf("unable to obtain exclusive lock for %s", this.path) + return nil, errors.Errorf("unable to obtain exclusive lock for %s", lockPath) } + // Reread the file while in write-lock, to make the // read-modify-write atomic, and thus reduce the chance of // multiple calls to update() in parallel. - contents, err := ioutil.ReadFile(this.path) - if err != nil { - return nil, errors.Wrapf(err, "rereading %s", this.path) - } - if !isExpired(contents) { - return contents, nil + if pathExists { + contents, err := ioutil.ReadFile(this.path) + if err != nil { + return nil, errors.Wrapf(err, "rereading %s", this.path) + } + if !isExpired(contents) { + return contents, nil + } } + contents = update(contents) // TODO(twifkak): Should I write to a tempfile in the same dir and move into place, instead? - if err = ioutil.WriteFile(this.path, contents, 0700); err != nil { + if err = ioutil.WriteFile(this.path, contents, 0600); err != nil { return nil, errors.Wrapf(err, "writing %s", this.path) } return contents, nil @@ -135,6 +179,7 @@ func (this *Chained) Read(ctx context.Context, isExpired func([]byte) bool, upda return this.first.Read(ctx, isExpired, func([]byte) []byte { contents, err := this.second.Read(ctx, isExpired, update) if err != nil { + log.Printf("%+v", err) return nil } return contents diff --git a/packager/mux/mux.go b/packager/mux/mux.go new file mode 100644 index 000000000..b7d8e8ddd --- /dev/null +++ b/packager/mux/mux.go @@ -0,0 +1,129 @@ +// Copyright 2019 Google LLC +// +// 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 +// +// https://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. + +// Implements the HTTP routing strategy used by the amppkg server. The main +// purpose for rolling our own is to be able to route the following types of +// URLs: +// /priv/doc/https://example.com/esc%61ped%2Furl.html +// and have the signer sign the URL exactly as it is encoded in the request, in +// order to meet docs/cache_requirements.md. +// +// The default http mux had some problems which led to me investigating 3rd +// party routers. (TODO(twifkak): Remember and document those problems.) +// +// I investigated two 3rd party routers capable of handling catch-all suffix +// parameters: +// https://github.com/julienschmidt/httprouter +// https://github.com/dimfeld/httptreemux +// Both libs unescape their catch-all parameters, making the above use-case +// impossible. The latter does so despite documenting support for unmodified +// URL escapings. This, plus a lack of feature needs, led me to believe that +// writing our own mux was the best approach. +package mux + +import ( + "context" + "net/http" + "net/url" + "strings" + + "github.com/ampproject/amppackager/packager/util" +) + +type mux struct { + certCache http.Handler + signer http.Handler + validityMap http.Handler +} + +// The main entry point. Use the return value for http.Server.Handler. +func New(certCache http.Handler, signer http.Handler, validityMap http.Handler) http.Handler { + return &mux{certCache, signer, validityMap} +} + +func tryTrimPrefix(s, prefix string) (string, bool) { + sLen := len(s) + trimmed := strings.TrimPrefix(s, prefix) + return trimmed, len(trimmed) != sLen +} + +var allowedMethods = map[string]bool{http.MethodGet: true, http.MethodHead: true} + +func (this *mux) ServeHTTP(resp http.ResponseWriter, req *http.Request) { + if !allowedMethods[req.Method] { + http.Error(resp, "405 method not allowed", http.StatusMethodNotAllowed) + return + } + + params := map[string]string{} + req = WithParams(req, params) + + // Use EscapedPath rather than RequestURI because the latter can take + // absolute-form, per https://tools.ietf.org/html/rfc7230#section-5.3. + // + // Use EscapedPath rather than RawPath to verify that the request URI + // is a valid encoding, per + // https://wicg.github.io/webpackage/draft-yasskin-http-origin-signed-responses.html#application-signed-exchange + // item 3. + path := req.URL.EscapedPath() + if suffix, ok := tryTrimPrefix(path, "/priv/doc"); ok { + if suffix == "" { + this.signer.ServeHTTP(resp, req) + } else if suffix[0] == '/' { + params["signURL"] = suffix[1:] + if req.URL.RawQuery != "" { + params["signURL"] += "?" + req.URL.RawQuery + } + this.signer.ServeHTTP(resp, req) + } else { + http.NotFound(resp, req) + } + } else if suffix, ok := tryTrimPrefix(path, util.CertURLPrefix+"/"); ok { + unescaped, err := url.PathUnescape(suffix) + if err != nil { + http.Error(resp, "400 bad request - bad URL encoding", http.StatusBadRequest) + } else { + params["certName"] = unescaped + this.certCache.ServeHTTP(resp, req) + } + } else if path == util.ValidityMapPath { + this.validityMap.ServeHTTP(resp, req) + } else { + http.NotFound(resp, req) + } +} + +type paramsKeyType struct{} + +var paramsKey = paramsKeyType{} + +// Gets the params from the request context, injected by the mux. Guaranteed to +// be non-nil. Call from the handlers. +func Params(req *http.Request) map[string]string { + params := req.Context().Value(paramsKey) + switch v := params.(type) { + case map[string]string: + return v + default: + // This should never happen, but just in case, let's not panic. + return map[string]string{} + } +} + +// Returns a copy of req annotated with the given params. (Params is stored by +// reference, and so may be mutated afterwards.) To be called only by this +// library itself or by tests. +func WithParams(req *http.Request, params map[string]string) *http.Request { + return req.WithContext(context.WithValue(req.Context(), paramsKey, params)) +} diff --git a/packager/signer/signer.go b/packager/signer/signer.go index 7f01acefd..caaa3e490 100644 --- a/packager/signer/signer.go +++ b/packager/signer/signer.go @@ -32,35 +32,19 @@ import ( "github.com/WICG/webpackage/go/signedexchange" "github.com/ampproject/amppackager/packager/accept" "github.com/ampproject/amppackager/packager/amp_cache_transform" + "github.com/ampproject/amppackager/packager/mux" "github.com/ampproject/amppackager/packager/rtv" "github.com/ampproject/amppackager/packager/util" "github.com/ampproject/amppackager/transformer" rpb "github.com/ampproject/amppackager/transformer/request" - "github.com/julienschmidt/httprouter" "github.com/pkg/errors" ) -// The Content-Security-Policy in use by the AMP Cache today. Specifying here -// provides protection for the publisher against bugs in the transformers, as -// these pages will now run on the publisher's origin. In the future, this -// value will likely be versioned along with the transforms. -var contentSecurityPolicy = "default-src * blob: data:; script-src blob: https://cdn.ampproject.org/rtv/ https://cdn.ampproject.org/v0.js https://cdn.ampproject.org/v0/ https://cdn.ampproject.org/viewer/; object-src 'none'; style-src 'unsafe-inline' https://cdn.ampproject.org/rtv/ https://cdn.materialdesignicons.com https://cloud.typography.com https://fast.fonts.net https://fonts.googleapis.com https://maxcdn.bootstrapcdn.com https://p.typekit.net https://pro.fontawesome.com https://use.fontawesome.com https://use.typekit.net; report-uri https://csp-collector.appspot.com/csp/amp" - // The user agent to send when issuing fetches. Should look like a mobile device. const userAgent = "Mozilla/5.0 (Linux; Android 6.0.1; Nexus 5X Build/MMB29P) " + "AppleWebKit/537.36 (KHTML, like Gecko) Chrome/41.0.2272.96 Mobile " + "Safari/537.36 (compatible; amppackager/0.0.0; +https://github.com/ampproject/amppackager)" -// Conditional request headers that ServeHTTP may receive and need to be sent with fetchURL. -// https://developer.mozilla.org/en-US/docs/Web/HTTP/Conditional_requests#Conditional_headers -var conditionalRequestHeaders = map[string]bool{ - "If-Match": true, - "If-None-Match": true, - "If-Modified-Since": true, - "If-Unmodified-Since": true, - "If-Range": true, -} - // Advised against, per // https://tools.ietf.org/html/draft-yasskin-httpbis-origin-signed-exchanges-impl-00#section-4.1 // and blocked in http://crrev.com/c/958945. @@ -119,34 +103,6 @@ var getTransformerRequest = func(r *rtv.RTVCache, s, u string) *rpb.Request { // in that it allows multiple slashes, as well as initial and terminal slashes. var protocol = regexp.MustCompile("^[!#$%&'*+\\-.^_`|~0-9a-zA-Z/]+$") -// The following hop-by-hop headers should be removed even when not specified -// in Connection, for backwards compatibility with downstream servers that were -// written against RFC 2616, and expect gateways to behave according to -// https://tools.ietf.org/html/rfc2616#section-13.5.1. (Note: "Trailers" is a -// typo there; should be "Trailer".) -// -// Connection header should also be removed per -// https://tools.ietf.org/html/rfc7230#section-6.1. -// -// Proxy-Connection should also be deleted, per -// https://github.com/WICG/webpackage/pull/339. -var legacyHeaders = []string{"Connection", "Keep-Alive", "Proxy-Authenticate", "Proxy-Authorization", "Proxy-Connection", "TE", "Trailer", "Transfer-Encoding", "Upgrade"} - -// Remove hop-by-hop headers, per https://tools.ietf.org/html/rfc7230#section-6.1. -func removeHopByHopHeaders(resp *http.Response) { - if connections, ok := resp.Header[http.CanonicalHeaderKey("Connection")]; ok { - for _, connection := range connections { - headerNames := util.Comma.Split(connection, -1) - for _, headerName := range headerNames { - resp.Header.Del(headerName) - } - } - } - for _, headerName := range legacyHeaders { - resp.Header.Del(headerName) - } -} - // Gets all values of the named header, joined on comma. func GetJoined(h http.Header, name string) string { if values, ok := h[http.CanonicalHeaderKey(name)]; ok { @@ -174,6 +130,7 @@ type Signer struct { shouldPackage func() bool overrideBaseURL *url.URL requireHeaders bool + forwardedRequestHeaders []string } func noRedirects(req *http.Request, via []*http.Request) error { @@ -182,14 +139,14 @@ func noRedirects(req *http.Request, via []*http.Request) error { func New(cert *x509.Certificate, key crypto.PrivateKey, urlSets []util.URLSet, rtvCache *rtv.RTVCache, shouldPackage func() bool, overrideBaseURL *url.URL, - requireHeaders bool) (*Signer, error) { + requireHeaders bool, forwardedRequestHeaders []string) (*Signer, error) { client := http.Client{ CheckRedirect: noRedirects, // TODO(twifkak): Load-test and see if default transport settings are okay. Timeout: 60 * time.Second, } - return &Signer{cert, key, &client, urlSets, rtvCache, shouldPackage, overrideBaseURL, requireHeaders}, nil + return &Signer{cert, key, &client, urlSets, rtvCache, shouldPackage, overrideBaseURL, requireHeaders, forwardedRequestHeaders}, nil } func (this *Signer) fetchURL(fetch *url.URL, serveHTTPReq *http.Request) (*http.Request, *http.Response, *util.HTTPError) { @@ -201,6 +158,14 @@ func (this *Signer) fetchURL(fetch *url.URL, serveHTTPReq *http.Request) (*http. return nil, nil, util.NewHTTPError(http.StatusInternalServerError, "Error building request: ", err) } req.Header.Set("User-Agent", userAgent) + // copy forwardedRequestHeaders + for _, header := range this.forwardedRequestHeaders { + if http.CanonicalHeaderKey(header) == "Host" { + req.Host = serveHTTPReq.Host + } else if value := GetJoined(serveHTTPReq.Header, header); value != "" { + req.Header.Set(header, value) + } + } // Golang's HTTP parser appears not to validate the protocol it parses // from the request line, so we do so here. if protocol.MatchString(serveHTTPReq.Proto) { @@ -212,7 +177,7 @@ func (this *Signer) fetchURL(fetch *url.URL, serveHTTPReq *http.Request) (*http. req.Header.Set("Via", via) } // Set conditional headers that were included in ServeHTTP's Request. - for header := range conditionalRequestHeaders { + for header := range util.ConditionalRequestHeaders { if value := GetJoined(serveHTTPReq.Header, header); value != "" { req.Header.Set(header, value) } @@ -221,7 +186,7 @@ func (this *Signer) fetchURL(fetch *url.URL, serveHTTPReq *http.Request) (*http. if err != nil { return nil, nil, util.NewHTTPError(http.StatusBadGateway, "Error fetching: ", err) } - removeHopByHopHeaders(resp) + util.RemoveHopByHopHeaders(resp.Header) return req, resp, nil } @@ -300,7 +265,7 @@ func (this *Signer) genCertURL(cert *x509.Certificate, signURL *url.URL) (*url.U return ret, nil } -func (this *Signer) ServeHTTP(resp http.ResponseWriter, req *http.Request, params httprouter.Params) { +func (this *Signer) ServeHTTP(resp http.ResponseWriter, req *http.Request) { resp.Header().Add("Vary", "Accept, AMP-Cache-Transform") if err := req.ParseForm(); err != nil { @@ -308,11 +273,9 @@ func (this *Signer) ServeHTTP(resp http.ResponseWriter, req *http.Request, param return } var fetch, sign string - if inPathSignURL := params.ByName("signURL"); inPathSignURL != "" { - sign = inPathSignURL[1:] // Strip leading "/" produced by httprouter. - if req.URL.RawQuery != "" { - sign += "?" + req.URL.RawQuery - } + params := mux.Params(req) + if inPathSignURL := params["signURL"]; inPathSignURL != "" { + sign = inPathSignURL } else { if len(req.Form["fetch"]) > 1 { util.NewHTTPError(http.StatusBadRequest, "More than 1 fetch param").LogAndRespond(resp) @@ -348,17 +311,16 @@ func (this *Signer) ServeHTTP(resp http.ResponseWriter, req *http.Request, param proxy(resp, fetchResp, nil) return } + var act string var transformVersion int64 if this.requireHeaders { header_value := GetJoined(req.Header, "AMP-Cache-Transform") - var act string act, transformVersion = amp_cache_transform.ShouldSendSXG(header_value) if act == "" { log.Println("Not packaging because AMP-Cache-Transform request header is invalid:", header_value) proxy(resp, fetchResp, nil) return } - resp.Header().Set("AMP-Cache-Transform", act) } else { var err error transformVersion, err = transformer.SelectVersion(nil) @@ -387,18 +349,11 @@ func (this *Signer) ServeHTTP(resp http.ResponseWriter, req *http.Request, param proxy(resp, fetchResp, nil) return } - fetchResp.Header.Del(header) } - // Mutate the fetched CSP to make sure it cannot break AMP pages. - fetchResp.Header.Set( - "Content-Security-Policy", - MutateFetchedContentSecurityPolicy( - fetchResp.Header.Get("Content-Security-Policy"))) - - fetchResp.Header.Del("Link") // Ensure there are no privacy-violating Link:rel=preload headers. - - if fetchResp.Header.Get("Variants") != "" || fetchResp.Header.Get("Variant-Key") != "" { + if fetchResp.Header.Get("Variants") != "" || fetchResp.Header.Get("Variant-Key") != "" || + // Include versioned headers per https://github.com/WICG/webpackage/pull/406. + fetchResp.Header.Get("Variants-04") != "" || fetchResp.Header.Get("Variant-Key-04") != "" { // Variants headers (https://tools.ietf.org/html/draft-ietf-httpbis-variants-04) are disallowed by AMP Cache. // We could delete the headers, but it's safest to assume they reflect the downstream server's intent. log.Println("Not packaging because response contains a Variants header.") @@ -406,7 +361,7 @@ func (this *Signer) ServeHTTP(resp http.ResponseWriter, req *http.Request, param return } - this.serveSignedExchange(resp, fetchResp, signURL, transformVersion) + this.serveSignedExchange(resp, fetchResp, signURL, act, transformVersion) case 304: // If fetchURL returns a 304, then also return a 304 with appropriate headers. @@ -449,9 +404,7 @@ func formatLinkHeader(preloads []*rpb.Metadata_Preload) (string, error) { } // serveSignedExchange does the actual work of transforming, packaging and signed and writing to the response. -func (this *Signer) serveSignedExchange(resp http.ResponseWriter, fetchResp *http.Response, signURL *url.URL, transformVersion int64) { - fetchResp.Header.Set("X-Content-Type-Options", "nosniff") - +func (this *Signer) serveSignedExchange(resp http.ResponseWriter, fetchResp *http.Response, signURL *url.URL, act string, transformVersion int64) { // After this, fetchResp.Body is consumed, and attempts to read or proxy it will result in an empty body. fetchBody, err := ioutil.ReadAll(io.LimitReader(fetchResp.Body, maxBodyLength)) if err != nil { @@ -468,17 +421,43 @@ func (this *Signer) serveSignedExchange(resp http.ResponseWriter, fetchResp *htt proxy(resp, fetchResp, fetchBody) return } - fetchResp.Header.Set("Content-Length", strconv.Itoa(len(transformed))) + + // Validate and format Link header. linkHeader, err := formatLinkHeader(metadata.Preloads) if err != nil { log.Println("Not packaging due to Link header error:", err) proxy(resp, fetchResp, fetchBody) return } + + // Begin mutations on original fetch response. From this point forward, do + // not fall-back to proxy(). + + // Remove stateful headers. + for header := range statefulResponseHeaders { + fetchResp.Header.Del(header) + } + + // Set Link header if formatting returned a valid value, otherwise, delete + // it to ensure there are no privacy-violating Link:rel=preload headers. if linkHeader != "" { fetchResp.Header.Set("Link", linkHeader) + } else { + fetchResp.Header.Del("Link") } + // Set content length. + fetchResp.Header.Set("Content-Length", strconv.Itoa(len(transformed))) + + // Set general security headers. + fetchResp.Header.Set("X-Content-Type-Options", "nosniff") + + // Mutate the fetched CSP to make sure it cannot break AMP pages. + fetchResp.Header.Set( + "Content-Security-Policy", + MutateFetchedContentSecurityPolicy( + fetchResp.Header.Get("Content-Security-Policy"))) + exchange := signedexchange.NewExchange( accept.SxgVersion, /*uri=*/signURL.String(), /*method=*/"GET", http.Header{}, fetchResp.StatusCode, fetchResp.Header, []byte(transformed)) @@ -518,6 +497,13 @@ func (this *Signer) serveSignedExchange(resp http.ResponseWriter, fetchResp *htt util.NewHTTPError(http.StatusInternalServerError, "Error serializing exchange: ", err).LogAndRespond(resp) } + // If requireHeaders was true when constructing signer, the + // AMP-Cache-Transform outer response header is required (and has already + // been validated) + if (act != "") { + resp.Header().Set("AMP-Cache-Transform", act) + } + // TODO(twifkak): Add Cache-Control: public with expiry to match when we think the AMP Cache // should fetch an update (half-way between signature date & expires). resp.Header().Set("Content-Type", accept.SxgContentType) diff --git a/packager/signer/signer_test.go b/packager/signer/signer_test.go index 42e5235ae..a00d15f97 100644 --- a/packager/signer/signer_test.go +++ b/packager/signer/signer_test.go @@ -28,12 +28,12 @@ import ( "github.com/WICG/webpackage/go/signedexchange" "github.com/ampproject/amppackager/packager/accept" + "github.com/ampproject/amppackager/packager/mux" "github.com/ampproject/amppackager/packager/rtv" pkgt "github.com/ampproject/amppackager/packager/testing" "github.com/ampproject/amppackager/packager/util" "github.com/ampproject/amppackager/transformer" rpb "github.com/ampproject/amppackager/transformer/request" - "github.com/julienschmidt/httprouter" "github.com/stretchr/testify/suite" ) @@ -61,25 +61,26 @@ type SignerSuite struct { lastRequest *http.Request } -func (this *SignerSuite) new(urlSets []util.URLSet) *Signer { - handler, err := New(pkgt.Certs[0], pkgt.Key, urlSets, &rtv.RTVCache{}, func() bool { return this.shouldPackage }, nil, true) +func (this *SignerSuite) new(urlSets []util.URLSet) http.Handler { + forwardedRequestHeaders := []string{"Host", "X-Foo"} + handler, err := New(pkgt.Certs[0], pkgt.Key, urlSets, &rtv.RTVCache{}, func() bool { return this.shouldPackage }, nil, true, forwardedRequestHeaders) this.Require().NoError(err) // Accept the self-signed certificate generated by the test server. handler.client = this.httpsClient - return handler + return mux.New(nil, handler, nil) } -func (this *SignerSuite) get(t *testing.T, handler pkgt.AlmostHandler, target string) *http.Response { - return this.getP(t, handler, target, httprouter.Params{}) +func (this *SignerSuite) get(t *testing.T, handler http.Handler, target string) *http.Response { + return pkgt.GetH(t, handler, target, http.Header{ + "AMP-Cache-Transform": {"google"}, "Accept": {"application/signed-exchange;v=" + accept.AcceptedSxgVersion}}) } -func (this *SignerSuite) getP(t *testing.T, handler pkgt.AlmostHandler, target string, params httprouter.Params) *http.Response { - return pkgt.GetHP(t, handler, target, http.Header{ - "AMP-Cache-Transform": {"google"}, "Accept": {"application/signed-exchange;v=" + accept.AcceptedSxgVersion}}, params) +func (this *SignerSuite) getFRH(t *testing.T, handler http.Handler, target string, host string, header http.Header) *http.Response { + return pkgt.GetHH(t, handler, target, host, header) } -func (this *SignerSuite) getB(t *testing.T, handler pkgt.AlmostHandler, target string, body string) *http.Response { - return pkgt.GetBH(t, handler, target, strings.NewReader(body), http.Header{ +func (this *SignerSuite) getB(t *testing.T, handler http.Handler, target string, body string) *http.Response { + return pkgt.GetBHH(t, handler, target, "", strings.NewReader(body), http.Header{ "AMP-Cache-Transform": {"google"}, "Accept": {"application/signed-exchange;v=" + accept.AcceptedSxgVersion}}) } @@ -101,6 +102,17 @@ func (this *SignerSuite) httpSignURL() string { return u.String() } +func (this *SignerSuite) certSubjectCN() string { + return pkgt.Certs[0].Subject.CommonName +} + +func (this *SignerSuite) httpSignURL_CertSubjectCN() string { + u, err := url.Parse(this.certSubjectCN()) + this.Require().NoError(err) + u.Scheme = "https" + return u.String() +} + func (this *SignerSuite) httpsURL() string { return this.tlsServer.URL } @@ -171,7 +183,6 @@ func (this *SignerSuite) TestSimple() { []string{"content-encoding", "content-length", "content-security-policy", "content-type", "date", "digest", "x-content-type-options"}, headerNames(exchange.ResponseHeaders)) this.Assert().Equal("text/html", exchange.ResponseHeaders.Get("Content-Type")) - this.Assert().Equal("text/html", exchange.ResponseHeaders.Get("Content-Type")) this.Assert().Equal("nosniff", exchange.ResponseHeaders.Get("X-Content-Type-Options")) this.Assert().Contains(exchange.SignatureHeaderValue, "validity-url=\""+this.httpSignURL()+"/amppkg/validity\"") this.Assert().Contains(exchange.SignatureHeaderValue, "integrity=\"digest/mi-sha256-03\"") @@ -186,20 +197,52 @@ func (this *SignerSuite) TestSimple() { this.Assert().Equal(append(payloadPrefix.Bytes(), transformedBody...), exchange.Payload) } -func (this *SignerSuite) TestParamsInPostBody() { +func (this *SignerSuite) TestFetchSignWithForwardedRequestHeaders() { urlSets := []util.URLSet{{ - Sign: &util.URLPattern{[]string{"https"}, "", this.httpHost(), stringPtr("/amp/.*"), []string{}, stringPtr(""), false, 2000, nil}, + Sign: &util.URLPattern{[]string{"https"}, "", this.certSubjectCN(), stringPtr("/amp/.*"), []string{}, stringPtr(""), false, 2000, nil}, Fetch: &util.URLPattern{[]string{"http"}, "", this.httpHost(), stringPtr("/amp/.*"), []string{}, stringPtr(""), false, 2000, boolPtr(true)}, }} - resp := this.getB(this.T(), this.new(urlSets), "/priv/doc", - "fetch="+url.QueryEscape(this.httpURL()+fakePath)+ - "&sign="+url.QueryEscape(this.httpSignURL()+fakePath)) - this.Assert().Equal(http.StatusOK, resp.StatusCode, "incorrect status: %#v", resp) + this.fakeHandler = func(resp http.ResponseWriter, req *http.Request) { + // Host and X-Foo headers are forwarded with forwardedRequestHeaders + this.Assert().Equal("www.example.com", req.Host) + this.Assert().Equal("foo", req.Header.Get("X-Foo")) + this.Assert().Equal("", req.Header.Get("X-Bar")) + this.lastRequest = req + resp.Header().Set("Content-Type", "text/html") + resp.Write(fakeBody) + } + // Request with ForwardedRequestHeaders + header := http.Header{"AMP-Cache-Transform": {"google"}, "Accept": {"application/signed-exchange;v=" + accept.AcceptedSxgVersion}, + "X-Foo": {"foo"}, "X-Bar": {"bar"}} + resp := this.getFRH(this.T(), this.new(urlSets), + "/priv/doc?fetch="+url.QueryEscape(this.httpURL()+fakePath)+"&sign="+url.QueryEscape(this.httpSignURL_CertSubjectCN()+fakePath), + "www.example.com", header) this.Assert().Equal(fakePath, this.lastRequest.URL.String()) - + this.Assert().Equal(userAgent, this.lastRequest.Header.Get("User-Agent")) + this.Assert().Equal("1.1 amppkg", this.lastRequest.Header.Get("Via")) + this.Assert().Equal(http.StatusOK, resp.StatusCode, "incorrect status: %#v", resp) + this.Assert().Equal(fmt.Sprintf(`google;v="%d"`, transformer.SupportedVersions[0].Max), resp.Header.Get("AMP-Cache-Transform")) + this.Assert().Equal("nosniff", resp.Header.Get("X-Content-Type-Options")) + this.Assert().Equal("Accept, AMP-Cache-Transform", resp.Header.Get("Vary")) exchange, err := signedexchange.ReadExchange(resp.Body) this.Require().NoError(err) - this.Assert().Equal(this.httpSignURL()+fakePath, exchange.RequestURI) + this.Assert().Equal(this.httpSignURL_CertSubjectCN()+fakePath, exchange.RequestURI) + this.Assert().Equal("GET", exchange.RequestMethod) + this.Assert().Equal(http.Header{}, exchange.RequestHeaders) + this.Assert().Equal(200, exchange.ResponseStatus) + this.Assert().Equal( + []string{"content-encoding", "content-length", "content-security-policy", "content-type", "date", "digest", "x-content-type-options"}, + headerNames(exchange.ResponseHeaders)) + this.Assert().Equal("text/html", exchange.ResponseHeaders.Get("Content-Type")) + this.Assert().Equal("text/html", exchange.ResponseHeaders.Get("Content-Type")) + this.Assert().Equal("nosniff", exchange.ResponseHeaders.Get("X-Content-Type-Options")) + this.Assert().Contains(exchange.SignatureHeaderValue, "validity-url=\""+this.httpSignURL_CertSubjectCN()+"/amppkg/validity\"") + this.Assert().Contains(exchange.SignatureHeaderValue, "integrity=\"digest/mi-sha256-03\"") + this.Assert().Contains(exchange.SignatureHeaderValue, "cert-url=\""+this.httpSignURL_CertSubjectCN()+"/amppkg/cert/"+pkgt.CertName+"\"") + this.Assert().Contains(exchange.SignatureHeaderValue, "cert-sha256=*"+pkgt.CertName+"=*") + var payloadPrefix bytes.Buffer + binary.Write(&payloadPrefix, binary.BigEndian, uint64(miRecordSize)) + this.Assert().Equal(append(payloadPrefix.Bytes(), transformedBody...), exchange.Payload) } func (this *SignerSuite) TestEscapeQueryParamsInFetchAndSign() { @@ -218,6 +261,15 @@ func (this *SignerSuite) TestEscapeQueryParamsInFetchAndSign() { this.Assert().Equal(this.httpSignURL()+fakePath+"?%3Chi%3E", exchange.RequestURI) } +func (this *SignerSuite) TestDisallowInvalidCharsSign() { + urlSets := []util.URLSet{{ + Sign: &util.URLPattern{[]string{"https"}, "", this.httpsHost(), stringPtr("/amp/.*"), []string{}, stringPtr(""), false, 2000, nil}, + }} + resp := this.get(this.T(), this.new(urlSets), + "/priv/doc?&sign="+url.QueryEscape(this.httpSignURL()+fakePath+"")) + this.Assert().Equal(http.StatusBadRequest, resp.StatusCode, "incorrect status: %#v", resp) +} + func (this *SignerSuite) TestNoFetchParam() { urlSets := []util.URLSet{{ Sign: &util.URLPattern{[]string{"https"}, "", this.httpsHost(), stringPtr("/amp/.*"), []string{}, stringPtr(""), false, 2000, nil}}} @@ -234,7 +286,7 @@ func (this *SignerSuite) TestSignAsPathParam() { urlSets := []util.URLSet{{ Sign: &util.URLPattern{[]string{"https"}, "", this.httpsHost(), stringPtr("/amp/.*"), []string{}, stringPtr(""), false, 2000, nil}, }} - resp := this.getP(this.T(), this.new(urlSets), `/priv/doc/`, httprouter.Params{httprouter.Param{"signURL", "/" + this.httpsURL() + fakePath}}) + resp := this.get(this.T(), this.new(urlSets), `/priv/doc/`+this.httpsURL()+fakePath) this.Assert().Equal(http.StatusOK, resp.StatusCode, "incorrect status: %#v", resp) exchange, err := signedexchange.ReadExchange(resp.Body) @@ -243,6 +295,33 @@ func (this *SignerSuite) TestSignAsPathParam() { this.Assert().Equal(this.httpsURL()+fakePath, exchange.RequestURI) } +func (this *SignerSuite) TestSignAsPathParamWithQuery() { + urlSets := []util.URLSet{{ + Sign: &util.URLPattern{[]string{"https"}, "", this.httpsHost(), stringPtr("/amp/.*"), []string{}, stringPtr(".*"), false, 2000, nil}, + }} + resp := this.get(this.T(), this.new(urlSets), `/priv/doc/`+this.httpsURL()+fakePath+"?amp=1") + this.Assert().Equal(http.StatusOK, resp.StatusCode, "incorrect status: %#v", resp) + + exchange, err := signedexchange.ReadExchange(resp.Body) + this.Require().NoError(err) + this.Assert().Equal(fakePath+"?amp=1", this.lastRequest.URL.String()) + this.Assert().Equal(this.httpsURL()+fakePath+"?amp=1", exchange.RequestURI) +} + +// Ensure that the server doesn't attempt to percent-decode the sign URL. +func (this *SignerSuite) TestSignAsPathParamWithUnusualPctEncoding() { + urlSets := []util.URLSet{{ + Sign: &util.URLPattern{[]string{"https"}, "", this.httpsHost(), stringPtr("/amp/.*"), []string{}, stringPtr(""), false, 2000, nil}, + }} + resp := this.get(this.T(), this.new(urlSets), `/priv/doc/`+this.httpsURL()+fakePath+`%2A`) + this.Assert().Equal(http.StatusOK, resp.StatusCode, "incorrect status: %#v", resp) + + exchange, err := signedexchange.ReadExchange(resp.Body) + this.Require().NoError(err) + this.Assert().Equal(fakePath+"%2A", this.lastRequest.URL.String()) + this.Assert().Equal(this.httpsURL()+fakePath+"%2A", exchange.RequestURI) +} + func (this *SignerSuite) TestPreservesContentType() { urlSets := []util.URLSet{{ Sign: &util.URLPattern{[]string{"https"}, "", this.httpsHost(), stringPtr("/amp/.*"), []string{}, stringPtr(""), false, 2000, nil}}} @@ -462,6 +541,20 @@ func (this *SignerSuite) TestProxyUnsignedIfMissingAMPCacheTransformHeader() { this.Assert().Equal(fakeBody, body, "incorrect body: %#v", resp) } +func (this *SignerSuite) TestProxyUnsignedIfInvalidAMPCacheTransformHeader() { + urlSets := []util.URLSet{{ + Sign: &util.URLPattern{[]string{"https"}, "", this.httpsHost(), stringPtr("/amp/.*"), []string{}, stringPtr(""), false, 2000, nil}, + }} + resp := pkgt.GetH(this.T(), this.new(urlSets), "/priv/doc?sign="+url.QueryEscape(this.httpsURL()+fakePath), http.Header{ + "Accept": {"application/signed-exchange;v=" + accept.AcceptedSxgVersion}, + "AMP-Cache-Transform": {"donotmatch"}, + }) + this.Assert().Equal(http.StatusOK, resp.StatusCode, "incorrect status: %#v", resp) + body, err := ioutil.ReadAll(resp.Body) + this.Require().NoError(err) + this.Assert().Equal(fakeBody, body, "incorrect body: %#v", resp) +} + func (this *SignerSuite) TestProxyUnsignedIfMissingAcceptHeader() { urlSets := []util.URLSet{{ Sign: &util.URLPattern{[]string{"https"}, "", this.httpsHost(), stringPtr("/amp/.*"), []string{}, stringPtr(""), false, 2000, nil}, @@ -540,6 +633,22 @@ func (this *SignerSuite) TestProxyUnsignedOnVariants() { this.Assert().Equal("text/html", resp.Header.Get("Content-Type")) } +func (this *SignerSuite) TestProxyUnsignedOnVariants04() { + urlSets := []util.URLSet{{ + Sign: &util.URLPattern{[]string{"https"}, "", this.httpsHost(), stringPtr("/amp/.*"), []string{}, stringPtr(""), true, 2000, nil}, + }} + this.fakeHandler = func(resp http.ResponseWriter, req *http.Request) { + resp.Header().Set("Content-Type", "text/html; charset=utf-8") + resp.Header().Set("Variants-04", "foo") + resp.Header().Set("Content-Type", "text/html") + resp.WriteHeader(200) + } + + resp := this.get(this.T(), this.new(urlSets), "/priv/doc?sign="+url.QueryEscape(this.httpsURL()+fakePath)) + this.Assert().Equal(200, resp.StatusCode) + this.Assert().Equal("foo", resp.Header.Get("Variants-04")) + this.Assert().Equal("text/html", resp.Header.Get("Content-Type")) +} func (this *SignerSuite) TestProxyUnsignedIfNotAMP() { urlSets := []util.URLSet{{ @@ -593,6 +702,49 @@ func (this *SignerSuite) TestProxyTransformError() { this.Assert().Equal(fakeBody, body, "incorrect body: %#v", resp) } +func (this *SignerSuite) TestProxyHeadersUnaltered() { + urlSets := []util.URLSet{{ + Sign: &util.URLPattern{[]string{"https"}, "", this.httpsHost(), stringPtr("/amp/.*"), []string{}, stringPtr(""), false, 2000, nil}, + }} + + // "Perform local transformations" is close to the last opportunity that a + // response could be proxied instead of signed. Intentionally cause an error + // to occur so that we can verify the proxy response has not been altered. + getTransformerRequest = func(r *rtv.RTVCache, s, u string) *rpb.Request { + return &rpb.Request{Html: string(s), DocumentUrl: u, Config: rpb.Request_CUSTOM, + AllowedFormats: []rpb.Request_HtmlFormat{rpb.Request_AMP}, + Transformers: []string{"bogus"}} + } + + originalHeaders := map[string]string { + "Content-Type": "text/html", + "Set-Cookie": "chocolate chip", + "Cache-Control": "max-age=31536000", + "Content-Length": fmt.Sprintf("%d", len(fakeBody)), + } + + this.fakeHandler = func(resp http.ResponseWriter, req *http.Request) { + for key, value := range originalHeaders { + resp.Header().Set(key, value) + } + resp.Write(fakeBody) + } + resp := this.get(this.T(), this.new(urlSets), "/priv/doc?sign="+url.QueryEscape(this.httpsURL()+fakePath)) + this.Assert().Equal(200, resp.StatusCode) + + // Compare the final headers to the originals, removing each one after + // checking, so that we can finally verify that no additional headers were + // appended. + for key, value := range originalHeaders { + this.Assert().Equal([]string{value}, resp.Header[key]) + resp.Header.Del(key) + } + this.Assert().Equal([]string{"Accept, AMP-Cache-Transform"}, resp.Header["Vary"]) + resp.Header.Del("Vary") + resp.Header.Del("Date") // Date header is not tested; it may be updated. + this.Assert().Empty(resp.Header) +} + func TestSignerSuite(t *testing.T) { suite.Run(t, new(SignerSuite)) } diff --git a/packager/signer/validation.go b/packager/signer/validation.go index 00f21c629..52644263e 100644 --- a/packager/signer/validation.go +++ b/packager/signer/validation.go @@ -123,10 +123,34 @@ func fetchURLMatches(url *url.URL, pattern *util.URLPattern) error { return urlMatches(url, *pattern) } +// Determines if b is either a valid byte of a fallback URL, per +// https://wicg.github.io/webpackage/draft-yasskin-http-origin-signed-responses.html#seccons-content-sniffing +// item 3.1, or is the U+0025 (%) character. +func isFallbackURLCodePoint(b byte) bool { + // https://url.spec.whatwg.org/#url-code-points, but in codepoint order: + // + // U+0021 (!), U+0024 ($), U+0026 (&), U+0027 ('), U+0028 LEFT PARENTHESIS, + // U+0029 RIGHT PARENTHESIS, U+002A (*), U+002B (+), U+002C (,), U+002D (-), + // U+002E (.), U+002F (/), ASCII digits (U+0030 - U+0039), U+003A (:), + // U+003B (;), U+003D (=), U+003F (?), U+0040 (@), + // ASCII upper alpha (U+0041 - U+005A), U+005F (_), + // ASCII lower alpha (U+0061 - U+007A), U+007E (~) + + // Vaguely ordered most to least common, to aid short-circuiting: + return (b >= 'a' && b <= 'z') || b == '_' || b == '~' || + (b >= '!' && b <= 'Z' && b != '"' /*x22*/ && b != '#' /*x23*/ && b != '<' /*x3C*/ && b != '>' /*x3E*/) +} + // True iff url matches pattern, as defined by an [URLSet.Sign] block in the // config file. The format of this URLPattern is validated by // validateSignURLPattern in config.go. func signURLMatches(url *url.URL, pattern *util.URLPattern) error { + for _, b := range []byte(url.String()) { + if !isFallbackURLCodePoint(b) { + return errors.New("Contains invalid byte") + } + } + // The sign block may not specify which schemes are allowed. Only HTTPS // is allowed: // https://wicg.github.io/webpackage/draft-yasskin-httpbis-origin-signed-exchanges-impl.html#rfc.section.5.3 @@ -181,6 +205,8 @@ func parseURLs(fetch string, sign string, urlSets []util.URLSet) (*url.URL, *url // TODO(twifkak): Use errors.Wrap() after changing return types to error. return nil, nil, false, err } + + errs := []string{} for _, set := range urlSets { err := urlsMatch(fetchURL, signURL, set) if err == nil { @@ -189,8 +215,9 @@ func parseURLs(fetch string, sign string, urlSets []util.URLSet) (*url.URL, *url } return fetchURL, signURL, set.Sign.ErrorOnStatefulHeaders, nil } + errs = append(errs, err.Error()) } - return nil, nil, false, util.NewHTTPError(http.StatusBadRequest, "fetch/sign URLs do not match config") + return nil, nil, false, util.NewHTTPError(http.StatusBadRequest, "fetch/sign URLs do not match config; caused by: ", strings.Join(errs, ", ")) } // Given a request/response pair for the fetch from the packager to the backend diff --git a/packager/signer/validation_test.go b/packager/signer/validation_test.go index 42b2dd84f..e77dc18f1 100644 --- a/packager/signer/validation_test.go +++ b/packager/signer/validation_test.go @@ -4,6 +4,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "strings" "testing" "github.com/ampproject/amppackager/packager/util" @@ -76,6 +77,15 @@ func TestFetchURLMatches(t *testing.T) { "URL too long") } +func TestIsFallbackURLCodePoint(t *testing.T) { + // https://url.spec.whatwg.org/#url-code-points + "%", in codepoint order: + validURLCodepoints := `!$%&'()*+,-./0123456789:;=?@ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz~` + for b := 0; b < 0x100; b++ { + expected := strings.ContainsRune(validURLCodepoints, rune(b)) + assert.Equal(t, expected, isFallbackURLCodePoint(byte(b)), "char: %#v", string(rune(b))) + } +} + func TestSignURLMatches(t *testing.T) { assert.NoError(t, signURLMatches(urlOrDie("https://example.com/"), &util.URLPattern{Domain: "example.com", PathRE: stringPtr(".*"), QueryRE: stringPtr(".*"), MaxLength: 2000})) @@ -138,7 +148,9 @@ func TestParseURLs(t *testing.T) { {Sign: &util.URLPattern{Domain: "badexample.com", PathRE: stringPtr(".*"), QueryRE: stringPtr(".*"), MaxLength: 2000}}, }) if assert.NotNil(t, err) { - assert.EqualError(t, err, "fetch/sign URLs do not match config") + if assert.Error(t, err) { + assert.Contains(t, err.Error(), "fetch/sign URLs do not match config") + } } } diff --git a/packager/testing/testing.go b/packager/testing/testing.go index 5a3a7a5cc..457d034b9 100644 --- a/packager/testing/testing.go +++ b/packager/testing/testing.go @@ -25,7 +25,6 @@ import ( "github.com/WICG/webpackage/go/signedexchange" "github.com/ampproject/amppackager/packager/util" - "github.com/julienschmidt/httprouter" ) // A cert (with its issuer chain) for testing. @@ -43,37 +42,66 @@ var Key = func() crypto.PrivateKey { return key }() -// The URL path component corresponding to the cert's sha-256. -var CertName = util.CertName(Certs[0]) +// 90 days cert of amppackageexample.com and www.amppackageexample.com in SAN +var B3Certs = func() []*x509.Certificate { + certPem, _ := ioutil.ReadFile("../../testdata/b3/fullchain.cert") + certs, _ := signedexchange.ParseCertificates(certPem) + return certs +}() -// A variant of http.Handler that's required by httprouter. -type AlmostHandler interface { - ServeHTTP(http.ResponseWriter, *http.Request, httprouter.Params) -} +// Private key of B3Certs +var B3Key = func() crypto.PrivateKey { + keyPem, _ := ioutil.ReadFile("../../testdata/b3/server.privkey") + key, _ := util.ParsePrivateKey(keyPem) + return key +}() -// TODO(twifkak): Make a fluent builder interface for requests, instead of this mess. +// 90 days cert of amppackageexample2.com and www.amppackageexample2.com in SAN +var B3Certs2 = func() []*x509.Certificate { + certPem, _ := ioutil.ReadFile("../../testdata/b3/fullchain2.cert") + certs, _ := signedexchange.ParseCertificates(certPem) + return certs +}() -func Get(t *testing.T, handler AlmostHandler, target string) *http.Response { - return GetP(t, handler, target, httprouter.Params{}) -} +// Private key of B3Certs2 +var B3Key2 = func() crypto.PrivateKey { + keyPem, _ := ioutil.ReadFile("../../testdata/b3/server2.privkey") + key, _ := util.ParsePrivateKey(keyPem) + return key +}() -func GetH(t *testing.T, handler AlmostHandler, target string, headers http.Header) *http.Response { - return GetHP(t, handler, target, headers, httprouter.Params{}) -} +// 91 days cert from B3Key +var B3Certs91Days = func() []*x509.Certificate { + certPem, _ := ioutil.ReadFile("../../testdata/b3/fullchain_91days.cert") + certs, _ := signedexchange.ParseCertificates(certPem) + return certs +}() -func GetP(t *testing.T, handler AlmostHandler, target string, params httprouter.Params) *http.Response { - return GetHP(t, handler, target, http.Header{}, params) +// secp521r1 private key +var B3KeyP521 = func() crypto.PrivateKey { + keyPem, _ := ioutil.ReadFile("../../testdata/b3/server_p521.privkey") + key, _ := util.ParsePrivateKey(keyPem) + return key +}() + +// The URL path component corresponding to the cert's sha-256. +var CertName = util.CertName(Certs[0]) + +// TODO(twifkak): Make a fluent builder interface for requests, instead of this mess. + +func Get(t *testing.T, handler http.Handler, target string) *http.Response { + return GetH(t, handler, target, http.Header{}) } -func GetBH(t *testing.T, handler AlmostHandler, target string, body io.Reader, headers http.Header) *http.Response { - return GetBHP(t, handler, target, body, headers, httprouter.Params{}) +func GetH(t *testing.T, handler http.Handler, target string, headers http.Header) *http.Response { + return GetBHH(t, handler, target, "", nil, headers) } -func GetHP(t *testing.T, handler AlmostHandler, target string, headers http.Header, params httprouter.Params) *http.Response { - return GetBHP(t, handler, target, nil, headers, params) +func GetHH(t *testing.T, handler http.Handler, target string, host string, headers http.Header) *http.Response { + return GetBHH(t, handler, target, host, nil, headers) } -func GetBHP(t *testing.T, handler AlmostHandler, target string, body io.Reader, headers http.Header, params httprouter.Params) *http.Response { +func GetBHH(t *testing.T, handler http.Handler, target string, host string, body io.Reader, headers http.Header) *http.Response { rec := httptest.NewRecorder() method := "" if body != nil { @@ -86,6 +114,9 @@ func GetBHP(t *testing.T, handler AlmostHandler, target string, body io.Reader, req.Header.Add(name, value) } } - handler.ServeHTTP(rec, req, params) + if host != "" { + req.Host = host + } + handler.ServeHTTP(rec, req) return rec.Result() } diff --git a/packager/util/config.go b/packager/util/config.go index 17ea9151e..1896dfe29 100644 --- a/packager/util/config.go +++ b/packager/util/config.go @@ -29,6 +29,7 @@ type Config struct { CertFile string // This must be the full certificate chain. KeyFile string // Just for the first cert, obviously. OCSPCache string + ForwardedRequestHeaders []string URLSet []URLSet } @@ -57,7 +58,7 @@ var emptyRegexp = "" var defaultPathRegexp = ".*" // Also sets defaults. -func validateURLPattern(pattern *URLPattern) error { +func ValidateURLPattern(pattern *URLPattern) error { if pattern.PathRE == nil { pattern.PathRE = &defaultPathRegexp } else if _, err := regexp.Compile(*pattern.PathRE); err != nil { @@ -79,7 +80,7 @@ func validateURLPattern(pattern *URLPattern) error { return nil } -func validateSignURLPattern(pattern *URLPattern) error { +func ValidateSignURLPattern(pattern *URLPattern) error { if pattern == nil { return errors.New("This section must be specified") } @@ -95,7 +96,7 @@ func validateSignURLPattern(pattern *URLPattern) error { if pattern.SamePath != nil { return errors.New("SamePath not allowed here") } - if err := validateURLPattern(pattern); err != nil { + if err := ValidateURLPattern(pattern); err != nil { return err } return nil @@ -103,7 +104,7 @@ func validateSignURLPattern(pattern *URLPattern) error { var allowedFetchSchemes = map[string]bool{"http": true, "https": true} -func validateFetchURLPattern(pattern *URLPattern) error { +func ValidateFetchURLPattern(pattern *URLPattern) error { if pattern == nil { return nil } @@ -136,12 +137,21 @@ func validateFetchURLPattern(pattern *URLPattern) error { if pattern.ErrorOnStatefulHeaders { return errors.New("ErrorOnStatefulHeaders not allowed here") } - if err := validateURLPattern(pattern); err != nil { + if err := ValidateURLPattern(pattern); err != nil { return err } return nil } +func ValidateForwardedRequestHeaders(hs []string) error { + for _, h := range hs { + if msg := haveInvalidForwardedRequestHeader(h); msg != "" { + return errors.Errorf("ForwardedRequestHeaders must not %s", msg) + } + } + return nil +} + // ReadConfig reads the config file specified at --config and validates it. func ReadConfig(configBytes []byte) (*Config, error) { tree, err := toml.LoadBytes(configBytes) @@ -166,6 +176,11 @@ func ReadConfig(configBytes []byte) (*Config, error) { if config.OCSPCache == "" { return nil, errors.New("must specify OCSPCache") } + if len(config.ForwardedRequestHeaders) > 0 { + if err := ValidateForwardedRequestHeaders(config.ForwardedRequestHeaders); err != nil { + return nil, err + } + } ocspDir := filepath.Dir(config.OCSPCache) if stat, err := os.Stat(ocspDir); os.IsNotExist(err) || !stat.Mode().IsDir() { return nil, errors.Errorf("OCSPCache parent directory must exist: %s", ocspDir) @@ -176,11 +191,11 @@ func ReadConfig(configBytes []byte) (*Config, error) { } for i := range config.URLSet { if config.URLSet[i].Fetch != nil { - if err := validateFetchURLPattern(config.URLSet[i].Fetch); err != nil { + if err := ValidateFetchURLPattern(config.URLSet[i].Fetch); err != nil { return nil, errors.Wrapf(err, "parsing URLSet.%d.Fetch", i) } } - if err := validateSignURLPattern(config.URLSet[i].Sign); err != nil { + if err := ValidateSignURLPattern(config.URLSet[i].Sign); err != nil { return nil, errors.Wrapf(err, "parsing URLSet.%d.Sign", i) } } diff --git a/packager/util/config_test.go b/packager/util/config_test.go index 4fb8f7944..c49414afd 100644 --- a/packager/util/config_test.go +++ b/packager/util/config_test.go @@ -51,6 +51,82 @@ func TestMinimalValidConfig(t *testing.T) { }, *config) } +func TestForwardedRequestHeader(t *testing.T) { + config, err := ReadConfig([]byte(` + CertFile = "cert.pem" + KeyFile = "key.pem" + OCSPCache = "/tmp/ocsp" + ForwardedRequestHeaders = ["X-Foo", "X-Bar"] + [[URLSet]] + [URLSet.Sign] + Domain = "example.com" + `)) + require.NoError(t, err) + assert.Equal(t, Config{ + Port: 8080, + CertFile: "cert.pem", + KeyFile: "key.pem", + OCSPCache: "/tmp/ocsp", + ForwardedRequestHeaders: []string{"X-Foo", "X-Bar"}, + URLSet: []URLSet{{ + Sign: &URLPattern{ + Domain: "example.com", + PathRE: stringPtr(".*"), + QueryRE: stringPtr(""), + MaxLength: 2000, + }, + }}, + }, *config) +} + +func TestForwardedRequestHeadersHaveHopByHopHeader(t *testing.T) { + assert.Contains(t, errorFrom(ReadConfig([]byte(` + CertFile = "cert.pem" + KeyFile = "key.pem" + OCSPCache = "/tmp/ocsp" + ForwardedRequestHeaders = ["X-Foo", "X-Bar", "connection"] + [[URLSet]] + [URLSet.Sign] + Domain = "example.com" + `))), "ForwardedRequestHeaders must not have hop-by-hop header of connection") +} + +func TestForwardedRequestHeadersHaveConditionalRequestHeader(t *testing.T) { + assert.Contains(t, errorFrom(ReadConfig([]byte(` + CertFile = "cert.pem" + KeyFile = "key.pem" + OCSPCache = "/tmp/ocsp" + ForwardedRequestHeaders = ["X-Foo", "X-Bar", "if-match"] + [[URLSet]] + [URLSet.Sign] + Domain = "example.com" + `))), "ForwardedRequestHeaders must not have conditional request header of if-match") +} + +func TestForwardedRequestHeadersHaveDisallowedHeader(t *testing.T) { + assert.Contains(t, errorFrom(ReadConfig([]byte(` + CertFile = "cert.pem" + KeyFile = "key.pem" + OCSPCache = "/tmp/ocsp" + ForwardedRequestHeaders = ["X-Foo", "X-Bar", "via"] + [[URLSet]] + [URLSet.Sign] + Domain = "example.com" + `))), "ForwardedRequestHeaders must not include request header of via") +} + +func TestForwardedRequestHeadersHaveTE(t *testing.T) { + assert.Contains(t, errorFrom(ReadConfig([]byte(` + CertFile = "cert.pem" + KeyFile = "key.pem" + OCSPCache = "/tmp/ocsp" + ForwardedRequestHeaders = ["X-Foo", "X-Bar", "TE"] + [[URLSet]] + [URLSet.Sign] + Domain = "example.com" + `))), "ForwardedRequestHeaders must not include request header of TE") +} + func TestOCSPDirDoesntExist(t *testing.T) { assert.Contains(t, errorFrom(ReadConfig([]byte(` CertFile = "cert.pem" diff --git a/packager/util/http.go b/packager/util/http.go index 254d63a2c..824cb47ab 100644 --- a/packager/util/http.go +++ b/packager/util/http.go @@ -1,10 +1,93 @@ package util import ( + "fmt" "regexp" + "net/http" + "strings" ) // A comma, as defined in https://tools.ietf.org/html/rfc7230#section-7, with // OWS defined in https://tools.ietf.org/html/rfc7230#appendix-B. This is // commonly used as a separator in header field value definitions. var Comma *regexp.Regexp = regexp.MustCompile(`[ \t]*,[ \t]*`) + +// Trim optional whitespace from a header value, adhering to +// https://tools.ietf.org/html/rfc7230#section-7 with OWS defined in +// https://tools.ietf.org/html/rfc7230#appendix-B. +func TrimHeaderValue(s string) string { + return strings.TrimFunc(s, func(r rune) bool { + return r == ' ' || r == '\t' + }) +} + +// Conditional request headers that ServeHTTP may receive and need to be sent with fetchURL. +// https://developer.mozilla.org/en-US/docs/Web/HTTP/Conditional_requests#Conditional_headers +var ConditionalRequestHeaders = map[string]bool{ + "If-Match": true, + "If-None-Match": true, + "If-Modified-Since": true, + "If-Unmodified-Since": true, + "If-Range": true, +} + +// The following hop-by-hop headers should be removed even when not specified +// in Connection, for backwards compatibility with downstream servers that were +// written against RFC 2616, and expect gateways to behave according to +// https://tools.ietf.org/html/rfc2616#section-13.5.1. (Note: "Trailers" is a +// typo there; should be "Trailer".) +// +// Connection header should also be removed per +// https://tools.ietf.org/html/rfc7230#section-6.1. +// +// Proxy-Connection should also be deleted, per +// https://github.com/WICG/webpackage/pull/339. +var legacyHeaders = map[string]bool{ + "Connection": true, + "Keep-Alive": true, + "Proxy-Authenticate": true, + "Proxy-Connection": true, + "Trailer": true, + "Transfer-Encoding": true, + "Upgrade": true, +} + +// Via is implicitly forwarded and disallowed to be included in +// config.ForwardedRequestHeaders +// TE is a hop-by-hop request header and must not be forwarded. +// Proxy-Authorization can be forwarded per rfc7235#section-4.4 but +// remove it to mitigate the risk of over-signing. +var notForwardedRequestHeader = map[string]bool{ + "Proxy-Authorization": true, + "Te": true, + "Via": true, +} + +// Remove hop-by-hop headers, per https://tools.ietf.org/html/rfc7230#section-6.1. +func RemoveHopByHopHeaders(h http.Header) { + if connections, ok := h[http.CanonicalHeaderKey("Connection")]; ok { + for _, connection := range connections { + headerNames := Comma.Split(connection, -1) + for _, headerName := range headerNames { + h.Del(headerName) + } + } + } + + for headerName, _ := range legacyHeaders { + h.Del(headerName) + } +} + +func haveInvalidForwardedRequestHeader(h string) string { + if _, ok := legacyHeaders[http.CanonicalHeaderKey(h)]; ok { + return fmt.Sprintf("have hop-by-hop header of %s", h) + } + if _, ok := ConditionalRequestHeaders[http.CanonicalHeaderKey(h)]; ok { + return fmt.Sprintf("have conditional request header of %s", h) + } + if _, ok := notForwardedRequestHeader[http.CanonicalHeaderKey(h)]; ok { + return fmt.Sprintf("include request header of %s", h) + } + return "" +} diff --git a/packager/util/util.go b/packager/util/util.go index dc7dbd06c..4c22c0d65 100644 --- a/packager/util/util.go +++ b/packager/util/util.go @@ -17,11 +17,13 @@ package util import ( "bytes" "crypto" + "crypto/ecdsa" "crypto/sha256" "crypto/x509" "encoding/asn1" "encoding/base64" "encoding/pem" + "time" "github.com/WICG/webpackage/go/signedexchange" "github.com/pkg/errors" @@ -29,6 +31,12 @@ import ( const CertURLPrefix = "/amppkg/cert" +// https://wicg.github.io/webpackage/draft-yasskin-http-origin-signed-responses.html#cross-origin-cert-req +// Clients MUST reject certificates with this extension that were issued after 2019-05-01 and have a Validity Period longer than 90 days. +// After 2019-08-01, clients MUST reject all certificates with this extension that have a Validity Period longer than 90 days. +var start90DayGracePeriod = time.Date(2019, time.May, 1, 0, 0, 0, 0, time.UTC) +var end90DayGracePeriod = time.Date(2019, time.August, 1, 0, 0, 0, 0, time.UTC) + // CertName returns the basename for the given cert, as served by this // packager's cert cache. Should be stable and unique (e.g. // content-addressing). Clients should url.PathEscape this, just in case its @@ -63,10 +71,7 @@ func ParsePrivateKey(keyPem []byte) (crypto.PrivateKey, error) { } } -// CanSignHttpExchanges returns true if the given certificate has the -// CanSignHttpExchanges extension. This is not the only requirement for SXGs; -// it also needs to use the right public key type, which is not checked here. -func CanSignHttpExchanges(cert *x509.Certificate) bool { +func hasCanSignHttpExchangesExtension(cert *x509.Certificate) bool { // https://wicg.github.io/webpackage/draft-yasskin-httpbis-origin-signed-exchanges-impl.html#cross-origin-cert-req for _, ext := range cert.Extensions { // 0x05, 0x00 is the DER encoding of NULL. @@ -76,3 +81,40 @@ func CanSignHttpExchanges(cert *x509.Certificate) bool { } return false } + +// CanSignHttpExchanges returns nil if the given certificate has the +// CanSignHttpExchanges extension, and a valid lifetime per the SXG spec; +// otherwise it returns an error. These are not the only requirements for SXGs; +// it also needs to use the right public key type, which is not checked here. +func CanSignHttpExchanges(cert *x509.Certificate, now time.Time) error { + if !hasCanSignHttpExchangesExtension(cert) { + return errors.New("Certificate is missing CanSignHttpExchanges extension") + } + + // TODO: remove issue date and current time check after 2019-08-01 + if cert.NotBefore.After(start90DayGracePeriod) || now.After(end90DayGracePeriod) { + if cert.NotBefore.AddDate(0,0,90).Before(cert.NotAfter) { + return errors.New("Certificate MUST have a Validity Period no greater than 90 days") + } + } + return nil +} + +// Returns nil if the certificate matches the private key and domain, else the appropriate error. +func CertificateMatches(cert *x509.Certificate, priv crypto.PrivateKey, domain string) error { + certPubKey := cert.PublicKey.(*ecdsa.PublicKey) + pubKey := priv.(*ecdsa.PrivateKey).PublicKey + if certPubKey.Curve != pubKey.Curve { + return errors.New("PublicKey.Curve not match") + } + if certPubKey.X.Cmp(pubKey.X) != 0 { + return errors.New("PublicKey.X not match") + } + if certPubKey.Y.Cmp(pubKey.Y) != 0 { + return errors.New("PublicKey.Y not match") + } + if err := cert.VerifyHostname(domain); err != nil { + return err + } + return nil +} diff --git a/packager/util/util_test.go b/packager/util/util_test.go index c44fcb16f..12f79d356 100644 --- a/packager/util/util_test.go +++ b/packager/util/util_test.go @@ -4,6 +4,7 @@ import ( "crypto/ecdsa" "crypto/elliptic" "testing" + "time" pkgt "github.com/ampproject/amppackager/packager/testing" "github.com/ampproject/amppackager/packager/util" @@ -11,6 +12,13 @@ import ( "github.com/stretchr/testify/require" ) +func errorFrom(err error) string { + if err == nil { + return "" + } + return err.Error() +} + func TestCertName(t *testing.T) { assert.Equal(t, "PJ1IwfP1igOlJd2oTUVs2mj4dWIZcOWHMk5jfJYS2Qc", util.CertName(pkgt.Certs[0])) } @@ -21,9 +29,56 @@ func TestParsePrivateKey(t *testing.T) { assert.Equal(t, elliptic.P256(), pkgt.Key.(*ecdsa.PrivateKey).PublicKey.Curve) } -func TestCanSignHttpExchanges(t *testing.T) { +func TestCanSignHttpExchangesExtension(t *testing.T) { + // Before grace period, to allow the >90-day lifetime. + now := time.Date(2019, time.July, 31, 0, 0, 0, 0, time.UTC) + // Leaf node has the extension. - assert.True(t, util.CanSignHttpExchanges(pkgt.Certs[0])) + assert.Nil(t, util.CanSignHttpExchanges(pkgt.Certs[0], now)) // CA node does not. - assert.False(t, util.CanSignHttpExchanges(pkgt.Certs[1])) + assert.EqualError(t, util.CanSignHttpExchanges(pkgt.Certs[1], now), "Certificate is missing CanSignHttpExchanges extension") +} + +func TestParseCertificate(t *testing.T) { + assert.Nil(t, util.CertificateMatches(pkgt.B3Certs[0], pkgt.B3Key, "amppackageexample.com")) +} + +func TestParseCertificateSubjectAltName(t *testing.T) { + assert.Nil(t, util.CertificateMatches(pkgt.B3Certs[0], pkgt.B3Key, "www.amppackageexample.com")) +} + +func TestParseCertificateNotMatchX(t *testing.T) { + assert.Contains(t, errorFrom(util.CertificateMatches(pkgt.B3Certs[0], + pkgt.B3Key2, "amppackageexample.com")), "PublicKey.X not match") +} + +func TestParseCertificateNotMatchCurve(t *testing.T) { + assert.Contains(t, errorFrom(util.CertificateMatches(pkgt.B3Certs[0], + pkgt.B3KeyP521, "amppackageexample.com")), "PublicKey.Curve not match") +} + +func TestParseCertificateNotMatchDomain(t *testing.T) { + assert.Contains(t, errorFrom(util.CertificateMatches(pkgt.B3Certs2[0], + pkgt.B3Key2, "amppackageexample.com")), "x509: certificate is valid for amppackageexample2.com, www.amppackageexample2.com, not amppackageexample.com") +} + +func TestParse90DaysCertificateAfterGracePeriod(t *testing.T) { + now := time.Date(2019, time.August, 1, 0, 0, 0, 1, time.UTC) + assert.Nil(t, util.CanSignHttpExchanges(pkgt.B3Certs[0], now)) +} + +func TestParse91DaysCertificate(t *testing.T) { + assert.Contains(t, errorFrom(util.CanSignHttpExchanges(pkgt.B3Certs91Days[0], + time.Now())), "Certificate MUST have a Validity Period no greater than 90 days") +} + +func TestParseCertificateIssuedBeforeMay1InGarcePeriod(t *testing.T) { + now := time.Date(2019, time.July, 31, 0, 0, 0, 0, time.UTC) + assert.Nil(t, util.CanSignHttpExchanges(pkgt.Certs[0], now)) +} + +func TestParseCertificateIssuedBeforeMay1AfterGracePeriod(t *testing.T) { + now := time.Date(2019, time.August, 1, 0, 0, 0, 1, time.UTC) + assert.Contains(t, errorFrom(util.CanSignHttpExchanges(pkgt.Certs[0], + now)), "Certificate MUST have a Validity Period no greater than 90 days") } diff --git a/packager/validitymap/validitymap.go b/packager/validitymap/validitymap.go index d49c279dc..89982413b 100644 --- a/packager/validitymap/validitymap.go +++ b/packager/validitymap/validitymap.go @@ -18,8 +18,6 @@ import ( "bytes" "net/http" "time" - - "github.com/julienschmidt/httprouter" ) type ValidityMap struct { @@ -34,7 +32,7 @@ func New() (*ValidityMap, error) { return this, nil } -func (this *ValidityMap) ServeHTTP(resp http.ResponseWriter, req *http.Request, _ httprouter.Params) { +func (this *ValidityMap) ServeHTTP(resp http.ResponseWriter, req *http.Request) { resp.Header().Set("Content-Type", "application/cbor") resp.Header().Set("Cache-Control", "public, max-age=604800") resp.Header().Set("X-Content-Type-Options", "nosniff") diff --git a/packager/validitymap/validitymap_test.go b/packager/validitymap/validitymap_test.go index c582e0822..4158236b4 100644 --- a/packager/validitymap/validitymap_test.go +++ b/packager/validitymap/validitymap_test.go @@ -4,6 +4,7 @@ import ( "io/ioutil" "testing" + "github.com/ampproject/amppackager/packager/mux" pkgt "github.com/ampproject/amppackager/packager/testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -13,7 +14,7 @@ func TestValidityMap(t *testing.T) { handler, err := New() require.NoError(t, err) - resp := pkgt.Get(t, handler, "/") + resp := pkgt.Get(t, mux.New(nil, nil, handler), "/amppkg/validity") defer resp.Body.Close() assert.Equal(t, "application/cbor", resp.Header.Get("Content-Type")) assert.Equal(t, "public, max-age=604800", resp.Header.Get("Cache-Control")) diff --git a/testdata/b3/README.md b/testdata/b3/README.md new file mode 100644 index 000000000..807bf3a91 --- /dev/null +++ b/testdata/b3/README.md @@ -0,0 +1,50 @@ +# Test certificates for b3 + +This is example certificates for tests built under the constraints set by `v=b3` except having AIA and SCT extension. + +To generate: + +CA private key and cert, +``` +$ openssl genrsa -out ca.privkey 2048 +$ openssl req -x509 -new -nodes -key ca.privkey -sha256 -days 1825 -out ca.cert -subj '/C=US/ST=California/O=Google LLC/CN=Fake CA' +``` + +server.privkey and server.cert of amppackageexample.com and www.amppackageexample.com, +``` +$ openssl ecparam -out server.privkey -name prime256v1 -genkey +$ openssl req -new -sha256 -key server.privkey -out server.csr -subj /CN=amppackageexample.com +$ openssl x509 -req -in server.csr -CA ca.cert -CAkey ca.privkey -CAcreateserial -out server.cert -days 90 -extfile <(echo -e "subjectAltName = DNS:amppackageexample.com,DNS:www.amppackageexample.com\n1.3.6.1.4.1.11129.2.1.22 = ASN1:NULL") +$ cat server.cert ca.cert > fullchain.cert + +server2.privkey and server2.cert of amppackageexample2.com and www.amppackageexample2.com, +``` +$ openssl ecparam -out server2.privkey -name prime256v1 -genkey +$ openssl req -new -sha256 -key server2.privkey -out server2.csr -subj /CN=amppackageexample2.com +$ openssl x509 -req -in server2.csr -CA ca.cert -CAkey ca.privkey -CAcreateserial -out server2.cert -days 90 -extfile <(echo -e "subjectAltName = DNS:amppackageexample2.com,DNS:www.amppackageexample2.com\n1.3.6.1.4.1.11129.2.1.22 = ASN1:NULL") +$ cat server2.cert ca.cert > fullchain2.cert +``` + +and others +``` +$ openssl x509 -req -in server.csr -CA ca.cert -CAkey ca.privkey -CAcreateserial -out server_91days.cert -days 91 -extfile <(echo -e "subjectAltName = DNS:amppackageexample.com,DNS:www.amppackageexample.com\n1.3.6.1.4.1.11129.2.1.22 = ASN1:NULL") +$ cat server_91days.cert ca.cert > fullchain_91days.cert +$ openssl ecparam -out server_p521.privkey -name secp521r1 -genkey +``` + +### Appendix + + + +See some tutorials: + - https://github.com/WICG/webpackage/tree/master/go/signedexchange + - https://deliciousbrains.com/ssl-certificate-authority-for-local-https-development/ + - https://github.com/jmmcatee/cracklord/wiki/Creating-Certificate-Authentication-From-Scratch-OpenSSL + - https://gist.github.com/Soarez/9688998 + - https://jamielinux.com/docs/openssl-certificate-authority/online-certificate-status-protocol.html diff --git a/testdata/b3/ca.cert b/testdata/b3/ca.cert new file mode 100644 index 000000000..3c25bf9cd --- /dev/null +++ b/testdata/b3/ca.cert @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDaDCCAlCgAwIBAgIJAKRDrJBUupZAMA0GCSqGSIb3DQEBCwUAMEkxCzAJBgNV +BAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMRMwEQYDVQQKDApHb29nbGUgTExD +MRAwDgYDVQQDDAdGYWtlIENBMB4XDTE4MDgyOTIxMjIyOVoXDTIzMDgyODIxMjIy +OVowSTELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExEzARBgNVBAoM +Ckdvb2dsZSBMTEMxEDAOBgNVBAMMB0Zha2UgQ0EwggEiMA0GCSqGSIb3DQEBAQUA +A4IBDwAwggEKAoIBAQC52bjEZwQIt5pIZY712P6nbQOoRGKmalU6SksHjhVx8x93 +58Fjs+z1S1xm+HwU8LDj82wapj2/GZbLgk6wiYeSF17a4cKbdOfZUqQiQpQfciAB +S3sd2ZG0YghR/VdTHa3mH56lX90Z/3pq0KlwIDk1U/PpKYaWwC1Ywpj1COEJ9Omd +TjcoPQ1nEc1vKdlyjlAvDYqKptrK2grGykvu0Rh2ZeJ99YSSYknh6zx6U3FWtKlm +PgMjSWO2gtpbMLPI3Uehde0b3Bq8JntjPvbh8gLoGaOEsvvDgTfFRpRkNqMAjLVf +z/zol9CpBdggg4Fyj9J0CGjM9f1ZBhXLmtoxkAQjAgMBAAGjUzBRMB0GA1UdDgQW +BBSiY4BoP6E4Rx6tSi0iXDHU5D3vlTAfBgNVHSMEGDAWgBSiY4BoP6E4Rx6tSi0i +XDHU5D3vlTAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQCG9ARQ +S1tOW0PVuJ5H4WU0hRmDn5oUkEu9DzD1WoGGTN6CCM0AfLBo0ARwqntvSNtERIA6 +BfVSaeqO6WNrvGDLwi+YcUlTxyGLsGlDz/xeQ7WIVGlbhyTJYH5B1yliRyZN4M2E +hIqWqBMSvtqCnMMJEr/0BsigDIEiEUXwsc6tC77HudXxV0AVs4MLBz3iowkeZMB6 +qZAVRKCmVptIsPyP+eO3vIIWi0y7eQx/8WOpW52CYj9MhhDa9/IcJLIRQIOe7fRn +1tL/h+aRMq7ywOrwbO/s2HQ7kG/jFIRj0QMxeCExTzPhND22EVvVisfocXS91QuL +PjZuIWmKwDo/Vvau +-----END CERTIFICATE----- diff --git a/testdata/b3/ca.privkey b/testdata/b3/ca.privkey new file mode 100644 index 000000000..a3f2c488e --- /dev/null +++ b/testdata/b3/ca.privkey @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEowIBAAKCAQEAudm4xGcECLeaSGWO9dj+p20DqERipmpVOkpLB44VcfMfd+fB +Y7Ps9UtcZvh8FPCw4/NsGqY9vxmWy4JOsImHkhde2uHCm3Tn2VKkIkKUH3IgAUt7 +HdmRtGIIUf1XUx2t5h+epV/dGf96atCpcCA5NVPz6SmGlsAtWMKY9QjhCfTpnU43 +KD0NZxHNbynZco5QLw2KiqbaytoKxspL7tEYdmXiffWEkmJJ4es8elNxVrSpZj4D +I0ljtoLaWzCzyN1HoXXtG9wavCZ7Yz724fIC6BmjhLL7w4E3xUaUZDajAIy1X8/8 +6JfQqQXYIIOBco/SdAhozPX9WQYVy5raMZAEIwIDAQABAoIBACHsv1CCqXbZ5PzQ +JQ91g86WFLPTf9p20IXqZ9XCNuHtClJ96IxFnLyN/BkDxMqhwPhrR9F5hQ3sIt2V +NL3+7NNbFsKHsVllNqkx76odUyKGV5dE6v1g6LrvpispPpZ6dXLrVK9FV3vWaccz +vaotB6RXZc+q99luzRhFtVwNOd7yGQWkn1sKO1i/QWiARdob/inKArbNl3W0GtJ0 +kIjQNm28JFpzTxUbjoWOOUoH4B8PyOS+k6+ByjJDBhLiinl3I7M+g6LWqr2cdam8 +xyoHHyUtYM1DbCoPJKDtJeJkxNP9TZTJLLq/lqRjm14kvcIHvQxFSZF/NqJivurq +LYtghDkCgYEA7SC1ot2PjkZ2MZNQQsT5HrAwqqfogb9AvQ2HSkFN71LGZQcfxhNS +chg1/MH3R9k95xDPSFc2T5DxEBUFaImeGhPNnm1YzHJbixZgBu2G6DAEks95qtYf +bDWRFG60ulaJAeveVYxhnoXARc9NMPAbek6VgFi1QWFY+M/VTC3cve8CgYEAyKRH +J6eUurQZti9mUwbSPWB41+hi7KFL3qC618qOf0XDnDlGvhSxkMLdnICMDOxg6U/b +0G4Zlyb6JbyZ5jiDpvcAbqqGsickmI8SJeKbp9zQT0mELuRdx12YuMi0aA81gaIm +ekYDiQp28enO778s033ikrYwVu2yLakrFayakQ0CgYEAnc8Z8nSfGCF+gUm3rWfn +HuxExx4Nl2OPkwGQ2vMRCce9rviJxcmQIcxJCZiQl+lU0BUYzdz0kQk11O0Yd1S2 +ukYZnmjJIu6sS6ktaQ7krFtgf8/B+dacfOg9UCrI7gWvEm9FvQs64EPFDPCEP6Bb +uQ7ZYdwnbIZ7rsKqAhO3h1MCgYBfGwejc1sbmO0rH5K4Pl5/u2/sn/nsQpStBbEr +QpeDGrWbIsc2qKZ2gPf9DC3WnmFdln4ScW3t6QrfwmOM7jLxfNmWm3xXjBhbvE2U +6bJwwkl3m9htRdByBRq0VGa3gKYTOaJViUR5vB0flH2DxTHhWiWA9504R1mTLUH/ +9x4ZLQKBgE039cFcqazKgUrhGfnHo6DKMYv9tLfpwBrfGrjS5eL9HzwCjqflH7Ql +rN/8D4hNeT+lU9m8gAZkf7a/9Atz+V97gIKRzLg8ShHPg4lcjHaAK35CRSrNc+i0 +Hf1WpUKryO9CCzqsmO1Z+hxLVC1rSYJ2C8TRI3FMf3Gay/IzUUXF +-----END RSA PRIVATE KEY----- diff --git a/testdata/b3/ca.srl b/testdata/b3/ca.srl new file mode 100644 index 000000000..39d5a1078 --- /dev/null +++ b/testdata/b3/ca.srl @@ -0,0 +1 @@ +E9E246FA1A4FB5BF diff --git a/testdata/b3/fullchain.cert b/testdata/b3/fullchain.cert new file mode 100644 index 000000000..ba367b7a9 --- /dev/null +++ b/testdata/b3/fullchain.cert @@ -0,0 +1,37 @@ +-----BEGIN CERTIFICATE----- +MIICcjCCAVqgAwIBAgIJAOniRvoaT7W9MA0GCSqGSIb3DQEBCwUAMEkxCzAJBgNV +BAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMRMwEQYDVQQKDApHb29nbGUgTExD +MRAwDgYDVQQDDAdGYWtlIENBMB4XDTE5MDUwOTA1NDMzMloXDTE5MDgwNzA1NDMz +MlowIDEeMBwGA1UEAwwVYW1wcGFja2FnZWV4YW1wbGUuY29tMFkwEwYHKoZIzj0C +AQYIKoZIzj0DAQcDQgAEOFjcTTfGjzqRM1fZ+vzaDcOwQcO0enVPHYqe6c5pPEux +3DYBUbPOkLuyzCwUpC+kxu1a3GcvdO4yyVT56GvEfqNRME8wOwYDVR0RBDQwMoIV +YW1wcGFja2FnZWV4YW1wbGUuY29tghl3d3cuYW1wcGFja2FnZWV4YW1wbGUuY29t +MBAGCisGAQQB1nkCARYEAgUAMA0GCSqGSIb3DQEBCwUAA4IBAQA15v5hJEiinNQk +GplWJBYeBSjWMnwfvXqpWYiIw47692+ELBihlmYY+xcAhUbvBty7695LiHRRbJLL +c4QC8EoAePuvrty6vZVZbVc9FUFRKvgQ0n3FzMok/0QTfUNeQbfQBWomxxmlySsD +WwQ3fcRtGRETspcBMrhHorVplSAqAtr41bxe1Wh+RywmJRtG+1/Tp8zU6Py6bkPz +pHkwkkua+ucwhU0xk3Ipe5vZVOSZQofVqDpSuUadrhU6B7fDI+B+nniiyOXplgNx +CVoSy6nrsUb1nAFIDxKxrTrhlHi8oXGR9JvMLf4u+A+0d64SeXkgmYFIc5ftYHlz +BKMgdmFI +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIIDaDCCAlCgAwIBAgIJAKRDrJBUupZAMA0GCSqGSIb3DQEBCwUAMEkxCzAJBgNV +BAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMRMwEQYDVQQKDApHb29nbGUgTExD +MRAwDgYDVQQDDAdGYWtlIENBMB4XDTE4MDgyOTIxMjIyOVoXDTIzMDgyODIxMjIy +OVowSTELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExEzARBgNVBAoM +Ckdvb2dsZSBMTEMxEDAOBgNVBAMMB0Zha2UgQ0EwggEiMA0GCSqGSIb3DQEBAQUA +A4IBDwAwggEKAoIBAQC52bjEZwQIt5pIZY712P6nbQOoRGKmalU6SksHjhVx8x93 +58Fjs+z1S1xm+HwU8LDj82wapj2/GZbLgk6wiYeSF17a4cKbdOfZUqQiQpQfciAB +S3sd2ZG0YghR/VdTHa3mH56lX90Z/3pq0KlwIDk1U/PpKYaWwC1Ywpj1COEJ9Omd +TjcoPQ1nEc1vKdlyjlAvDYqKptrK2grGykvu0Rh2ZeJ99YSSYknh6zx6U3FWtKlm +PgMjSWO2gtpbMLPI3Uehde0b3Bq8JntjPvbh8gLoGaOEsvvDgTfFRpRkNqMAjLVf +z/zol9CpBdggg4Fyj9J0CGjM9f1ZBhXLmtoxkAQjAgMBAAGjUzBRMB0GA1UdDgQW +BBSiY4BoP6E4Rx6tSi0iXDHU5D3vlTAfBgNVHSMEGDAWgBSiY4BoP6E4Rx6tSi0i +XDHU5D3vlTAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQCG9ARQ +S1tOW0PVuJ5H4WU0hRmDn5oUkEu9DzD1WoGGTN6CCM0AfLBo0ARwqntvSNtERIA6 +BfVSaeqO6WNrvGDLwi+YcUlTxyGLsGlDz/xeQ7WIVGlbhyTJYH5B1yliRyZN4M2E +hIqWqBMSvtqCnMMJEr/0BsigDIEiEUXwsc6tC77HudXxV0AVs4MLBz3iowkeZMB6 +qZAVRKCmVptIsPyP+eO3vIIWi0y7eQx/8WOpW52CYj9MhhDa9/IcJLIRQIOe7fRn +1tL/h+aRMq7ywOrwbO/s2HQ7kG/jFIRj0QMxeCExTzPhND22EVvVisfocXS91QuL +PjZuIWmKwDo/Vvau +-----END CERTIFICATE----- diff --git a/testdata/b3/fullchain2.cert b/testdata/b3/fullchain2.cert new file mode 100644 index 000000000..a9b08c78c --- /dev/null +++ b/testdata/b3/fullchain2.cert @@ -0,0 +1,37 @@ +-----BEGIN CERTIFICATE----- +MIICdTCCAV2gAwIBAgIJAOniRvoaT7W+MA0GCSqGSIb3DQEBCwUAMEkxCzAJBgNV +BAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMRMwEQYDVQQKDApHb29nbGUgTExD +MRAwDgYDVQQDDAdGYWtlIENBMB4XDTE5MDUwOTA1NDQ1MVoXDTE5MDgwNzA1NDQ1 +MVowITEfMB0GA1UEAwwWYW1wcGFja2FnZWV4YW1wbGUyLmNvbTBZMBMGByqGSM49 +AgEGCCqGSM49AwEHA0IABPrQ/l9HqUGjmupFkJo+dZt6/UoR5sFunT72ZoRc0Nvh +zP+qpvZdTBzkZejExW3xor6iIGowuKKPWOJA976Sj52jUzBRMD0GA1UdEQQ2MDSC +FmFtcHBhY2thZ2VleGFtcGxlMi5jb22CGnd3dy5hbXBwYWNrYWdlZXhhbXBsZTIu +Y29tMBAGCisGAQQB1nkCARYEAgUAMA0GCSqGSIb3DQEBCwUAA4IBAQCmh+Ff+GUC +S/IFh2xLtXcnB+elfr75PwldD91LqRnTYu/DC9tKcfDxKZIwHw0B5gLg362lAdB1 +88I8XJrrf8WfH6auCcuhdoCDyNVbbXqsapwTHEaUFZiCVnPt0eJRdau6voZLKGZM +/zEMVUg9qPOSCR5+j2hpmPyFNCUqK6RpNpCKFXf/WJOHphAIqLeFbLJa4kIfIL6b +m/K2Og93H0uVX3j5jgaQncRxX6ZKCje+2MzNvPBZhWJgU+xTDPLPOPeQX+sHj6k6 +TjsTgQ9GbpHPuoWc0keW6ulVwN7VW5EfUmM3fvftO2/5hHI6qIVnhrTgto+u+7vH +oDSK9AJC357d +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIIDaDCCAlCgAwIBAgIJAKRDrJBUupZAMA0GCSqGSIb3DQEBCwUAMEkxCzAJBgNV +BAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMRMwEQYDVQQKDApHb29nbGUgTExD +MRAwDgYDVQQDDAdGYWtlIENBMB4XDTE4MDgyOTIxMjIyOVoXDTIzMDgyODIxMjIy +OVowSTELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExEzARBgNVBAoM +Ckdvb2dsZSBMTEMxEDAOBgNVBAMMB0Zha2UgQ0EwggEiMA0GCSqGSIb3DQEBAQUA +A4IBDwAwggEKAoIBAQC52bjEZwQIt5pIZY712P6nbQOoRGKmalU6SksHjhVx8x93 +58Fjs+z1S1xm+HwU8LDj82wapj2/GZbLgk6wiYeSF17a4cKbdOfZUqQiQpQfciAB +S3sd2ZG0YghR/VdTHa3mH56lX90Z/3pq0KlwIDk1U/PpKYaWwC1Ywpj1COEJ9Omd +TjcoPQ1nEc1vKdlyjlAvDYqKptrK2grGykvu0Rh2ZeJ99YSSYknh6zx6U3FWtKlm +PgMjSWO2gtpbMLPI3Uehde0b3Bq8JntjPvbh8gLoGaOEsvvDgTfFRpRkNqMAjLVf +z/zol9CpBdggg4Fyj9J0CGjM9f1ZBhXLmtoxkAQjAgMBAAGjUzBRMB0GA1UdDgQW +BBSiY4BoP6E4Rx6tSi0iXDHU5D3vlTAfBgNVHSMEGDAWgBSiY4BoP6E4Rx6tSi0i +XDHU5D3vlTAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQCG9ARQ +S1tOW0PVuJ5H4WU0hRmDn5oUkEu9DzD1WoGGTN6CCM0AfLBo0ARwqntvSNtERIA6 +BfVSaeqO6WNrvGDLwi+YcUlTxyGLsGlDz/xeQ7WIVGlbhyTJYH5B1yliRyZN4M2E +hIqWqBMSvtqCnMMJEr/0BsigDIEiEUXwsc6tC77HudXxV0AVs4MLBz3iowkeZMB6 +qZAVRKCmVptIsPyP+eO3vIIWi0y7eQx/8WOpW52CYj9MhhDa9/IcJLIRQIOe7fRn +1tL/h+aRMq7ywOrwbO/s2HQ7kG/jFIRj0QMxeCExTzPhND22EVvVisfocXS91QuL +PjZuIWmKwDo/Vvau +-----END CERTIFICATE----- diff --git a/testdata/b3/fullchain_91days.cert b/testdata/b3/fullchain_91days.cert new file mode 100644 index 000000000..f08ad75a6 --- /dev/null +++ b/testdata/b3/fullchain_91days.cert @@ -0,0 +1,37 @@ +-----BEGIN CERTIFICATE----- +MIICcjCCAVqgAwIBAgIJAOniRvoaT7W/MA0GCSqGSIb3DQEBCwUAMEkxCzAJBgNV +BAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMRMwEQYDVQQKDApHb29nbGUgTExD +MRAwDgYDVQQDDAdGYWtlIENBMB4XDTE5MDUwOTA1NDUzNloXDTE5MDgwODA1NDUz +NlowIDEeMBwGA1UEAwwVYW1wcGFja2FnZWV4YW1wbGUuY29tMFkwEwYHKoZIzj0C +AQYIKoZIzj0DAQcDQgAEOFjcTTfGjzqRM1fZ+vzaDcOwQcO0enVPHYqe6c5pPEux +3DYBUbPOkLuyzCwUpC+kxu1a3GcvdO4yyVT56GvEfqNRME8wOwYDVR0RBDQwMoIV +YW1wcGFja2FnZWV4YW1wbGUuY29tghl3d3cuYW1wcGFja2FnZWV4YW1wbGUuY29t +MBAGCisGAQQB1nkCARYEAgUAMA0GCSqGSIb3DQEBCwUAA4IBAQAsXV23u2GWOkXT +HLV/hqmBE/sgcAYYqaUTByptHwP5gua7ARcJ3AfIcMYCLzuIvgEKLSp3KK14Ejhx +ZopyrzbfQNYw+cDUpvUlV1gERL6EULBg6olA8qtaxINRQ1a9VE32+Kyu1Ht8/NMf +VU5x+OW35qPT/LOC2TkY3/KfmSuRF1sYNgnR/fuP3pUi4DhF9Vh1aT3pgLCXbmKz +fpScky+edojNEM8TLM9bVjrKyW4cReJVu4bRq5VHgMAebNM2CYOME7j330fB3VCs +qksBz2Q2j6ThRIpTLYosiqOBEff9NAfsbOsgLuCkbbMewD7TDOh7tMyZfJ/1YLhT +KXTzvbo3 +-----END CERTIFICATE----- +-----BEGIN CERTIFICATE----- +MIIDaDCCAlCgAwIBAgIJAKRDrJBUupZAMA0GCSqGSIb3DQEBCwUAMEkxCzAJBgNV +BAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMRMwEQYDVQQKDApHb29nbGUgTExD +MRAwDgYDVQQDDAdGYWtlIENBMB4XDTE4MDgyOTIxMjIyOVoXDTIzMDgyODIxMjIy +OVowSTELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNhbGlmb3JuaWExEzARBgNVBAoM +Ckdvb2dsZSBMTEMxEDAOBgNVBAMMB0Zha2UgQ0EwggEiMA0GCSqGSIb3DQEBAQUA +A4IBDwAwggEKAoIBAQC52bjEZwQIt5pIZY712P6nbQOoRGKmalU6SksHjhVx8x93 +58Fjs+z1S1xm+HwU8LDj82wapj2/GZbLgk6wiYeSF17a4cKbdOfZUqQiQpQfciAB +S3sd2ZG0YghR/VdTHa3mH56lX90Z/3pq0KlwIDk1U/PpKYaWwC1Ywpj1COEJ9Omd +TjcoPQ1nEc1vKdlyjlAvDYqKptrK2grGykvu0Rh2ZeJ99YSSYknh6zx6U3FWtKlm +PgMjSWO2gtpbMLPI3Uehde0b3Bq8JntjPvbh8gLoGaOEsvvDgTfFRpRkNqMAjLVf +z/zol9CpBdggg4Fyj9J0CGjM9f1ZBhXLmtoxkAQjAgMBAAGjUzBRMB0GA1UdDgQW +BBSiY4BoP6E4Rx6tSi0iXDHU5D3vlTAfBgNVHSMEGDAWgBSiY4BoP6E4Rx6tSi0i +XDHU5D3vlTAPBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQCG9ARQ +S1tOW0PVuJ5H4WU0hRmDn5oUkEu9DzD1WoGGTN6CCM0AfLBo0ARwqntvSNtERIA6 +BfVSaeqO6WNrvGDLwi+YcUlTxyGLsGlDz/xeQ7WIVGlbhyTJYH5B1yliRyZN4M2E +hIqWqBMSvtqCnMMJEr/0BsigDIEiEUXwsc6tC77HudXxV0AVs4MLBz3iowkeZMB6 +qZAVRKCmVptIsPyP+eO3vIIWi0y7eQx/8WOpW52CYj9MhhDa9/IcJLIRQIOe7fRn +1tL/h+aRMq7ywOrwbO/s2HQ7kG/jFIRj0QMxeCExTzPhND22EVvVisfocXS91QuL +PjZuIWmKwDo/Vvau +-----END CERTIFICATE----- diff --git a/testdata/b3/server.cert b/testdata/b3/server.cert new file mode 100644 index 000000000..d32ce22e2 --- /dev/null +++ b/testdata/b3/server.cert @@ -0,0 +1,16 @@ +-----BEGIN CERTIFICATE----- +MIICcjCCAVqgAwIBAgIJAOniRvoaT7W9MA0GCSqGSIb3DQEBCwUAMEkxCzAJBgNV +BAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMRMwEQYDVQQKDApHb29nbGUgTExD +MRAwDgYDVQQDDAdGYWtlIENBMB4XDTE5MDUwOTA1NDMzMloXDTE5MDgwNzA1NDMz +MlowIDEeMBwGA1UEAwwVYW1wcGFja2FnZWV4YW1wbGUuY29tMFkwEwYHKoZIzj0C +AQYIKoZIzj0DAQcDQgAEOFjcTTfGjzqRM1fZ+vzaDcOwQcO0enVPHYqe6c5pPEux +3DYBUbPOkLuyzCwUpC+kxu1a3GcvdO4yyVT56GvEfqNRME8wOwYDVR0RBDQwMoIV +YW1wcGFja2FnZWV4YW1wbGUuY29tghl3d3cuYW1wcGFja2FnZWV4YW1wbGUuY29t +MBAGCisGAQQB1nkCARYEAgUAMA0GCSqGSIb3DQEBCwUAA4IBAQA15v5hJEiinNQk +GplWJBYeBSjWMnwfvXqpWYiIw47692+ELBihlmYY+xcAhUbvBty7695LiHRRbJLL +c4QC8EoAePuvrty6vZVZbVc9FUFRKvgQ0n3FzMok/0QTfUNeQbfQBWomxxmlySsD +WwQ3fcRtGRETspcBMrhHorVplSAqAtr41bxe1Wh+RywmJRtG+1/Tp8zU6Py6bkPz +pHkwkkua+ucwhU0xk3Ipe5vZVOSZQofVqDpSuUadrhU6B7fDI+B+nniiyOXplgNx +CVoSy6nrsUb1nAFIDxKxrTrhlHi8oXGR9JvMLf4u+A+0d64SeXkgmYFIc5ftYHlz +BKMgdmFI +-----END CERTIFICATE----- diff --git a/testdata/b3/server.csr b/testdata/b3/server.csr new file mode 100644 index 000000000..6be20ce0c --- /dev/null +++ b/testdata/b3/server.csr @@ -0,0 +1,7 @@ +-----BEGIN CERTIFICATE REQUEST----- +MIHbMIGCAgEAMCAxHjAcBgNVBAMMFWFtcHBhY2thZ2VleGFtcGxlLmNvbTBZMBMG +ByqGSM49AgEGCCqGSM49AwEHA0IABDhY3E03xo86kTNX2fr82g3DsEHDtHp1Tx2K +nunOaTxLsdw2AVGzzpC7sswsFKQvpMbtWtxnL3TuMslU+ehrxH6gADAKBggqhkjO +PQQDAgNIADBFAiEA1msHhDtlGqDfUjKtdZBMzUTVzXkDQXj2Y2ZIBngValYCIHBj +7+mqccm/hu/cpBaPUd9A+hmVaYg8yZe+1c4thfkO +-----END CERTIFICATE REQUEST----- diff --git a/testdata/b3/server.privkey b/testdata/b3/server.privkey new file mode 100644 index 000000000..f4c231dbd --- /dev/null +++ b/testdata/b3/server.privkey @@ -0,0 +1,8 @@ +-----BEGIN EC PARAMETERS----- +BggqhkjOPQMBBw== +-----END EC PARAMETERS----- +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIOI6jTDzviR1kPMJH6ga0AIfjPSbGdwEL/EONwYuiXJjoAoGCCqGSM49 +AwEHoUQDQgAEOFjcTTfGjzqRM1fZ+vzaDcOwQcO0enVPHYqe6c5pPEux3DYBUbPO +kLuyzCwUpC+kxu1a3GcvdO4yyVT56GvEfg== +-----END EC PRIVATE KEY----- diff --git a/testdata/b3/server2.cert b/testdata/b3/server2.cert new file mode 100644 index 000000000..12283d3ed --- /dev/null +++ b/testdata/b3/server2.cert @@ -0,0 +1,16 @@ +-----BEGIN CERTIFICATE----- +MIICdTCCAV2gAwIBAgIJAOniRvoaT7W+MA0GCSqGSIb3DQEBCwUAMEkxCzAJBgNV +BAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMRMwEQYDVQQKDApHb29nbGUgTExD +MRAwDgYDVQQDDAdGYWtlIENBMB4XDTE5MDUwOTA1NDQ1MVoXDTE5MDgwNzA1NDQ1 +MVowITEfMB0GA1UEAwwWYW1wcGFja2FnZWV4YW1wbGUyLmNvbTBZMBMGByqGSM49 +AgEGCCqGSM49AwEHA0IABPrQ/l9HqUGjmupFkJo+dZt6/UoR5sFunT72ZoRc0Nvh +zP+qpvZdTBzkZejExW3xor6iIGowuKKPWOJA976Sj52jUzBRMD0GA1UdEQQ2MDSC +FmFtcHBhY2thZ2VleGFtcGxlMi5jb22CGnd3dy5hbXBwYWNrYWdlZXhhbXBsZTIu +Y29tMBAGCisGAQQB1nkCARYEAgUAMA0GCSqGSIb3DQEBCwUAA4IBAQCmh+Ff+GUC +S/IFh2xLtXcnB+elfr75PwldD91LqRnTYu/DC9tKcfDxKZIwHw0B5gLg362lAdB1 +88I8XJrrf8WfH6auCcuhdoCDyNVbbXqsapwTHEaUFZiCVnPt0eJRdau6voZLKGZM +/zEMVUg9qPOSCR5+j2hpmPyFNCUqK6RpNpCKFXf/WJOHphAIqLeFbLJa4kIfIL6b +m/K2Og93H0uVX3j5jgaQncRxX6ZKCje+2MzNvPBZhWJgU+xTDPLPOPeQX+sHj6k6 +TjsTgQ9GbpHPuoWc0keW6ulVwN7VW5EfUmM3fvftO2/5hHI6qIVnhrTgto+u+7vH +oDSK9AJC357d +-----END CERTIFICATE----- diff --git a/testdata/b3/server2.csr b/testdata/b3/server2.csr new file mode 100644 index 000000000..d6dc5e9f8 --- /dev/null +++ b/testdata/b3/server2.csr @@ -0,0 +1,7 @@ +-----BEGIN CERTIFICATE REQUEST----- +MIHcMIGDAgEAMCExHzAdBgNVBAMMFmFtcHBhY2thZ2VleGFtcGxlMi5jb20wWTAT +BgcqhkjOPQIBBggqhkjOPQMBBwNCAAT60P5fR6lBo5rqRZCaPnWbev1KEebBbp0+ +9maEXNDb4cz/qqb2XUwc5GXoxMVt8aK+oiBqMLiij1jiQPe+ko+doAAwCgYIKoZI +zj0EAwIDSAAwRQIgV4v9X5hsyWaf24G3Uof3vktjEj1k2Pk62ZcUnUoHraACIQCq +kraWMKIDCVvp1qfgsqH+6WUST44x9KD9eYK7/Nrh/A== +-----END CERTIFICATE REQUEST----- diff --git a/testdata/b3/server2.privkey b/testdata/b3/server2.privkey new file mode 100644 index 000000000..2aded130b --- /dev/null +++ b/testdata/b3/server2.privkey @@ -0,0 +1,8 @@ +-----BEGIN EC PARAMETERS----- +BggqhkjOPQMBBw== +-----END EC PARAMETERS----- +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEILV8EeBUFqgQs2J4d0eXP6v8coUYixefDJU/6XY/QWmioAoGCCqGSM49 +AwEHoUQDQgAE+tD+X0epQaOa6kWQmj51m3r9ShHmwW6dPvZmhFzQ2+HM/6qm9l1M +HORl6MTFbfGivqIgajC4oo9Y4kD3vpKPnQ== +-----END EC PRIVATE KEY----- diff --git a/testdata/b3/server_91days.cert b/testdata/b3/server_91days.cert new file mode 100644 index 000000000..14fc6281d --- /dev/null +++ b/testdata/b3/server_91days.cert @@ -0,0 +1,16 @@ +-----BEGIN CERTIFICATE----- +MIICcjCCAVqgAwIBAgIJAOniRvoaT7W/MA0GCSqGSIb3DQEBCwUAMEkxCzAJBgNV +BAYTAlVTMRMwEQYDVQQIDApDYWxpZm9ybmlhMRMwEQYDVQQKDApHb29nbGUgTExD +MRAwDgYDVQQDDAdGYWtlIENBMB4XDTE5MDUwOTA1NDUzNloXDTE5MDgwODA1NDUz +NlowIDEeMBwGA1UEAwwVYW1wcGFja2FnZWV4YW1wbGUuY29tMFkwEwYHKoZIzj0C +AQYIKoZIzj0DAQcDQgAEOFjcTTfGjzqRM1fZ+vzaDcOwQcO0enVPHYqe6c5pPEux +3DYBUbPOkLuyzCwUpC+kxu1a3GcvdO4yyVT56GvEfqNRME8wOwYDVR0RBDQwMoIV +YW1wcGFja2FnZWV4YW1wbGUuY29tghl3d3cuYW1wcGFja2FnZWV4YW1wbGUuY29t +MBAGCisGAQQB1nkCARYEAgUAMA0GCSqGSIb3DQEBCwUAA4IBAQAsXV23u2GWOkXT +HLV/hqmBE/sgcAYYqaUTByptHwP5gua7ARcJ3AfIcMYCLzuIvgEKLSp3KK14Ejhx +ZopyrzbfQNYw+cDUpvUlV1gERL6EULBg6olA8qtaxINRQ1a9VE32+Kyu1Ht8/NMf +VU5x+OW35qPT/LOC2TkY3/KfmSuRF1sYNgnR/fuP3pUi4DhF9Vh1aT3pgLCXbmKz +fpScky+edojNEM8TLM9bVjrKyW4cReJVu4bRq5VHgMAebNM2CYOME7j330fB3VCs +qksBz2Q2j6ThRIpTLYosiqOBEff9NAfsbOsgLuCkbbMewD7TDOh7tMyZfJ/1YLhT +KXTzvbo3 +-----END CERTIFICATE----- diff --git a/testdata/b3/server_p521.privkey b/testdata/b3/server_p521.privkey new file mode 100644 index 000000000..40d584994 --- /dev/null +++ b/testdata/b3/server_p521.privkey @@ -0,0 +1,10 @@ +-----BEGIN EC PARAMETERS----- +BgUrgQQAIw== +-----END EC PARAMETERS----- +-----BEGIN EC PRIVATE KEY----- +MIHbAgEBBEEk8j2XVQhW6I72vTr/RHDL6/Qm2iDzC/Qmpa3KXkO7prBc77v6clJl +IouG25S1fbe0PCxi19nlbA9eqnFwVIDcG6AHBgUrgQQAI6GBiQOBhgAEAP+Q0tpd +6GV2h5h1sXLXz8zIWldhQX/1uwXa1I00ix3Fw7W+R06IJ/gFOZXPeZPoYaVx69K3 +sUs34K/5o54J7zZ+AWjID9t2trjCDZIC5HItffxCtPu+LT1ieRULX+kgVxXKeBmV +A4ZbE4Bp8/JPWOU7b1ZqNZ0ed3gGA71HLyVSYoSm +-----END EC PRIVATE KEY----- diff --git a/transformer/internal/amphtml/amphtml.go b/transformer/internal/amphtml/amphtml.go index f265cec8d..5f6e7b585 100644 --- a/transformer/internal/amphtml/amphtml.go +++ b/transformer/internal/amphtml/amphtml.go @@ -51,6 +51,8 @@ const ( AMPCustomTemplate = "custom-template" + AMPHostService = "host-service" + AMPDynamicCSSClasses = "amp-dynamic-css-classes" AMPExperiment = "amp-experiment" @@ -67,12 +69,25 @@ func IsAMPCustomElement(n *html.Node) bool { return n.Type == html.ElementNode && strings.HasPrefix(n.Data, "amp-") } -// IsScriptAMPExtension returns true if the node is a script tag with either attribute `custom-element` or `custom-template` present. -func IsScriptAMPExtension(n *html.Node) bool { +// AMPExtensionName returns the name of the extension this node represents. For most extensions this is the value of the "custom-element" attribute. Returns ok=false if this isn't an extension. +func AMPExtensionName(n *html.Node) (string, bool) { if n.DataAtom != atom.Script { - return false + return "", false + } + for _, attr := range n.Attr { + for _, k := range []string{AMPCustomElement, AMPCustomTemplate, AMPHostService} { + if attr.Key == k { + return attr.Val, true + } + } } - return htmlnode.HasAttribute(n, "", AMPCustomElement) || htmlnode.HasAttribute(n, "", AMPCustomTemplate) + return "", false +} + +// IsScriptAMPExtension returns true if the node is a script tag representing an extension. +func IsScriptAMPExtension(n *html.Node) bool { + _, ok := AMPExtensionName(n) + return ok } // IsScriptAMPRuntime returns true if the node is of the form @@ -82,8 +97,7 @@ func IsScriptAMPRuntime(n *html.Node) bool { } if v, ok := htmlnode.GetAttributeVal(n, "", "src"); ok { return htmlnode.HasAttribute(n, "", "async") && - !htmlnode.HasAttribute(n, "", AMPCustomElement) && - !htmlnode.HasAttribute(n, "", AMPCustomTemplate) && + !IsScriptAMPExtension(n) && strings.HasPrefix(v, AMPCacheRootURL) && (strings.HasSuffix(v, "/v0.js") || strings.HasSuffix(v, "/amp4ads-v0.js")) @@ -98,12 +112,11 @@ func IsScriptAMPViewer(n *html.Node) bool { } a, ok := htmlnode.FindAttribute(n, "", "src") return ok && - !htmlnode.HasAttribute(n, "", AMPCustomTemplate) && + !IsScriptAMPExtension(n) && strings.HasPrefix(a.Val, AMPCacheSchemeAndHost+"/v0/amp-viewer-integration-") && strings.HasSuffix(a.Val, ".js") && - htmlnode.HasAttribute(n, "", "async") && - !htmlnode.HasAttribute(n, "", AMPCustomElement) + htmlnode.HasAttribute(n, "", "async") } // IsScriptRenderDelaying returns true if the node has one of these values for attribute 'custom-element': amp-dynamic-css-classes, amp-experiment, amp-story. diff --git a/transformer/internal/amphtml/amphtml_test.go b/transformer/internal/amphtml/amphtml_test.go index c1efd9a06..ab00b38ec 100644 --- a/transformer/internal/amphtml/amphtml_test.go +++ b/transformer/internal/amphtml/amphtml_test.go @@ -51,6 +51,85 @@ func TestIsAMPCustomElement(t *testing.T) { } } +func TestIsScriptAmpExtension(t *testing.T) { + tcs := []struct { + desc string + n *html.Node + k string + v string + expected bool + }{ + { + "custom-element true", + &html.Node{Type: html.ElementNode, + Data: "script", + DataAtom: atom.Script, + Attr: []html.Attribute{ + {Key: "custom-element", + Val: "amp-experiment"}}}, + "custom-element", + "amp-experiment", + true, + }, + { + "custom-template true", + &html.Node{Type: html.ElementNode, + Data: "script", + DataAtom: atom.Script, + Attr: []html.Attribute{ + {Key: "custom-template", + Val: "amp-mustache"}}}, + "custom-template", + "amp-mustache", + true, + }, + { + "host-service true", + &html.Node{Type: html.ElementNode, + Data: "script", + DataAtom: atom.Script, + Attr: []html.Attribute{ + {Key: "host-service", + Val: "amp-mraid"}}}, + "host-service", + "amp-mraid", + true, + }, + { + "something else false", + &html.Node{Type: html.ElementNode, + Data: "script", + DataAtom: atom.Script, + Attr: []html.Attribute{ + {Key: "custom-service", + Val: "amp-just-a-random-example"}}}, + "", + "", + false, + }, + { + "not a script false", + &html.Node{Type: html.ElementNode, + Data: "img", + DataAtom: atom.Img, + Attr: []html.Attribute{ + {Key: "custom-element", + Val: "amp-experiment"}}}, + "", + "", + false, + }, + } + for _, tc := range tcs { + if ok := IsScriptAMPExtension(tc.n); ok != tc.expected { + t.Errorf("%s: IsScriptAMPExtension=%t want=%t", tc.desc, ok, tc.expected) + } + if v, ok := AMPExtensionName(tc.n); ok != tc.expected || v != tc.v { + t.Errorf("%s: AMPExtensionName=%s want=%s", tc.desc, v, tc.v) + } + } +} + func TestIsScriptAMPRuntime(t *testing.T) { tcs := []struct { desc string diff --git a/transformer/internal/amphtml/urls.go b/transformer/internal/amphtml/urls.go index 0046c43c1..5782c2ef5 100644 --- a/transformer/internal/amphtml/urls.go +++ b/transformer/internal/amphtml/urls.go @@ -23,6 +23,7 @@ import ( "strconv" "strings" + ampurl "github.com/ampproject/amppackager/internal/url" "github.com/pkg/errors" "golang.org/x/net/idna" ) @@ -34,10 +35,10 @@ func sameURLIgnoringFragment(base string, u *url.URL) bool { // Due to https://github.com/golang/go/issues/29603 we have an extra check // for the empty fragment case. if u.Fragment == "" { - return base == u.String() + return base == ampurl.String(u) } - return base+"#"+u.Fragment == u.String() + return base+"#"+u.Fragment == ampurl.String(u) } // isProtocolRelative is a mostly correct parse for protocol relative inputs @@ -50,11 +51,9 @@ func isProtocolRelative(urlParam string) bool { return strings.HasPrefix(urlParam, "//") } -var queryEncoder = strings.NewReplacer(" ", "+") - // ToAbsoluteURL absolute-ifies |urlParam|, using |baseURL| as the base if // |urlParam| is relative. If |urlParam| contains a fragment, this method -// will return only a fragment if it's absolute URL matches |documentURL|, +// will return only a fragment if its absolute URL matches |documentURL|, // which prevents changing an in-document navigation to a out-of-document // navigation. func ToAbsoluteURL(documentURL string, baseURL *url.URL, @@ -109,12 +108,7 @@ func ToAbsoluteURL(documentURL string, baseURL *url.URL, return "#" + absoluteURL.Fragment } - // Go's URL parser doesn't properly encode query string at parse time. - // See https://github.com/golang/go/issues/22907 . - // This currently only does the bare minimum: - // - encode space to "+" - absoluteURL.RawQuery = queryEncoder.Replace(absoluteURL.RawQuery) - return absoluteURL.String() + return ampurl.String(absoluteURL) } // SubresourceType describes the type of subresource @@ -154,7 +148,7 @@ func (c *CacheURL) OriginDomain() string { // String reassembles the URL into a URL string func (c *CacheURL) String() string { - s := c.URL.String() + s := ampurl.String(c.URL) if len(c.descriptor) > 0 { s = s + " " + c.descriptor } diff --git a/transformer/internal/amphtml/urls_test.go b/transformer/internal/amphtml/urls_test.go index 0a4672174..9950d4d52 100644 --- a/transformer/internal/amphtml/urls_test.go +++ b/transformer/internal/amphtml/urls_test.go @@ -172,11 +172,11 @@ func TestToAbsoluteURL(t *testing.T) { expected: rootURL, }, { - desc: "spaces encoded. '%20' in path, '+' in query string", + desc: "spaces encoded", input: "https://foo.com/i haz spaces?q=i haz spaces", baseURL: rootURL, documentURL: rootURL, - expected: "https://foo.com/i%20haz%20spaces?q=i+haz+spaces", + expected: "https://foo.com/i%20haz%20spaces?q=i%20haz%20spaces", }, { desc: "key only query param", @@ -185,6 +185,13 @@ func TestToAbsoluteURL(t *testing.T) { documentURL: rootURL, expected: "https://foo.com?q", }, + { + desc: "relative", + input: "../blah.jpg", + baseURL: fooURL + "/", + documentURL: rootURL, + expected: "https://www.example.com/blah.jpg", + }, } for _, tc := range tcs { baseURL, _ := url.Parse(tc.baseURL) @@ -274,6 +281,12 @@ func TestGetCacheURL(t *testing.T) { expectedImage: "https://www-example-com.cdn.ampproject.org/i/s/www.example.com/cdn.ampproject.org/blah.jpg", expectedOther: "https://www-example-com.cdn.ampproject.org/r/s/www.example.com/cdn.ampproject.org/blah.jpg", }, + { + desc: "absolute with relative path", + input: "https://www.example.com/cdn.ampproject.org/one/two/three/../../../blah.jpg", + expectedImage: "https://www-example-com.cdn.ampproject.org/i/s/www.example.com/cdn.ampproject.org/blah.jpg", + expectedOther: "https://www-example-com.cdn.ampproject.org/r/s/www.example.com/cdn.ampproject.org/blah.jpg", + }, } base, _ := url.Parse("https://example.com/") for _, tc := range tcs { diff --git a/transformer/internal/htmlnode/htmlnode_test.go b/transformer/internal/htmlnode/htmlnode_test.go index aa4a2e96b..6ddb30727 100644 --- a/transformer/internal/htmlnode/htmlnode_test.go +++ b/transformer/internal/htmlnode/htmlnode_test.go @@ -18,7 +18,6 @@ import ( "strings" "testing" - "github.com/golang/protobuf/proto" "golang.org/x/net/html/atom" "golang.org/x/net/html" ) @@ -43,6 +42,8 @@ const htmltext = ` ` +func stringPtr(v string) *string { return &v } + // Tests a bunch of different methods on the test HTML. func TestHTMLNode(t *testing.T) { doc, err := html.Parse(strings.NewReader(htmltext)) @@ -140,9 +141,9 @@ func TestGetAttributeValOrNil(t *testing.T) { namespace string expected *string }{ - {Element("p", html.Attribute{Namespace: "foo", Key: "id", Val: "A"}), "foo", proto.String("A")}, + {Element("p", html.Attribute{Namespace: "foo", Key: "id", Val: "A"}), "foo", stringPtr("A")}, {Element("p", html.Attribute{Namespace: "foo", Key: "id", Val: "A"}), "differentnamespace", nil}, - {Element("p", html.Attribute{Key: "id", Val: "A"}), "", proto.String("A")}, + {Element("p", html.Attribute{Key: "id", Val: "A"}), "", stringPtr("A")}, {Element("p"), "", nil}, } for _, tc := range testCases { diff --git a/transformer/internal/testing/testing.go b/transformer/internal/testing/testing.go index dad78cde9..a1c46a2c8 100644 --- a/transformer/internal/testing/testing.go +++ b/transformer/internal/testing/testing.go @@ -78,6 +78,9 @@ const ( // ScriptAMPForm is the script for amp-form. ScriptAMPForm = "" + // ScriptAMPMraid is the script for amp-mraid. + ScriptAMPMraid = "" + // ScriptAMPMustache is the script for amp-mustache. ScriptAMPMustache = "" diff --git a/transformer/layout/layout.go b/transformer/layout/layout.go index d7050b5a5..c7487c1ae 100644 --- a/transformer/layout/layout.go +++ b/transformer/layout/layout.go @@ -79,7 +79,7 @@ func ApplyLayout(n *html.Node) error { return nil } - inputLayout := parseAMPLayout(n) + inputLayout := ParseAMPLayout(n) dimensions, err := getNormalizedDimensions(n, inputLayout) if err != nil { return err @@ -97,7 +97,7 @@ func ApplyLayout(n *html.Node) error { // Parses the layout attribute value of the given node and returns the // corresponding AmpLayout_Layout enum. -func parseAMPLayout(n *html.Node) amppb.AmpLayout_Layout { +func ParseAMPLayout(n *html.Node) amppb.AmpLayout_Layout { v, ok := htmlnode.GetAttributeVal(n, "", "layout") if !ok || v == "" { return amppb.AmpLayout_UNKNOWN diff --git a/transformer/request/request.pb.go b/transformer/request/request.pb.go index dc6ddd51b..a521816e1 100644 --- a/transformer/request/request.pb.go +++ b/transformer/request/request.pb.go @@ -1,11 +1,13 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // source: transformer/request/request.proto -package request // import "github.com/ampproject/amppackager/transformer/request" +package request -import proto "github.com/golang/protobuf/proto" -import fmt "fmt" -import math "math" +import ( + fmt "fmt" + proto "github.com/golang/protobuf/proto" + math "math" +) // Reference imports to suppress errors if they are not otherwise used. var _ = proto.Marshal @@ -16,7 +18,7 @@ var _ = math.Inf // is compatible with the proto package it is being compiled against. // A compilation error at this line likely means your copy of the // proto package needs to be updated. -const _ = proto.ProtoPackageIsVersion2 // please upgrade the proto package +const _ = proto.ProtoPackageIsVersion3 // please upgrade the proto package // This should be kept in sync with HtmlFormat.Code in // github.com/ampproject/amphtml/validator/validator.proto. @@ -142,14 +144,15 @@ func (*Request) ProtoMessage() {} func (*Request) Descriptor() ([]byte, []int) { return fileDescriptor_762cce2ac5f73405, []int{0} } + func (m *Request) XXX_Unmarshal(b []byte) error { return xxx_messageInfo_Request.Unmarshal(m, b) } func (m *Request) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { return xxx_messageInfo_Request.Marshal(b, m, deterministic) } -func (dst *Request) XXX_Merge(src proto.Message) { - xxx_messageInfo_Request.Merge(dst, src) +func (m *Request) XXX_Merge(src proto.Message) { + xxx_messageInfo_Request.Merge(m, src) } func (m *Request) XXX_Size() int { return xxx_messageInfo_Request.Size(m) @@ -231,14 +234,15 @@ func (*VersionRange) ProtoMessage() {} func (*VersionRange) Descriptor() ([]byte, []int) { return fileDescriptor_762cce2ac5f73405, []int{1} } + func (m *VersionRange) XXX_Unmarshal(b []byte) error { return xxx_messageInfo_VersionRange.Unmarshal(m, b) } func (m *VersionRange) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { return xxx_messageInfo_VersionRange.Marshal(b, m, deterministic) } -func (dst *VersionRange) XXX_Merge(src proto.Message) { - xxx_messageInfo_VersionRange.Merge(dst, src) +func (m *VersionRange) XXX_Merge(src proto.Message) { + xxx_messageInfo_VersionRange.Merge(m, src) } func (m *VersionRange) XXX_Size() int { return xxx_messageInfo_VersionRange.Size(m) @@ -286,14 +290,15 @@ func (*Metadata) ProtoMessage() {} func (*Metadata) Descriptor() ([]byte, []int) { return fileDescriptor_762cce2ac5f73405, []int{2} } + func (m *Metadata) XXX_Unmarshal(b []byte) error { return xxx_messageInfo_Metadata.Unmarshal(m, b) } func (m *Metadata) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { return xxx_messageInfo_Metadata.Marshal(b, m, deterministic) } -func (dst *Metadata) XXX_Merge(src proto.Message) { - xxx_messageInfo_Metadata.Merge(dst, src) +func (m *Metadata) XXX_Merge(src proto.Message) { + xxx_messageInfo_Metadata.Merge(m, src) } func (m *Metadata) XXX_Size() int { return xxx_messageInfo_Metadata.Size(m) @@ -320,8 +325,11 @@ type Metadata_Preload struct { // https://html.spec.whatwg.org/multipage/semantics.html#attr-link-as. The // full list of potential values is specified in // https://fetch.spec.whatwg.org/#concept-request-destination, though for - // the time being only "script" and "style" are allowed. - As string `protobuf:"bytes,2,opt,name=as,proto3" json:"as,omitempty"` + // the time being only "script", "style" and image are allowed. + As string `protobuf:"bytes,2,opt,name=as,proto3" json:"as,omitempty"` + // The media attribute for image preload link. This attribute is useful + // only for image links. + Media string `protobuf:"bytes,3,opt,name=media,proto3" json:"media,omitempty"` XXX_NoUnkeyedLiteral struct{} `json:"-"` XXX_unrecognized []byte `json:"-"` XXX_sizecache int32 `json:"-"` @@ -333,14 +341,15 @@ func (*Metadata_Preload) ProtoMessage() {} func (*Metadata_Preload) Descriptor() ([]byte, []int) { return fileDescriptor_762cce2ac5f73405, []int{2, 0} } + func (m *Metadata_Preload) XXX_Unmarshal(b []byte) error { return xxx_messageInfo_Metadata_Preload.Unmarshal(m, b) } func (m *Metadata_Preload) XXX_Marshal(b []byte, deterministic bool) ([]byte, error) { return xxx_messageInfo_Metadata_Preload.Marshal(b, m, deterministic) } -func (dst *Metadata_Preload) XXX_Merge(src proto.Message) { - xxx_messageInfo_Metadata_Preload.Merge(dst, src) +func (m *Metadata_Preload) XXX_Merge(src proto.Message) { + xxx_messageInfo_Metadata_Preload.Merge(m, src) } func (m *Metadata_Preload) XXX_Size() int { return xxx_messageInfo_Metadata_Preload.Size(m) @@ -365,48 +374,55 @@ func (m *Metadata_Preload) GetAs() string { return "" } +func (m *Metadata_Preload) GetMedia() string { + if m != nil { + return m.Media + } + return "" +} + func init() { + proto.RegisterEnum("amp.transform.Request_HtmlFormat", Request_HtmlFormat_name, Request_HtmlFormat_value) + proto.RegisterEnum("amp.transform.Request_TransformersConfig", Request_TransformersConfig_name, Request_TransformersConfig_value) proto.RegisterType((*Request)(nil), "amp.transform.Request") proto.RegisterType((*VersionRange)(nil), "amp.transform.VersionRange") proto.RegisterType((*Metadata)(nil), "amp.transform.Metadata") proto.RegisterType((*Metadata_Preload)(nil), "amp.transform.Metadata.Preload") - proto.RegisterEnum("amp.transform.Request_HtmlFormat", Request_HtmlFormat_name, Request_HtmlFormat_value) - proto.RegisterEnum("amp.transform.Request_TransformersConfig", Request_TransformersConfig_name, Request_TransformersConfig_value) } func init() { proto.RegisterFile("transformer/request/request.proto", fileDescriptor_762cce2ac5f73405) } var fileDescriptor_762cce2ac5f73405 = []byte{ - // 481 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x74, 0x52, 0x5d, 0x93, 0xd2, 0x30, - 0x14, 0xdd, 0x12, 0xa4, 0x70, 0x61, 0x31, 0x93, 0xa7, 0x8e, 0x2f, 0x96, 0x3e, 0xd5, 0x71, 0xa6, - 0xcc, 0xa0, 0x8e, 0x0f, 0x3e, 0x55, 0xe8, 0x2a, 0x4a, 0x0b, 0xd3, 0x85, 0xd5, 0xf1, 0x85, 0xc9, - 0x96, 0x2c, 0x8b, 0xb6, 0x0d, 0x26, 0x61, 0xdd, 0xbf, 0xe9, 0x3f, 0x72, 0x92, 0x96, 0x5d, 0xd7, - 0x8f, 0xa7, 0xdc, 0x7b, 0x72, 0x4e, 0x72, 0xe6, 0xde, 0x03, 0x03, 0x25, 0x68, 0x29, 0xaf, 0xb8, - 0x28, 0x98, 0x18, 0x0a, 0xf6, 0xfd, 0xc0, 0xa4, 0x3a, 0x9e, 0xc1, 0x5e, 0x70, 0xc5, 0xc9, 0x29, - 0x2d, 0xf6, 0xc1, 0x1d, 0xcd, 0xfb, 0x89, 0xc0, 0x4e, 0x2b, 0x02, 0x21, 0xd0, 0xbc, 0x56, 0x45, - 0xee, 0x58, 0xae, 0xe5, 0x77, 0x52, 0x53, 0x93, 0x01, 0xf4, 0x36, 0x3c, 0x3b, 0x14, 0xac, 0x54, - 0xeb, 0x83, 0xc8, 0x9d, 0x86, 0xb9, 0xeb, 0x1e, 0xb1, 0x95, 0xc8, 0x09, 0x06, 0x24, 0xd4, 0x8d, - 0xd3, 0x34, 0x37, 0xba, 0xd4, 0x48, 0x26, 0xa5, 0xf3, 0xa8, 0x42, 0x32, 0x29, 0xc9, 0x07, 0x78, - 0x4c, 0xf3, 0x9c, 0xff, 0x60, 0x9b, 0xb5, 0xfe, 0x96, 0x2a, 0xe9, 0xd8, 0x2e, 0xf2, 0xfb, 0xa3, - 0x41, 0xf0, 0xc0, 0x4f, 0x50, 0x7b, 0x09, 0xde, 0xab, 0x22, 0x3f, 0x33, 0xcc, 0xb4, 0x5f, 0x2b, - 0xab, 0x56, 0x92, 0x10, 0x5a, 0x19, 0x2f, 0xaf, 0x76, 0x5b, 0xa7, 0xe5, 0x5a, 0x7e, 0x7f, 0xf4, - 0xec, 0x3f, 0x4f, 0x2c, 0xef, 0x67, 0x21, 0xc7, 0x46, 0x90, 0xd6, 0x42, 0xe2, 0x41, 0xef, 0xb7, - 0x49, 0x49, 0x07, 0xb9, 0xc8, 0xef, 0xa4, 0x0f, 0x30, 0xe2, 0x80, 0x7d, 0xc3, 0x84, 0xdc, 0xf1, - 0xd2, 0x69, 0xbb, 0x96, 0x8f, 0xd2, 0x63, 0xeb, 0xad, 0x00, 0xee, 0xed, 0x11, 0x0c, 0xbd, 0x55, - 0xf2, 0x31, 0x99, 0x7f, 0x4a, 0xd6, 0xe3, 0xf9, 0x24, 0xc2, 0x27, 0xc4, 0x06, 0x14, 0xc6, 0x0b, - 0x6c, 0x91, 0x2e, 0xd8, 0x61, 0xbc, 0x78, 0x19, 0x4e, 0xce, 0x71, 0x83, 0x9c, 0x42, 0x47, 0x37, - 0x51, 0x1c, 0x4e, 0x67, 0x18, 0x69, 0x59, 0xf4, 0x79, 0x11, 0xa5, 0xd3, 0x38, 0x4a, 0x96, 0xe1, - 0x0c, 0x37, 0xbd, 0x77, 0x40, 0xfe, 0xb6, 0xac, 0xdf, 0x98, 0x44, 0x67, 0xe1, 0x6a, 0xb6, 0xc4, - 0x27, 0xa4, 0x0d, 0xcd, 0x64, 0x9e, 0x44, 0xd8, 0x22, 0x7d, 0x80, 0x8b, 0x70, 0x36, 0x9d, 0x84, - 0xcb, 0xe9, 0x3c, 0xc1, 0x0d, 0x02, 0xd0, 0x1a, 0xaf, 0xce, 0x97, 0xf3, 0x18, 0x23, 0x6f, 0x04, - 0xbd, 0x8b, 0xca, 0x6a, 0x4a, 0xcb, 0x2d, 0xd3, 0xeb, 0x28, 0x76, 0xa5, 0x59, 0x2b, 0x4a, 0x75, - 0x69, 0x10, 0x7a, 0x6b, 0x96, 0xa9, 0x11, 0x7a, 0xeb, 0x29, 0x68, 0xc7, 0x4c, 0xd1, 0x0d, 0x55, - 0x94, 0xbc, 0x81, 0xf6, 0x5e, 0xb0, 0x9c, 0xd3, 0x8d, 0x74, 0x2c, 0x17, 0xf9, 0xdd, 0xd1, 0xd3, - 0x3f, 0x46, 0x7c, 0xa4, 0x06, 0x8b, 0x8a, 0x97, 0xde, 0x09, 0x9e, 0x3c, 0x07, 0xbb, 0x06, 0xf5, - 0x2f, 0x3a, 0x32, 0x55, 0x9c, 0x74, 0x49, 0xfa, 0xd0, 0xa0, 0xb2, 0xce, 0x50, 0x83, 0xca, 0xb7, - 0xaf, 0xbf, 0xbc, 0xda, 0xee, 0xd4, 0xf5, 0xe1, 0x32, 0xc8, 0x78, 0x31, 0xa4, 0xc5, 0x7e, 0x2f, - 0xf8, 0x57, 0x96, 0x29, 0x53, 0xd2, 0xec, 0x1b, 0xdd, 0x32, 0x31, 0xfc, 0x47, 0xa6, 0x2f, 0x5b, - 0x26, 0xcc, 0x2f, 0x7e, 0x05, 0x00, 0x00, 0xff, 0xff, 0x3f, 0x5f, 0xd2, 0x0e, 0xf1, 0x02, 0x00, - 0x00, + // 495 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x74, 0x52, 0x4d, 0x73, 0xd3, 0x30, + 0x10, 0xad, 0xa3, 0x34, 0x4e, 0x37, 0x69, 0xd0, 0x68, 0x38, 0x68, 0xb8, 0xe0, 0xf8, 0x64, 0x2e, + 0xce, 0x4c, 0x80, 0xe1, 0xc0, 0xc9, 0x24, 0x2e, 0x04, 0x62, 0x27, 0xe3, 0x26, 0x85, 0xe1, 0x92, + 0x51, 0x1d, 0x35, 0x0d, 0xf8, 0x23, 0x48, 0x4a, 0xe9, 0x6f, 0xe0, 0xdf, 0xf1, 0x8f, 0x18, 0xc9, + 0x4e, 0x4b, 0xf9, 0x38, 0x69, 0xf7, 0xe9, 0x3d, 0xe9, 0xcd, 0xee, 0x83, 0xbe, 0x12, 0xac, 0x90, + 0x57, 0xa5, 0xc8, 0xb9, 0x18, 0x08, 0xfe, 0x6d, 0xcf, 0xa5, 0x3a, 0x9c, 0xfe, 0x4e, 0x94, 0xaa, + 0x24, 0xa7, 0x2c, 0xdf, 0xf9, 0x77, 0x34, 0xf7, 0x27, 0x02, 0x3b, 0xa9, 0x08, 0x84, 0x40, 0xf3, + 0x5a, 0xe5, 0x19, 0xb5, 0x1c, 0xcb, 0x3b, 0x49, 0x4c, 0x4d, 0xfa, 0xd0, 0x5d, 0x97, 0xe9, 0x3e, + 0xe7, 0x85, 0x5a, 0xed, 0x45, 0x46, 0x1b, 0xe6, 0xae, 0x73, 0xc0, 0x96, 0x22, 0x23, 0x18, 0x90, + 0x50, 0x37, 0xb4, 0x69, 0x6e, 0x74, 0xa9, 0x91, 0x54, 0x4a, 0x7a, 0x5c, 0x21, 0xa9, 0x94, 0xe4, + 0x3d, 0x3c, 0x62, 0x59, 0x56, 0x7e, 0xe7, 0xeb, 0x95, 0xfe, 0x96, 0x29, 0x49, 0x6d, 0x07, 0x79, + 0xbd, 0x61, 0xdf, 0x7f, 0xe0, 0xc7, 0xaf, 0xbd, 0xf8, 0xef, 0x54, 0x9e, 0x9d, 0x19, 0x66, 0xd2, + 0xab, 0x95, 0x55, 0x2b, 0x49, 0x00, 0xad, 0xb4, 0x2c, 0xae, 0xb6, 0x1b, 0xda, 0x72, 0x2c, 0xaf, + 0x37, 0x7c, 0xf6, 0x9f, 0x27, 0x16, 0xf7, 0xb3, 0x90, 0x23, 0x23, 0x48, 0x6a, 0x21, 0x71, 0xa1, + 0xfb, 0xdb, 0xa4, 0x24, 0x45, 0x0e, 0xf2, 0x4e, 0x92, 0x07, 0x18, 0xa1, 0x60, 0xdf, 0x70, 0x21, + 0xb7, 0x65, 0x41, 0xdb, 0x8e, 0xe5, 0xa1, 0xe4, 0xd0, 0xba, 0x4b, 0x80, 0x7b, 0x7b, 0x04, 0x43, + 0x77, 0x19, 0x7f, 0x88, 0x67, 0x1f, 0xe3, 0xd5, 0x68, 0x36, 0x0e, 0xf1, 0x11, 0xb1, 0x01, 0x05, + 0xd1, 0x1c, 0x5b, 0xa4, 0x03, 0x76, 0x10, 0xcd, 0x5f, 0x04, 0xe3, 0x73, 0xdc, 0x20, 0xa7, 0x70, + 0xa2, 0x9b, 0x30, 0x0a, 0x26, 0x53, 0x8c, 0xb4, 0x2c, 0xfc, 0x34, 0x0f, 0x93, 0x49, 0x14, 0xc6, + 0x8b, 0x60, 0x8a, 0x9b, 0xee, 0x5b, 0x20, 0x7f, 0x5b, 0xd6, 0x6f, 0x8c, 0xc3, 0xb3, 0x60, 0x39, + 0x5d, 0xe0, 0x23, 0xd2, 0x86, 0x66, 0x3c, 0x8b, 0x43, 0x6c, 0x91, 0x1e, 0xc0, 0x45, 0x30, 0x9d, + 0x8c, 0x83, 0xc5, 0x64, 0x16, 0xe3, 0x06, 0x01, 0x68, 0x8d, 0x96, 0xe7, 0x8b, 0x59, 0x84, 0x91, + 0x3b, 0x84, 0xee, 0x45, 0x65, 0x35, 0x61, 0xc5, 0x86, 0xeb, 0x75, 0xe4, 0xdb, 0xc2, 0xac, 0x15, + 0x25, 0xba, 0x34, 0x08, 0xbb, 0x35, 0xcb, 0xd4, 0x08, 0xbb, 0x75, 0x7f, 0x58, 0xd0, 0x8e, 0xb8, + 0x62, 0x6b, 0xa6, 0x18, 0x79, 0x0d, 0xed, 0x9d, 0xe0, 0x59, 0xc9, 0xd6, 0x92, 0x5a, 0x0e, 0xf2, + 0x3a, 0xc3, 0xa7, 0x7f, 0xcc, 0xf8, 0x40, 0xf5, 0xe7, 0x15, 0x2f, 0xb9, 0x13, 0x3c, 0x09, 0xc0, + 0xae, 0x41, 0xfd, 0x8d, 0xce, 0x4c, 0x95, 0x27, 0x5d, 0x92, 0x1e, 0x34, 0x98, 0xac, 0x43, 0xd4, + 0x60, 0x92, 0x3c, 0x86, 0xe3, 0x9c, 0xaf, 0xb7, 0x8c, 0x22, 0x03, 0x55, 0xcd, 0x9b, 0x57, 0x9f, + 0x5f, 0x6e, 0xb6, 0xea, 0x7a, 0x7f, 0xe9, 0xa7, 0x65, 0x3e, 0x60, 0xf9, 0x6e, 0x27, 0xca, 0x2f, + 0x3c, 0x55, 0xa6, 0x64, 0xe9, 0x57, 0xb6, 0xe1, 0x62, 0xf0, 0x8f, 0xa8, 0x5f, 0xb6, 0x4c, 0xc6, + 0x9f, 0xff, 0x0a, 0x00, 0x00, 0xff, 0xff, 0xce, 0x78, 0xb7, 0x04, 0x08, 0x03, 0x00, 0x00, } diff --git a/transformer/request/request.proto b/transformer/request/request.proto index f1357a590..f14f3ea7b 100644 --- a/transformer/request/request.proto +++ b/transformer/request/request.proto @@ -113,8 +113,12 @@ message Metadata { // https://html.spec.whatwg.org/multipage/semantics.html#attr-link-as. The // full list of potential values is specified in // https://fetch.spec.whatwg.org/#concept-request-destination, though for - // the time being only "script" and "style" are allowed. + // the time being only "script", "style", and "image" are allowed. string as = 2; + + // The media attribute for image preload link. This attribute is useful + // only for image links. + string media = 3; } // Absolute URLs of resources that should be preloaded when the AMP is // prefetched. In a signed exchange (SXG) context, these would be included as diff --git a/transformer/transformer.go b/transformer/transformer.go index f5267dcb3..3dbf9f2c9 100644 --- a/transformer/transformer.go +++ b/transformer/transformer.go @@ -45,6 +45,7 @@ var transformerFunctionMap = map[string]func(*transformers.Context) error{ "linktag": transformers.LinkTag, "metatag": transformers.MetaTag, "nodecleanup": transformers.NodeCleanup, + "preloadimage": transformers.PreloadImage, "reorderhead": transformers.ReorderHead, "serversiderendering": transformers.ServerSideRendering, "stripjs": transformers.StripJS, @@ -69,6 +70,7 @@ var configMap = map[rpb.Request_TransformersConfig][]func(*transformers.Context) transformers.AMPRuntimeCSS, transformers.TransformedIdentifier, transformers.URLRewrite, + transformers.PreloadImage, // ReorderHead should run after all transformers that modify the // , as they may do so without preserving the proper order. transformers.ReorderHead, @@ -224,14 +226,16 @@ func setBaseURL(c *transformers.Context) { // If the requested list of transformers is empty, apply the default. func Process(r *rpb.Request) (string, *rpb.Metadata, error) { context := &transformers.Context{} - var err error - err = setDOM(context, r.Html) - if err != nil { + if err := validateUTF8ForHTML(r.Html); err != nil { return "", nil, err } - if err = requireAMPAttribute(context.DOM, r.AllowedFormats); err != nil { + if err := setDOM(context, r.Html); err != nil { + return "", nil, err + } + + if err := requireAMPAttribute(context.DOM, r.AllowedFormats); err != nil { return "", nil, err } @@ -246,17 +250,19 @@ func Process(r *rpb.Request) (string, *rpb.Metadata, error) { } } - context.DocumentURL, err = url.Parse(r.DocumentUrl) + documentURL, err := url.Parse(r.DocumentUrl) if err != nil { return "", nil, err } + context.DocumentURL = documentURL context.Version = r.Version if r.Version == 0 { - context.Version, err = SelectVersion(nil) + version, err := SelectVersion(nil) if err != nil { return "", nil, err } + context.Version = version } // This must run AFTER DocumentURL is parsed. @@ -266,8 +272,7 @@ func Process(r *rpb.Request) (string, *rpb.Metadata, error) { return "", nil, err } var o strings.Builder - err = printer.Print(&o, context.DOM.RootNode) - if err != nil { + if err := printer.Print(&o, context.DOM.RootNode); err != nil { return "", nil, err } return o.String(), &rpb.Metadata{Preloads: extractPreloads(context.DOM)}, nil diff --git a/transformer/transformer_test.go b/transformer/transformer_test.go index 0e1a8c79b..44f7aabbc 100644 --- a/transformer/transformer_test.go +++ b/transformer/transformer_test.go @@ -26,31 +26,33 @@ func TestProcess(t *testing.T) { config rpb.Request_TransformersConfig expectedLen int }{ - {rpb.Request_DEFAULT, 12}, + {rpb.Request_DEFAULT, 13}, {rpb.Request_NONE, 0}, {rpb.Request_VALIDATION, 1}, {rpb.Request_CUSTOM, 0}, } for _, tc := range tests { - r := rpb.Request{Html: "", Config: tc.config} - html, metadata, err := Process(&r) - if err != nil { - t.Fatalf("Process(%v) unexpectedly failed %v", tc.config, err) - } + t.Run(tc.config.String(), func(t *testing.T) { + r := rpb.Request{Html: "", Config: tc.config} + html, metadata, err := Process(&r) + if err != nil { + t.Fatalf("unexpected failure %v", err) + } - expectedHTML := "" - if html != expectedHTML { - t.Errorf("Process(%v) = %q, want = %q", tc.config, html, expectedHTML) - } + expectedHTML := "" + if html != expectedHTML { + t.Errorf("got = %q, want = %q", html, expectedHTML) + } - if metadata == nil { - t.Errorf("Process(%v) metadata unexpectedly nil", tc.config) - } + if metadata == nil { + t.Error("metadata unexpectedly nil") + } - if len(fns) != tc.expectedLen { - t.Errorf("Process(%v) number of transformers, got=%d, want=%d", tc.config, len(fns), tc.expectedLen) - } + if len(fns) != tc.expectedLen { + t.Errorf("number of transformers, got=%d, want=%d", len(fns), tc.expectedLen) + } + }) } } @@ -66,7 +68,7 @@ func TestPreloads(t *testing.T) { } } - tests := []struct { + tcs := []struct { html string expectedPreloads []*rpb.Metadata_Preload }{ @@ -100,15 +102,17 @@ func TestPreloads(t *testing.T) { }, } - for _, test := range tests { - _, metadata, err := Process(&rpb.Request{Html: test.html, Config: rpb.Request_NONE}) - if err != nil { - t.Fatalf("Process(%q) unexpectedly failed: %v", test.html, err) - } + for _, tc := range tcs { + t.Run(tc.html, func(t *testing.T) { + _, metadata, err := Process(&rpb.Request{Html: tc.html, Config: rpb.Request_NONE}) + if err != nil { + t.Fatalf("unexpected failure: %v", err) + } - if diff := cmp.Diff(test.expectedPreloads, metadata.Preloads); diff != "" { - t.Errorf("Process(%q) preloads differ (-want +got):\n%s", test.html, diff) - } + if diff := cmp.Diff(tc.expectedPreloads, metadata.Preloads); diff != "" { + t.Errorf("preloads differ (-want +got):\n%s", diff) + } + }) } } @@ -150,7 +154,7 @@ func TestCustom(t *testing.T) { } // Case insensitive - tests := []string{ + tcs := []string{ "aMpBoIlerplate", "AMPRuntimeCSS", "linktag", @@ -162,15 +166,17 @@ func TestCustom(t *testing.T) { "aBsolUTEuRl", "urlRewriTE", } - for _, tc := range tests { - r := rpb.Request{Html: "", Config: rpb.Request_CUSTOM, Transformers: []string{tc}} - if _, _, err := Process(&r); err != nil { - t.Fatalf("Process(%v) unexpectedly failed %v", tc, err) - } + for _, tc := range tcs { + t.Run(tc, func(t *testing.T) { + r := rpb.Request{Html: "", Config: rpb.Request_CUSTOM, Transformers: []string{tc}} + if _, _, err := Process(&r); err != nil { + t.Fatalf("unexpected failure %v", err) + } - if len(fns) != 1 { - t.Errorf("Process(%v) expected successful transformer lookup", tc) - } + if len(fns) != 1 { + t.Error("expected successful transformer lookup") + } + }) } } @@ -200,8 +206,27 @@ func TestError(t *testing.T) { } } +func TestInvalidUTF8(t *testing.T) { + tcs := []struct{ html, expectedError string }{ + {"", "character U+0003 at position 13 is not allowed in AMPHTML"}, + {"", "invalid UTF-8 at byte position 13"}, + } + for _, tc := range tcs { + t.Run(tc.html, func(t *testing.T) { + r := rpb.Request{Html: tc.html, Config: rpb.Request_DEFAULT} + _, _, err := Process(&r) + if err == nil { + t.Fatal("unexpected success") + } + if err.Error() != tc.expectedError { + t.Fatalf("mismatched error. got=%s, want=%s", err.Error(), tc.expectedError) + } + }) + } +} + func TestRequireAMPAttribute(t *testing.T) { - tests := []struct { + tcs := []struct { desc string html string expectedError bool @@ -270,36 +295,38 @@ func TestRequireAMPAttribute(t *testing.T) { true, true, true, true, }, } - for _, test := range tests { - r := rpb.Request{Html: test.html, Config: rpb.Request_NONE} - _, _, err := Process(&r) - if (err != nil) != test.expectedError { - t.Errorf("%s: Process() has error=%#v want=%t", test.desc, err, test.expectedError) - } + for _, tc := range tcs { + t.Run(tc.desc, func(t *testing.T) { + r := rpb.Request{Html: tc.html, Config: rpb.Request_NONE} + _, _, err := Process(&r) + if (err != nil) != tc.expectedError { + t.Errorf("Process() has error=%#v want=%t", err, tc.expectedError) + } - r = rpb.Request{Html: test.html, Config: rpb.Request_NONE, AllowedFormats: []rpb.Request_HtmlFormat{rpb.Request_AMP}} - _, _, err = Process(&r) - if (err != nil) != test.expectedErrorInAMP { - t.Errorf("%s: Process(AMP) has error=%#v want=%t", test.desc, err, test.expectedErrorInAMP) - } + r = rpb.Request{Html: tc.html, Config: rpb.Request_NONE, AllowedFormats: []rpb.Request_HtmlFormat{rpb.Request_AMP}} + _, _, err = Process(&r) + if (err != nil) != tc.expectedErrorInAMP { + t.Errorf("Process(AMP) has error=%#v want=%t", err, tc.expectedErrorInAMP) + } - r = rpb.Request{Html: test.html, Config: rpb.Request_NONE, AllowedFormats: []rpb.Request_HtmlFormat{rpb.Request_AMP4ADS}} - _, _, err = Process(&r) - if (err != nil) != test.expectedErrorInAMP4Ads { - t.Errorf("%s: Process(AMP4Ads) has error=%#v want=%t", test.desc, err, test.expectedErrorInAMP4Ads) - } + r = rpb.Request{Html: tc.html, Config: rpb.Request_NONE, AllowedFormats: []rpb.Request_HtmlFormat{rpb.Request_AMP4ADS}} + _, _, err = Process(&r) + if (err != nil) != tc.expectedErrorInAMP4Ads { + t.Errorf("Process(AMP4Ads) has error=%#v want=%t", err, tc.expectedErrorInAMP4Ads) + } - r = rpb.Request{Html: test.html, Config: rpb.Request_NONE, AllowedFormats: []rpb.Request_HtmlFormat{rpb.Request_AMP4EMAIL}} - _, _, err = Process(&r) - if (err != nil) != test.expectedErrorInAMP4Email { - t.Errorf("%s: Process(AMP4Email) has error=%#v want=%t", test.desc, err, test.expectedErrorInAMP4Email) - } + r = rpb.Request{Html: tc.html, Config: rpb.Request_NONE, AllowedFormats: []rpb.Request_HtmlFormat{rpb.Request_AMP4EMAIL}} + _, _, err = Process(&r) + if (err != nil) != tc.expectedErrorInAMP4Email { + t.Errorf("Process(AMP4Email) has error=%#v want=%t", err, tc.expectedErrorInAMP4Email) + } + }) } } func TestBaseURL(t *testing.T) { docURL := "http://example.com/a/page.html" - tests := []struct { + tcs := []struct { desc, base, expected string }{ { @@ -323,20 +350,22 @@ func TestBaseURL(t *testing.T) { "http://example.com/", }, } - for _, test := range tests { - // Remember the original function and reinstate after test - orig := runTransformers - defer func() { runTransformers = orig }() - runTransformers = func(e *transformers.Context, fs []func(*transformers.Context) error) error { - if e.BaseURL.String() != test.expected { - t.Errorf("%s : setBaseURL(%s)=%s, want=%s", test.desc, test.base, e.BaseURL, test.expected) + for _, tc := range tcs { + t.Run(tc.desc, func(t *testing.T) { + // Remember the original function and reinstate after test + orig := runTransformers + defer func() { runTransformers = orig }() + runTransformers = func(e *transformers.Context, fs []func(*transformers.Context) error) error { + if e.BaseURL.String() != tc.expected { + t.Errorf("setBaseURL(%s)=%s, want=%s", tc.base, e.BaseURL, tc.expected) + } + return nil } - return nil - } - r := rpb.Request{Html: "" + test.base + "", DocumentUrl: docURL, Config: rpb.Request_NONE} - _, _, err := Process(&r) - if err != nil { - t.Fatalf("Process(%v) unexpectedly failed %v", test.desc, err) - } + r := rpb.Request{Html: "" + tc.base + "", DocumentUrl: docURL, Config: rpb.Request_NONE} + _, _, err := Process(&r) + if err != nil { + t.Fatalf("unexpected failure %v", err) + } + }) } } diff --git a/transformer/transformers/context.go b/transformer/transformers/context.go index 6c3baca33..2e86257ff 100644 --- a/transformer/transformers/context.go +++ b/transformer/transformers/context.go @@ -21,6 +21,14 @@ import ( rpb "github.com/ampproject/amppackager/transformer/request" ) +// PreloadData stores the links of type script, image and style that are +// added as Link http headers in SXG package. +type PreloadData struct { + URL *url.URL + As string + Media string +} + // Context stores the root DOM Node and contextual data used for the // transformers. type Context struct { @@ -40,4 +48,9 @@ type Context struct { // The request parameters. Request *rpb.Request + + // Preload data. This data gets appended in: + // - head tag. + // - Link: HTTP Header. + Preloads []PreloadData } diff --git a/transformer/transformers/linktag.go b/transformer/transformers/linktag.go index c0f75383d..d97248443 100644 --- a/transformer/transformers/linktag.go +++ b/transformer/transformers/linktag.go @@ -25,6 +25,7 @@ import ( // LinkTag operates on the tag. // * It will add a preconnect link tag for Google Font resources. +// * It will add a preconnect link tag to the publisher's own origin. func LinkTag(e *Context) error { preconnectAdded := false @@ -34,6 +35,8 @@ func LinkTag(e *Context) error { preconnectAdded = true } } + + addLinkPublisherOriginPreconnect(e.DOM.HeadNode, e.DocumentURL) return nil } @@ -59,6 +62,35 @@ func addLinkGoogleFontPreconnect(n *html.Node) { if n.DataAtom != atom.Link { return } - preconnect := htmlnode.Element("link", html.Attribute{Key: "crossorigin"}, html.Attribute{Key: "href", Val: "https://fonts.gstatic.com/"}, html.Attribute{Key: "rel", Val: "dns-prefetch preconnect"}) + preconnect := + htmlnode.Element("link", + html.Attribute{Key: "crossorigin"}, + html.Attribute{Key: "href", Val: "https://fonts.gstatic.com/"}, + html.Attribute{Key: "rel", Val: "dns-prefetch preconnect"}) n.Parent.InsertBefore(preconnect, n) } + +// addLinkPublisherOriginPreconnect adds a preconnect link tag for the +// publisher's own origin. This will only occur once the SXG is fully loaded +// so does not invalidate privacy preserving preload. For publishers that load +// dynamic resources, this will speed up those requests. +func addLinkPublisherOriginPreconnect(n *html.Node, u *url.URL) { + if n.DataAtom != atom.Head { + return + } + // Generates a preconnect value, which does not need anything + // other than the origin to connect to, so to shave some bytes, strip + // everything else. + urlCopy := *u + urlCopy.User = nil + urlCopy.Path = "" + urlCopy.ForceQuery = false + urlCopy.RawQuery = "" + urlCopy.Fragment = "" + + preconnect := + htmlnode.Element("link", + html.Attribute{Key: "href", Val: urlCopy.String()}, + html.Attribute{Key: "rel", Val: "dns-prefetch preconnect"}) + n.AppendChild(preconnect) +} diff --git a/transformer/transformers/linktag_test.go b/transformer/transformers/linktag_test.go index fb0157316..d0be6ee9e 100644 --- a/transformer/transformers/linktag_test.go +++ b/transformer/transformers/linktag_test.go @@ -15,6 +15,7 @@ package transformers_test import ( + "net/url" "strings" "testing" @@ -24,6 +25,11 @@ import ( "golang.org/x/net/html" ) +const ( + PublisherURL = "https://publisher.com/amp-url.html" + LinkGoogleFontPreconnect = "" +) + func TestLinkTag(t *testing.T) { tcs := []tt.TestCase{ { @@ -37,6 +43,7 @@ func TestLinkTag(t *testing.T) { tt.MetaCharset, tt.MetaViewport, tt.ScriptAMPRuntime, tt.LinkFavicon, tt.LinkGoogleFontPreconnect, tt.LinkGoogleFont, tt.LinkCanonical, tt.StyleAMPBoilerplate, tt.NoscriptAMPBoilerplate, + LinkGoogleFontPreconnect, ""), }, { @@ -50,7 +57,7 @@ func TestLinkTag(t *testing.T) { tt.MetaCharset, tt.MetaViewport, tt.ScriptAMPRuntime, tt.LinkFavicon, tt.LinkGoogleFontPreconnect, tt.LinkGoogleFont, tt.LinkGoogleFont, tt.LinkCanonical, tt.StyleAMPBoilerplate, - tt.NoscriptAMPBoilerplate, + tt.NoscriptAMPBoilerplate, LinkGoogleFontPreconnect, ""), }, } @@ -65,7 +72,9 @@ func TestLinkTag(t *testing.T) { t.Errorf("%s\namphtml.NewDOM for %s failed %q", tc.Desc, tc.Input, err) continue } - transformers.LinkTag(&transformers.Context{DOM: inputDOM}) + context := transformers.Context{DOM: inputDOM} + context.DocumentURL, err = url.Parse(PublisherURL) + transformers.LinkTag(&context) var input strings.Builder if err := html.Render(&input, inputDoc); err != nil { diff --git a/transformer/transformers/preloadimage.go b/transformer/transformers/preloadimage.go new file mode 100644 index 000000000..ec3820d28 --- /dev/null +++ b/transformer/transformers/preloadimage.go @@ -0,0 +1,268 @@ +// Copyright 2019 Google LLC +// +// 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 +// +// https://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 transformers + +import ( + "fmt" + "math/big" + "net/url" + "sort" + "strconv" + "strings" + + "github.com/ampproject/amppackager/transformer/internal/htmlnode" + "github.com/ampproject/amppackager/transformer/layout" + "golang.org/x/net/html" + amppb "github.com/ampproject/amphtml/validator" +) + +// Images smaller than 150 pixels are ignored for preloading. +// This number is chosen after manually reviewing 10k samples. +const minImageSize int = 150 + +// For images with aspect ratio, ignores images with aspect ratio larger than this. +const maxAspectRatioSize int = 16 + +// PreloadImage adds link rel="prefetch" head element to preload the most revalent image in the AMP document. +func PreloadImage(e *Context) error { + for n := e.DOM.BodyNode; n != nil; n = htmlnode.Next(n) { + if isNodeHiddenInLayout(n) { + continue + } + + if n.Data == "amp-img" { + if imgsrcset, ok := candidateImageForPreloading(n); ok { + srcsetToPreloadData(imgsrcset, e) + } + } else if n.Data == "amp-video" || n.Data == "amp-video-iframe" { + if poster, ok := candidateVideoPosterImage(n); ok { + posterURL, err := url.Parse(poster) + if err != nil { + continue + } + e.Preloads = append(e.Preloads, PreloadData{URL: posterURL, As: "image"}) + } + } + } + return nil +} + +func candidateVideoPosterImage(i *html.Node) (string, bool) { + poster, hasPoster := htmlnode.GetAttributeVal(i, "", "poster") + if !hasPoster || poster == "" { + return "", false + } + + videoWidth, videoHeight := nodeDimensions(i) + if isTinyNode(videoWidth, videoHeight) { + return "", false + } + + return poster, true +} + +func isNodeHiddenInLayout(n *html.Node) bool { + return layout.ParseAMPLayout(n) == amppb.AmpLayout_NODISPLAY +} + +// Converts the raw srcset attribute value and populates Context.Preloads field. +func srcsetToPreloadData(srcset string, e *Context) { + type imageWithTargetSize struct { + imgURL *url.URL + size int + } + + srcSets := strings.FieldsFunc(strings.TrimSpace(srcset), func(c rune) bool { return c == ',' }) + srcSetsSize := len(srcSets) + imgSet := []imageWithTargetSize{} + + for _, src := range srcSets { + imgComponents := strings.Fields(src) + if len(imgComponents) != 2 { + e.Preloads = nil + return + } + imgTargetSize, err := strconv.Atoi(strings.TrimSuffix(imgComponents[1], "w")) + + if err != nil { + e.Preloads = nil + return + } + + urlObj, err := url.Parse(imgComponents[0]) + + if err != nil { + e.Preloads = nil + return + } + + imgSet = append(imgSet, imageWithTargetSize{urlObj, imgTargetSize}) + } + + // Sort the images based on their target sizes in asc order. + sort.Slice(imgSet, func(i, j int) bool { return imgSet[i].size < imgSet[j].size }) + + for i, ci := range imgSet { + var mediaQuery string + // srcset images should be sorted by width. + if i == 0 { + mediaQuery = fmt.Sprintf("(max-width: %d)", ci.size) + // Largest image has only min width limit of second largest image. + } else if i == srcSetsSize-1 { + mediaQuery = fmt.Sprintf("(min-width: %d)", imgSet[i-1].size+1) + } else { + mediaQuery = fmt.Sprintf("(min-width: %d) and (max-width: %d)", imgSet[i-1].size+1, ci.size) + } + + e.Preloads = append(e.Preloads, PreloadData{URL: ci.imgURL, As: "image", Media: mediaQuery}) + } +} + +// Decides if the given image node qualifies for preloading and returns tuple of +// (imagesrc, true) if the node qualifies for preloading, otherwise returns +// empty string and false. +func candidateImageForPreloading(n *html.Node) (string, bool) { + // amp-image under following containers do not qualify for preloading. + imgsrcset, hasSrcset := htmlnode.GetAttributeVal(n, "", "srcset") + + // Ignores images with no src attribute. + // These can be css images inside class definition. + if !hasSrcset || len(imgsrcset) == 0 { + return "", false + } + + // Ignores if image src is not a https url. + // URL rewrite transformer guarantees img srcs are https protocol. + if !strings.HasPrefix(imgsrcset, "https://") { + return "", false + } + + widthInt, heightInt := nodeDimensions(n) + + // Ignores smaller images, unless they are aspect ratio dimensions. + if isTinyNode(widthInt, heightInt) { + // Checks for aspect ratio images. + // Aspect ratio images larger than maxAspectRatioSize are ignored. + // Small images of icon types inside input type container types + // are ignored. + if widthInt > 0 && widthInt <= maxAspectRatioSize && heightInt > 0 && heightInt <= maxAspectRatioSize && isAspectRatioDimensions(n, widthInt, heightInt) && !containerTypeInput(n) { + return imgsrcset, true + } + return "", false + } + + // Checks if it is placeholder image for iframe. + // https://www.ampproject.org/docs/reference/components/amp-iframe#iframe-with-placeholder + _, hasPlaceholder := htmlnode.GetAttributeVal(n, "", "placeholder") + parentWidthInt, parentHeightInt := nodeDimensions(n.Parent) + if hasPlaceholder { + if n.Parent.Data == "amp-iframe" { + if isTinyNode(parentWidthInt, parentHeightInt) { + return "", false + } + return imgsrcset, true + } + return "", false + } + + layoutType := layout.ParseAMPLayout(n) + // Responsive and fill layout types generally accept parent containers dimensions. + if layoutType == amppb.AmpLayout_RESPONSIVE || layoutType == amppb.AmpLayout_FILL { + if widthInt == 0 && heightInt == 0 { + if isTinyNode(parentWidthInt, parentHeightInt) { + return "", false + } + return imgsrcset, true + } + + // Actual image dimension check is performed later. + } + + // For other layouts with no image dimensions, take parent containers + // dimensions into account. + if widthInt == 0 && heightInt == 0 { + widthInt = parentWidthInt + heightInt = parentHeightInt + } + + // Checks image meets minimum dimension requirements. + // Ignores the width size if it is not specified. In most layouts it + // defaults to auto or 100% size of container. + if (widthInt >= minImageSize || widthInt == 0) && heightInt >= minImageSize { + return imgsrcset, true + } + + return "", false +} + +// Consider a small dimension size as aspect ratio if they are relatively prime. +// TODO(amaltas): Fix it for float dimension types: 1x1.33. +func isAspectRatioDimensions(n *html.Node, width int, height int) bool { + // Aspect ratio doesn't work in fixed layout types. + layoutType := layout.ParseAMPLayout(n) + if !(layoutType == amppb.AmpLayout_FIXED || layoutType == amppb.AmpLayout_FIXED_HEIGHT) { + return false + } + + return new(big.Int).GCD(nil, nil, big.NewInt(int64(width)), big.NewInt(int64(height))).Int64() == 1 +} + +func containerTypeInput(i *html.Node) bool { + switch i.Parent.Data { + case + "button", + "input": + return true + } + return false +} + +// A tiny node is any container of amp-img that is smaller than 150x150. +// Node with no dimensions defaults to 0x0. A 0x0 node is not considered tiny as its dimensions +// defaults to its parent's dimensions. +// A node is small size only when width and height attribute are set and are positive value. +// Caller must check if the container's dimension are aspect ratio dimensions. +func isTinyNode(width, height int) bool { + return (width > 0 && width < minImageSize) || (height > 0 && height < minImageSize) +} + +func dimensionAsInt(d string) (int, error) { + // Remove px suffix. Some publishers treat width/height attribute similar to CSS. + replacer := strings.NewReplacer("px", "", "auto", "0") + return strconv.Atoi(replacer.Replace(d)) +} + +func nodeDimensions(i *html.Node) (int, int) { + var err error + + // Width and Height as int type. + width, hasWidth := htmlnode.GetAttributeVal(i, "", "width") + widthInt := 0 + if hasWidth { + if widthInt, err = dimensionAsInt(width); err != nil { + return 0, 0 + } + } + + height, hasHeight := htmlnode.GetAttributeVal(i, "", "height") + heightInt := 0 + if hasHeight { + if heightInt, err = dimensionAsInt(height); err != nil { + return 0, 0 + } + } + + return widthInt, heightInt +} diff --git a/transformer/transformers/preloadimage_test.go b/transformer/transformers/preloadimage_test.go new file mode 100644 index 000000000..ed95e3d4a --- /dev/null +++ b/transformer/transformers/preloadimage_test.go @@ -0,0 +1,472 @@ +// Copyright 2019 Google LLC +// +// 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 +// +// https://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 transformers_test + +import ( + "net/url" + "strings" + "testing" + + "github.com/ampproject/amppackager/transformer/internal/amphtml" + "github.com/ampproject/amppackager/transformer/transformers" + "golang.org/x/net/html" +) + +func pdata(imgURL string, media string) *transformers.PreloadData { + imgURLObj, err := url.Parse(imgURL) + if err != nil { + return &transformers.PreloadData{} + } + + return &transformers.PreloadData{URL: imgURLObj, Media: media, As: "image"} + +} + +func transformAndOutput(input string) (*transformers.Context, error) { + inputDoc, err := html.Parse(strings.NewReader(input)) + if err != nil { + return nil, err + } + inputDOM, err := amphtml.NewDOM(inputDoc) + if err != nil { + return nil, err + } + baseURL, _ := url.Parse("https://www.example.com") + documentURL, _ := url.Parse("https://www.example.com/foo") + + context := &transformers.Context{ + DOM: inputDOM, + BaseURL: baseURL, + DocumentURL: documentURL, + Preloads: []transformers.PreloadData{}, + } + transformers.PreloadImage(context) + var output strings.Builder + if err := html.Render(&output, inputDoc); err != nil { + return nil, err + } + return context, nil +} + +var testcaseInput = []struct { + testcaseName string + html string + noPrefetchImage bool + preloads []*transformers.PreloadData +}{ + {"DataImageIgnored", + ` + + + + + + + + + `, + true, []*transformers.PreloadData{}, + }, + {"TinyImagesIgnored", + ` + + + + + + + + + `, + true, []*transformers.PreloadData{}, + }, + {"ImagesNoSrcIgnored", + ` + + + + + + + + `, + true, []*transformers.PreloadData{}, + }, + {"ImagesnodisplayLayoutIgnored", + ` + + + + + + + + `, + true, []*transformers.PreloadData{}, + }, + {"PlaceholderForNoIframe", + ` + + + + + + + + + `, + true, []*transformers.PreloadData{}, + }, + {"IframePlaceholderImage", + ` + + + + + + + + + + `, + false, []*transformers.PreloadData{ + pdata("https://cdn.com/obama300.jpg", "(max-width: 300)"), + pdata("https//cdn.com/obama600.jpg", "(min-width: 301) and (max-width: 600)"), + pdata("https://cdn.com/obama1000.jpg", "(min-width: 601)")}, + }, + {"VideoPlaceholderImage", + ` + + + + + + + + `, + false, []*transformers.PreloadData{ + pdata("https://cdn.com/obama-video-img.jpg", "")}, + }, + {"VideoIframePlaceholderImage", + ` + + + + + + + + `, + false, []*transformers.PreloadData{ + pdata("https://cdn.com/trump-video-img.jpg", "")}, + }, + {"VideoIframeButNoPlaceholderImage", + ` + + + + + + + + `, + true, []*transformers.PreloadData{}, + }, + {"VideoButNoPlaceholderImage", + ` + + + + + + + + `, + true, []*transformers.PreloadData{}, + }, + {"NoImageDimensionsParentContainerOK", + ` + + + + + + + + + + `, + false, []*transformers.PreloadData{ + pdata("https://cdn.com/foo-from-parent-container300.jpg", "(max-width: 300)"), + pdata("https://cdn.com/foo-from-parent-container600.jpg", "(min-width: 301)")}, + }, + {"NoImageDimensionsParentContainerSmall", + ` + + + + + + + + + + `, + true, []*transformers.PreloadData{}, + }, + {"ImageSmall", + ` + + + + +
+ +
+ + `, + true, []*transformers.PreloadData{}, + }, + {"ImageNoSizeParentSmall", + ` + + + + +
+ +
+ + + `, + true, []*transformers.PreloadData{}, + }, + {"ImageNoSizeParentSizeOK", + ` + + + + +
+ +
+ + + `, + false, []*transformers.PreloadData{ + pdata("https://cdn.com/foo-from-parent-container300.jpg", "(max-width: 300)"), + pdata("https://cdn.com/foo-from-parent-container600.jpg", "(min-width: 301)")}, + }, + {"FirstCandidateImageSelectedForPreloading", + ` + + + + +
+ +
+
foo
+ + + + `, + false, []*transformers.PreloadData{ + pdata("https://cdn.com/second-candidate-image300.jpg", "(max-width: 300)"), + pdata("https://cdn.com/second-candidate-image600.jpg", "(min-width: 301) and (max-width: 600)"), + pdata("https://cdn.com/second-candidate-image900.jpg", "(min-width: 601)")}, + }, + {"MultipleImagesOnPage", + ` + + + + +
+ +
+
foo
+ + + + + `, + false, []*transformers.PreloadData{ + pdata("https://www.google.com/foo.png", "(max-width: 300)"), + pdata("https://www.google.com/foo600.png", "(min-width: 301) and (max-width: 600)"), + pdata("https://www.google.com/foo1000.png", "(min-width: 601)")}, + }, + {"SrcsetCommaSeparatorInPlaceOfWhitespace", + ` + + + + +
+ +
+
foo
+ + + + + + `, + true, []*transformers.PreloadData{}, + }, + {"SrcsetWhitespaceSeparatorInPlaceOfComma", + ` + + + + +
+ +
+
foo
+ + + + + + `, + true, []*transformers.PreloadData{}, + }, + {"EmptySrcset", + ` + + + + +
+ +
+
foo
+ + + + + `, + true, []*transformers.PreloadData{}, + }, + {"SrcsetWithWhitespaceInUrls", + ` + + + + +
+ +
+
foo
+ + +