Skip to content

Commit

Permalink
fix: Use JSONSchema to verify the original data submitted by users (#986
Browse files Browse the repository at this point in the history
)

* fix: JSONSchema verification should be performed on the original data submitted by the user

* fix: remove debug info

* test: add test cases

* fix: error info

* fix: script data type

* fix: script data type

* fix: error

* fix: error message

* fix: CI failed

* fix: error

* fix: according to review

* fix: e2e test case

* fix: according to review
  • Loading branch information
nic-chen authored Dec 11, 2020
1 parent f938c0c commit 05f1381
Show file tree
Hide file tree
Showing 7 changed files with 343 additions and 4 deletions.
195 changes: 195 additions & 0 deletions api/filter/schema.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,195 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package filter

import (
"bytes"
"crypto/tls"
"crypto/x509"
"encoding/json"
"encoding/pem"
"errors"
"fmt"
"io/ioutil"
"net/http"
"strings"

"github.com/gin-gonic/gin"

"github.com/apisix/manager-api/internal/core/store"
"github.com/apisix/manager-api/internal/utils/consts"
"github.com/apisix/manager-api/log"
)

var resources = map[string]string{
"routes": "route",
"upstreams": "upstream",
"services": "service",
"consumers": "consumer",
"ssl": "ssl",
}

func parseCert(crt, key string) ([]string, error) {
if crt == "" || key == "" {
return nil, errors.New("invalid certificate")
}

certDERBlock, _ := pem.Decode([]byte(crt))
if certDERBlock == nil {
return nil, errors.New("Certificate resolution failed")
}
// match
_, err := tls.X509KeyPair([]byte(crt), []byte(key))
if err != nil {
return nil, errors.New("key and cert don't match")
}

x509Cert, err := x509.ParseCertificate(certDERBlock.Bytes)

if err != nil {
return nil, errors.New("Certificate resolution failed")
}

//domain
snis := []string{}
if x509Cert.DNSNames != nil && len(x509Cert.DNSNames) > 0 {
snis = x509Cert.DNSNames
} else if x509Cert.IPAddresses != nil && len(x509Cert.IPAddresses) > 0 {
for _, ip := range x509Cert.IPAddresses {
snis = append(snis, ip.String())
}
} else {
if x509Cert.Subject.Names != nil && len(x509Cert.Subject.Names) > 0 {
var attributeTypeNames = map[string]string{
"2.5.4.6": "C",
"2.5.4.10": "O",
"2.5.4.11": "OU",
"2.5.4.3": "CN",
"2.5.4.5": "SERIALNUMBER",
"2.5.4.7": "L",
"2.5.4.8": "ST",
"2.5.4.9": "STREET",
"2.5.4.17": "POSTALCODE",
}
for _, tv := range x509Cert.Subject.Names {
oidString := tv.Type.String()
typeName, ok := attributeTypeNames[oidString]
if ok && typeName == "CN" {
valueString := fmt.Sprint(tv.Value)
snis = append(snis, valueString)
}
}
}
}

return snis, nil
}

func handleSpecialField(resource string, reqBody []byte) ([]byte, error) {
// remove script, because it's a map, and need to be parsed into lua code
if resource == "routes" {
var route map[string]interface{}
err := json.Unmarshal(reqBody, &route)
if err != nil {
return nil, fmt.Errorf("read request body failed: %s", err)
}
if _, ok := route["script"]; ok {
delete(route, "script")
reqBody, err = json.Marshal(route)
if err != nil {
return nil, fmt.Errorf("read request body failed: %s", err)
}
}
}

// SSL does not need to pass sni, we need to parse the SSL to get sni
if resource == "ssl" {
var ssl map[string]interface{}
err := json.Unmarshal(reqBody, &ssl)
if err != nil {
return nil, fmt.Errorf("read request body failed: %s", err)
}
ssl["snis"], err = parseCert(ssl["cert"].(string), ssl["key"].(string))
if err != nil {
return nil, fmt.Errorf("SSL parse failed: %s", err)
}
reqBody, err = json.Marshal(ssl)
if err != nil {
return nil, fmt.Errorf("read request body failed: %s", err)
}
}

return reqBody, nil
}

func SchemaCheck() gin.HandlerFunc {
return func(c *gin.Context) {
pathPrefix := "/apisix/admin/"
resource := strings.TrimPrefix(c.Request.URL.Path, pathPrefix)

idx := strings.LastIndex(resource, "/")
if idx > 1 {
resource = resource[:idx]
}
method := strings.ToUpper(c.Request.Method)

if method != "PUT" && method != "POST" {
c.Next()
return
}
schemaKey, ok := resources[resource]
if !ok {
c.Next()
return
}

reqBody, err := c.GetRawData()
if err != nil {
log.Errorf("read request body failed: %s", err)
c.AbortWithStatusJSON(http.StatusBadRequest, consts.ErrInvalidRequest)
return
}

// other filter need it
c.Request.Body = ioutil.NopCloser(bytes.NewBuffer(reqBody))

validator, err := store.NewAPISIXSchemaValidator("main." + schemaKey)
if err != nil {
errMsg := err.Error()
c.AbortWithStatusJSON(http.StatusBadRequest, consts.InvalidParam(errMsg))
log.Error(errMsg)
return
}

reqBody, err = handleSpecialField(resource, reqBody)
if err != nil {
errMsg := err.Error()
c.AbortWithStatusJSON(http.StatusBadRequest, consts.InvalidParam(errMsg))
log.Error(errMsg)
return
}

if err := validator.Validate(reqBody); err != nil {
errMsg := err.Error()
c.AbortWithStatusJSON(http.StatusBadRequest, consts.InvalidParam(errMsg))
log.Warn(errMsg)
return
}

c.Next()
}
}
42 changes: 42 additions & 0 deletions api/internal/core/store/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,3 +312,45 @@ func (v *APISIXJsonSchemaValidator) Validate(obj interface{}) error {

return nil
}

type APISIXSchemaValidator struct {
schema *gojsonschema.Schema
}

func NewAPISIXSchemaValidator(jsonPath string) (Validator, error) {
schemaDef := conf.Schema.Get(jsonPath).String()
if schemaDef == "" {
log.Warnf("schema validate failed: schema not found, path: %s", jsonPath)
return nil, fmt.Errorf("schema validate failed: schema not found, path: %s", jsonPath)
}

s, err := gojsonschema.NewSchema(gojsonschema.NewStringLoader(schemaDef))
if err != nil {
log.Warnf("new schema failed: %w", err)
return nil, fmt.Errorf("new schema failed: %w", err)
}
return &APISIXSchemaValidator{
schema: s,
}, nil
}

func (v *APISIXSchemaValidator) Validate(obj interface{}) error {
ret, err := v.schema.Validate(gojsonschema.NewBytesLoader(obj.([]byte)))
if err != nil {
log.Warnf("schema validate failed: %w", err)
return fmt.Errorf("schema validate failed: %w", err)
}

if !ret.Valid() {
errString := buffer.Buffer{}
for i, vErr := range ret.Errors() {
if i != 0 {
errString.AppendString("\n")
}
errString.AppendString(vErr.String())
}
return fmt.Errorf("schema validate failed: %s", errString.String())
}

return nil
}
41 changes: 41 additions & 0 deletions api/internal/core/store/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,3 +437,44 @@ func TestAPISIXJsonSchemaValidator_Route_checkRemoteAddr(t *testing.T) {
assert.Equal(t, tc.wantValidateErr, err, tc.caseDesc)
}
}

