mirror of https://github.com/grafana/grafana.git
				
				
				
			Postgres/MySQL/MSSQL: Don't return connection failure details to the client (#32408)
For security reasons, log any SQL connection error details rather than returning it to the client. Fixes #26623 Fixes #22000 Co-authored-by: Arve Knudsen <arve.knudsen@gmail.com>
This commit is contained in:
		
							parent
							
								
									a98fd22e34
								
							
						
					
					
						commit
						b82c510581
					
				|  | @ -6,6 +6,7 @@ import ( | |||
| 	"net/url" | ||||
| 	"regexp" | ||||
| 	"strconv" | ||||
| 	"strings" | ||||
| 
 | ||||
| 	"github.com/grafana/grafana/pkg/setting" | ||||
| 	"github.com/grafana/grafana/pkg/util" | ||||
|  | @ -146,5 +147,12 @@ func (t *mssqlQueryResultTransformer) TransformQueryResult(columnTypes []*sql.Co | |||
| } | ||||
| 
 | ||||
| func (t *mssqlQueryResultTransformer) TransformQueryError(err error) error { | ||||
| 	// go-mssql overrides source error, so we currently match on string
 | ||||
| 	// ref https://github.com/denisenkom/go-mssqldb/blob/045585d74f9069afe2e115b6235eb043c8047043/tds.go#L904
 | ||||
| 	if strings.HasPrefix(strings.ToLower(err.Error()), "unable to open tcp connection with host") { | ||||
| 		t.log.Error("query error", "err", err) | ||||
| 		return sqleng.ErrConnectionFailed | ||||
| 	} | ||||
| 
 | ||||
| 	return err | ||||
| } | ||||
|  |  | |||
|  | @ -10,11 +10,13 @@ import ( | |||
| 
 | ||||
| 	"github.com/grafana/grafana/pkg/components/securejsondata" | ||||
| 	"github.com/grafana/grafana/pkg/components/simplejson" | ||||
| 	"github.com/grafana/grafana/pkg/infra/log" | ||||
| 	"github.com/grafana/grafana/pkg/models" | ||||
| 	"github.com/grafana/grafana/pkg/plugins" | ||||
| 	"github.com/grafana/grafana/pkg/services/sqlstore/sqlutil" | ||||
| 	"github.com/grafana/grafana/pkg/tsdb/sqleng" | ||||
| 	. "github.com/smartystreets/goconvey/convey" | ||||
| 	"github.com/stretchr/testify/assert" | ||||
| 	"xorm.io/xorm" | ||||
| ) | ||||
| 
 | ||||
|  | @ -1137,6 +1139,28 @@ func TestMSSQL(t *testing.T) { | |||
| 	}) | ||||
| } | ||||
| 
 | ||||
