From 9b0201719493fe0612ec811f690ce86536774288 Mon Sep 17 00:00:00 2001 From: Behnam Momeni Date: Thu, 8 Feb 2024 13:13:37 +0330 Subject: [PATCH] Fix scram.*Client.Step so it returns an "ok" flag consistently The package documentation asks to call Step method as while as it returns true, so it has to return false only on the first error or if all expected steps were taken previously. This condition was implemented correctly in the first if-condition where it returned false when there were no more steps or an error was detected. However, the ultimate return statement and the method godoc used to return false as while as the Step should be called again. This commit resolves this inconsistency and updates the relevant godocs. The RFC 5802 example test case is added too. --- scram/rfc5802_consts_test.go | 50 ++++++++++++++++++++++++++++++++++++ scram/scram.go | 31 ++++++++++------------ scram/scram_test.go | 35 +++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 17 deletions(-) create mode 100644 scram/rfc5802_consts_test.go create mode 100644 scram/scram_test.go diff --git a/scram/rfc5802_consts_test.go b/scram/rfc5802_consts_test.go new file mode 100644 index 000000000..1963d7804 --- /dev/null +++ b/scram/rfc5802_consts_test.go @@ -0,0 +1,50 @@ +// Copyright (c) 2010 IETF Trust and the persons identified as the +// document authors. All rights reserved. Redistribution and use in source +// and binary forms, with or without modification, are permitted provided +// that the following conditions are met: +// +// - Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// - Redistributions in binary form must reproduce the above copyright +// notice, this list of conditions and the following disclaimer in +// the documentation and/or other materials provided with the +// distribution. +// - Neither the name of Internet Society, IETF or IETF Trust, nor the +// names of specific contributors, may be used to endorse or promote +// products derived from this software without specific prior +// written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// “AS IS” AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS +// FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE +// COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, +// INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, +// BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS +// OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED +// AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +// OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF +// THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH +// DAMAGE. + +package scram_test + +// Following values are extracted from section 5 of RFC 5802, +// available at https://datatracker.ietf.org/doc/html/rfc5802#section-5, +// therefore, the rfc5802_consts_test.go file is licensed under +// the Revised BSD License (included at the top of this file). +// +// Other files are licensed under MIT license as usual in the pq module. +var ( + rfc5802User = "user" + rfc5802Pass = "pencil" + rfc5802Nonce = []byte("fyko+d2lbbFgONRv9qkxdawL") + rfc5802ClientMsgs = []string{ + "n,,n=user,r=fyko+d2lbbFgONRv9qkxdawL", + "c=biws,r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,p=v0X8v3Bz2T0CJGbJQyF0X+HI4Ts=", + } + rfc5802ServerMsgs = [][]byte{ + []byte("r=fyko+d2lbbFgONRv9qkxdawL3rfcNHYJY1ZVvWVs7j,s=QSXCR+Q6sek8bf92,i=4096"), + []byte("v=rmF9pqV8S7suAoZWja4dJRkFsKQ="), + } +) diff --git a/scram/scram.go b/scram/scram.go index 477216b60..9c78b6bcc 100644 --- a/scram/scram.go +++ b/scram/scram.go @@ -25,7 +25,6 @@ // Package scram implements a SCRAM-{SHA-1,etc} client per RFC5802. // // http://tools.ietf.org/html/rfc5802 -// package scram import ( @@ -43,17 +42,16 @@ import ( // // A Client may be used within a SASL conversation with logic resembling: // -// var in []byte -// var client = scram.NewClient(sha1.New, user, pass) -// for client.Step(in) { -// out := client.Out() -// // send out to server -// in := serverOut -// } -// if client.Err() != nil { -// // auth failed -// } -// +// var in []byte // first step requires no input +// var client = scram.NewClient(sha256.New, user, pass) +// for client.Step(in) { // stop on 1st error or after 3rd step +// out := client.Out() +// // send out to server +// in = serverOut +// } +// if client.Err() != nil { +// // auth failed +// } type Client struct { newHash func() hash.Hash @@ -73,8 +71,7 @@ type Client struct { // // For SCRAM-SHA-256, for example, use: // -// client := scram.NewClient(sha256.New, user, pass) -// +// client := scram.NewClient(sha256.New, user, pass) func NewClient(newHash func() hash.Hash, user, pass string) *Client { c := &Client{ newHash: newHash, @@ -109,8 +106,8 @@ var escaper = strings.NewReplacer("=", "=3D", ",", "=2C") // Step processes the incoming data from the server and makes the // next round of data for the server available via Client.Out. -// Step returns false if there are no errors and more data is -// still expected. +// Step returns true if there are no errors and Step should be called +// again with upcoming data messages. func (c *Client) Step(in []byte) bool { c.out.Reset() if c.step > 2 || c.err != nil { @@ -125,7 +122,7 @@ func (c *Client) Step(in []byte) bool { case 3: c.err = c.step3(in) } - return c.step > 2 || c.err != nil + return c.step <= 2 && c.err == nil } func (c *Client) step1(in []byte) error { diff --git a/scram/scram_test.go b/scram/scram_test.go new file mode 100644 index 000000000..521097cca --- /dev/null +++ b/scram/scram_test.go @@ -0,0 +1,35 @@ +// Package scram_test provides functional tests for the scram package. +package scram_test + +import ( + "crypto/sha1" + "testing" + + "github.com/lib/pq/scram" +) + +func TestScramSHA1(t *testing.T) { + s := 0 + var in []byte // first step requires no input + var client = scram.NewClient(sha1.New, rfc5802User, rfc5802Pass) + client.SetNonce(rfc5802Nonce) + for client.Step(in) { + if s >= 2 { + t.Fatal("Step didn't stop after 3rd step") + } + out := client.Out() + if cm := rfc5802ClientMsgs[s]; cm != string(out) { + t.Fatalf( + `Step: %d +Expected message: %q +Actual message: %q`, + s+1, cm, out, + ) + } + in = rfc5802ServerMsgs[s] + s++ + } + if err := client.Err(); err != nil { + t.Fatalf("Unexpected error: %v", err) + } +}