func TestAPISIXSchemaValidator_Validate(t *testing.T) {
validator, err := NewAPISIXSchemaValidator("main.consumer")
assert.Nil(t, err)

// normal config, should pass
reqBody := `{
"id": "jack",
"username": "jack",
"plugins": {
"limit-count": {
"count": 2,
"time_window": 60,
"rejected_code": 503,
"key": "remote_addr"
}
},
"desc": "test description"
}`
err = validator.Validate([]byte(reqBody))
assert.Nil(t, err)

// config with non existent field, should be failed.
reqBody = `{
"username": "jack",
"not-exist": "val",
"plugins": {
"limit-count": {
"count": 2,
"time_window": 60,
"rejected_code": 503,
"key": "remote_addr"
}
},
"desc": "test description"
}`
err = validator.Validate([]byte(reqBody))
assert.NotNil(t, err)
assert.EqualError(t, err, "schema validate failed: (root): Additional property not-exist is not allowed")

}
2 changes: 1 addition & 1 deletion api/internal/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func SetUpRouter() *gin.Engine {
logger := log.GetLogger(log.AccessLog)
store := cookie.NewStore([]byte("secret"))
r.Use(sessions.Sessions("session", store))
r.Use(filter.CORS(), filter.RequestId(), filter.RequestLogHandler(logger), filter.Authentication(), filter.RecoverHandler())
r.Use(filter.CORS(), filter.RequestId(), filter.RequestLogHandler(logger), filter.SchemaCheck(), filter.Authentication(), filter.RecoverHandler())
r.Use(static.Serve("/", static.LocalFile(conf.WebDir, false)))
r.NoRoute(func(c *gin.Context) {
c.File(fmt.Sprintf("%s/index.html", conf.WebDir))
Expand Down
2 changes: 1 addition & 1 deletion api/internal/utils/consts/api_error.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func (err ApiError) Error() string {
}

func InvalidParam(message string) *ApiError {
return &ApiError{400, 400, message}
return &ApiError{400, 10000, message}
}

func SystemError(message string) *ApiError {
Expand Down
5 changes: 3 additions & 2 deletions api/internal/utils/consts/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,11 @@ package consts
import "github.com/shiningrush/droplet/data"

const (
ErrCodeDemoBiz = 20000
ErrBadRequest = 20001
)

var (
// base error please refer to github.com/shiningrush/droplet/data, such as data.ErrNotFound, data.ErrConflicted
ErrDemoBiz = data.BaseError{Code: ErrCodeDemoBiz, Message: "this is just a demo error"}
ErrInvalidRequest = data.BaseError{Code: ErrBadRequest, Message: "invalid request"}
ErrSchemaValidateFailed = data.BaseError{Code: ErrBadRequest, Message: "JSONSchema validate failed"}
)
60 changes: 60 additions & 0 deletions api/test/e2e/json_schema_validate_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one or more
* contributor license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright ownership.
* The ASF licenses this file to You under the Apache License, Version 2.0
* (the "License"); you may not use this file except in compliance with
* the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package e2e

import (
"net/http"
"testing"
)

func TestSchema_not_exist_field(t *testing.T) {
tests := []HttpTestCase{
{
caseDesc: "config route with non-existent fields",
Object: ManagerApiExpect(t),
Path: "/apisix/admin/routes/r1",
Method: http.MethodPut,
Body: `{
"uri": "/hello",
"nonexistent": "test non-existent",
"upstream": {
"type": "roundrobin",
"nodes": [{
"host": "172.16.238.20",
"port": 1980,
"weight": 1
}]
}
}`,
Headers: map[string]string{"Authorization": token},
ExpectStatus: http.StatusBadRequest,
ExpectBody: `{"code":10000,"message":"schema validate failed: (root): Additional property nonexistent is not allowed"}`,
},
{
caseDesc: "make sure the route create failed",
Object: APISIXExpect(t),
Method: http.MethodGet,
Path: "/hello",
ExpectStatus: http.StatusNotFound,
Sleep: sleepTime,
},
}

for _, tc := range tests {
testCaseCheck(tc)
}
}

0 comments on commit 05f1381

Please sign in to comment.