| func TestTransformQueryError(t *testing.T) { | ||||
| 	transformer := &mssqlQueryResultTransformer{ | ||||
| 		log: log.New("test"), | ||||
| 	} | ||||
| 
 | ||||
| 	randomErr := fmt.Errorf("random error") | ||||
| 
 | ||||
| 	tests := []struct { | ||||
| 		err         error | ||||
| 		expectedErr error | ||||
| 	}{ | ||||
| 		{err: fmt.Errorf("Unable to open tcp connection with host 'localhost:5000': dial tcp: connection refused"), expectedErr: sqleng.ErrConnectionFailed}, | ||||
| 		{err: fmt.Errorf("unable to open tcp connection with host 'localhost:5000': dial tcp: connection refused"), expectedErr: sqleng.ErrConnectionFailed}, | ||||
| 		{err: randomErr, expectedErr: randomErr}, | ||||
| 	} | ||||
| 
 | ||||
| 	for _, tc := range tests { | ||||
| 		resultErr := transformer.TransformQueryError(tc.err) | ||||
| 		assert.ErrorIs(t, resultErr, tc.expectedErr) | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
| func InitMSSQLTestDB(t *testing.T) *xorm.Engine { | ||||
| 	testDB := sqlutil.MSSQLTestDB() | ||||
| 	x, err := xorm.NewEngine(testDB.DriverName, strings.Replace(testDB.ConnStr, "localhost", | ||||
|  |  | |||
|  | @ -4,8 +4,10 @@ import ( | |||
| 	"container/list" | ||||
| 	"context" | ||||
| 	"database/sql" | ||||
| 	"errors" | ||||
| 	"fmt" | ||||
| 	"math" | ||||
| 	"net" | ||||
| 	"regexp" | ||||
| 	"strconv" | ||||
| 	"strings" | ||||
|  | @ -29,6 +31,8 @@ import ( | |||
| // MetaKeyExecutedQueryString is the key where the executed query should get stored
 | ||||
| const MetaKeyExecutedQueryString = "executedQueryString" | ||||
| 
 | ||||
| var ErrConnectionFailed = errors.New("failed to connect to server - please inspect Grafana server log for details") | ||||
| 
 | ||||
| // SQLMacroEngine interpolates macros into sql. It takes in the Query to have access to query context and
 | ||||
| // timeRange to be able to generate queries that use from and to.
 | ||||
| type SQLMacroEngine interface { | ||||
|  | @ -184,7 +188,7 @@ func (e *dataPlugin) DataQuery(ctx context.Context, dsInfo *models.DataSource, | |||
| 
 | ||||
| 			rows, err := db.Query(rawSQL) | ||||
| 			if err != nil { | ||||
| 				queryResult.Error = e.queryResultTransformer.TransformQueryError(err) | ||||
| 				queryResult.Error = e.transformQueryError(err) | ||||
| 				ch <- queryResult | ||||
| 				return | ||||
| 			} | ||||
|  | @ -431,6 +435,20 @@ func (e *dataPlugin) transformToTimeSeries(query plugins.DataSubQuery, rows *cor | |||
| 	return nil | ||||
| } | ||||
| 
 | ||||
| func (e *dataPlugin) transformQueryError(err error) error { | ||||
| 	// OpError is the error type usually returned by functions in the net
 | ||||
| 	// package. It describes the operation, network type, and address of
 | ||||
| 	// an error. We log this error rather than returing it to the client
 | ||||
| 	// for security purposes.
 | ||||
| 	var opErr *net.OpError | ||||
| 	if errors.As(err, &opErr) { | ||||
| 		e.log.Error("query error", "err", err) | ||||
| 		return ErrConnectionFailed | ||||
| 	} | ||||
| 
 | ||||
| 	return e.queryResultTransformer.TransformQueryError(err) | ||||
| } | ||||
| 
 | ||||
| type processCfg struct { | ||||
| 	rowCount           int | ||||
| 	columnTypes        []*sql.ColumnType | ||||
|  |  | |||
|  | @ -1,15 +1,20 @@ | |||
| package sqleng | ||||
| 
 | ||||
| import ( | ||||
| 	"database/sql" | ||||
| 	"fmt" | ||||
| 	"net" | ||||
| 	"testing" | ||||
| 	"time" | ||||
| 
 | ||||
| 	"github.com/grafana/grafana/pkg/components/null" | ||||
| 	"github.com/grafana/grafana/pkg/components/simplejson" | ||||
| 	"github.com/grafana/grafana/pkg/infra/log" | ||||
| 	"github.com/grafana/grafana/pkg/models" | ||||
| 	"github.com/grafana/grafana/pkg/plugins" | ||||
| 	"github.com/stretchr/testify/assert" | ||||
| 	"github.com/stretchr/testify/require" | ||||
| 	"xorm.io/core" | ||||
| ) | ||||
| 
 | ||||
| func TestSQLEngine(t *testing.T) { | ||||
|  | @ -309,4 +314,41 @@ func TestSQLEngine(t *testing.T) { | |||
| 			}) | ||||
| 		} | ||||
| 	}) | ||||
| 
 | ||||
| 	t.Run("Should handle connection errors", func(t *testing.T) { | ||||
| 		randomErr := fmt.Errorf("random error") | ||||
| 
 | ||||
| 		tests := []struct { | ||||
| 			err                                   error | ||||
| 			expectedErr                           error | ||||
| 			expectQueryResultTransformerWasCalled bool | ||||
| 		}{ | ||||
| 			{err: &net.OpError{Op: "Dial"}, expectedErr: ErrConnectionFailed, expectQueryResultTransformerWasCalled: false}, | ||||
| 			{err: randomErr, expectedErr: randomErr, expectQueryResultTransformerWasCalled: true}, | ||||
| 		} | ||||
| 
 | ||||
| 		for _, tc := range tests { | ||||
| 			transformer := &testQueryResultTransformer{} | ||||
| 			dp := dataPlugin{ | ||||
| 				log:                    log.New("test"), | ||||
| 				queryResultTransformer: transformer, | ||||
| 			} | ||||
| 			resultErr := dp.transformQueryError(tc.err) | ||||
| 			assert.ErrorIs(t, resultErr, tc.expectedErr) | ||||
| 			assert.Equal(t, tc.expectQueryResultTransformerWasCalled, transformer.transformQueryErrorWasCalled) | ||||
| 		} | ||||
| 	}) | ||||
| } | ||||
| 
 | ||||
| type testQueryResultTransformer struct { | ||||
| 	transformQueryErrorWasCalled bool | ||||
| } | ||||
| 
 | ||||
| func (t *testQueryResultTransformer) TransformQueryResult(columnTypes []*sql.ColumnType, rows *core.Rows) (plugins.DataRowValues, error) { | ||||
| 	return nil, nil | ||||
| } | ||||
| 
 | ||||
| func (t *testQueryResultTransformer) TransformQueryError(err error) error { | ||||
| 	t.transformQueryErrorWasCalled = true | ||||
| 	return err | ||||
| } | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue