Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/text/secure/precis: add ability to construct restricted profile from existing profile #24885

Closed
SamWhited opened this issue Apr 16, 2018 · 4 comments

Comments

@SamWhited
Copy link
Member

SamWhited commented Apr 16, 2018

The RFCs mention that PRECIS profiles may further restrict valid characters beyond what is specified in the underlying string class (RFC 8264 §6.2):

An application protocol that uses a profile MAY specify particular
code points that are not allowed in relevant slots within that
application protocol, above and beyond those excluded by the string
class or profile.

Currently this has to be done outside of the precis package, eg. by scanning the string for invalid characters after applying the profile, or by creating a new profile which matches one of the internally defined ones and adding a Disallow rule.

To make this easier and less fragile, I would like to add a new profile constructor which either specifically sets the disallow option, or a more generic constructor that lets you build a profile by overriding options on an existing profile. I'm leaning towards the first one to avoid profile proliferation (we probably don't want to encourage authors to make up their own profiles if possible, the existing ones are generally preferable):

// NewRestrictedProfile creates a new PRECIS profile based on an existing
// profile but with an additional set of runes disallowed.
// If the existing profile already had the Disallow option set, the new rule
// overrides the parents rule.
func NewRestrictedProfile(parent *Profile, disallowed runes.Set) *Profile { /* … */ }

// NewProfile creates a new PRECIS profile based on an existing profile.
// Any options provided override options from the parent profile.
func NewProfile(parent *Profile, opts ...Option) *Profile { /* … */ }

For example:

MyURLParamProfile = precis.NewRestrictedProfile(precis.UsernameCaseMapped, runes.Predicate(func(r rune) bool {
    return strings.ContainsRune(`%&;`, r)
})

// or:

MyURLParamProfile = precis.NewProfile(precis.UsernameCaseMapped, precis.Disallow(runes.Predicate(func(r rune) bool {
    return strings.ContainsRune(`%&;`, r)
}))

/cc @mpvl, @nigeltao

@nigeltao
Copy link
Contributor

NewRestrictedProfile(parent *Profile, disallowed runes.Set) seems OK to me.

I'll let @mpvl have the final say. I haven't really had much time for Go x/text stuff lately. Sorry.

@SamWhited
Copy link
Member Author

I'll go ahead and make a CL then and ping you both on it. If it turns out that this isn't something MPVL wants then I can do the other one or abandon the CL.

@gopherbot
Copy link

Change https://golang.org/cl/113816 mentions this issue: secure/precis: add ability to restrict profiles

@gopherbot
Copy link

Change https://golang.org/cl/121359 mentions this issue: secure/precis: add missing strings, x/text/runes imports for 1.9

gopherbot pushed a commit to golang/text that referenced this issue Jun 29, 2018
This is a followup to CL 113816. That CL added identical new code to
enforce10.0.0_test.go and enforce9.0.0_test.go, but didn't import new
packages in the latter file. This CL fixes that. As a result, package
tests now pass in Go 1.9.

Updates golang/go#24885.

Change-Id: I0286d873b7f3239bce1a151570c4f9c7ac0219c1
Reviewed-on: https://go-review.googlesource.com/121359
Reviewed-by: Sam Whited <sam@samwhited.com>
Reviewed-by: Dominik Honnef <dominik@honnef.co>
@golang golang locked and limited conversation to collaborators Jun 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants