From 90e15ebe0f773c7f61e2f3f4979e68bb8e89f1db Mon Sep 17 00:00:00 2001 From: dev Date: Thu, 1 Oct 2026 21:59:29 +0200 Subject: [PATCH] Review fixes: addresses, safe integers, the note on screen, -expect-author, author keys CheckURI works on the raw authority, as an HTTP client reads it: no percent signs, userinfo or backslashes, a host of letters, digits and hyphens or a public IP literal, a port from 1 to 65535, and Host returns that host. The integers of the locator stop at 2^53 - 1, and Info.Extension refuses what ParseInfo would. extension.Standard validates datekeys.capsule through locator.Standard, and Info.OpenLocator ties the locator to the round of its own DateKey. inspect shows the public note as text of the creator, with its prefix and wrapping and the warning, and says when a note is unusable. encrypt -note warns that it is public. decrypt -expect-author fails before the release is requested when the capsule is not format 3. Author key files are read with the work factor of the spec as their maximum, the passphrase is not read from a terminal, and two copies of secrets are cleared. Co-Authored-By: Claude Sonnet 5.5 --- authorkey/authorkey.go | 5 +- cmd/datekeys/author.go | 13 ++- cmd/datekeys/author_test.go | 3 +- cmd/datekeys/main.go | 15 ++- cmd/datekeys/present.go | 27 +++++ extension/datekeys.go | 17 ++- internal/inspectview/inspectview.go | 7 +- locator/locator.go | 157 ++++++++++++++++++++++++---- locator/open.go | 34 ++++++ 9 files changed, 240 insertions(+), 38 deletions(-) create mode 100644 locator/open.go diff --git a/authorkey/authorkey.go b/authorkey/authorkey.go index 304806a..f0bcd7f 100644 --- a/authorkey/authorkey.go +++ b/authorkey/authorkey.go @@ -56,7 +56,7 @@ func Generate() (*Key, error) { // NewFromSeed returns the key of a seed of 32 bytes. func NewFromSeed(seed []byte) (*Key, error) { if len(seed) != ed25519.SeedSize { - return nil, fmt.Errorf("authorkey: a seed has %d bytes, not %d", ed25519.SeedSize, len(seed)) + return nil, fmt.Errorf("authorkey: a seed has %d bytes, not %d", len(seed), ed25519.SeedSize) } return &Key{ed25519.NewKeyFromSeed(seed)}, nil } @@ -181,6 +181,9 @@ func Read(r io.Reader, passphrase string) (*Key, error) { if err != nil { return nil, err } + // A file of another work factor is not one of ours (§29.12), and a + // hostile one would make this ask for gigabytes of memory. + id.SetMaxWorkFactor(WorkFactor) ar, err := age.Decrypt(bytes.NewReader(b), id) if err != nil { return nil, fmt.Errorf("authorkey: %w", err) diff --git a/cmd/datekeys/author.go b/cmd/datekeys/author.go index 16323d1..0f95922 100644 --- a/cmd/datekeys/author.go +++ b/cmd/datekeys/author.go @@ -19,7 +19,14 @@ const maxPassFile = 4 << 10 // environment, where other processes can read it. func readPass(cmd, file string, stdin io.Reader) (string, error) { var r io.Reader = stdin - if file != "-" { + if file == "-" { + // A terminal would echo the passphrase and keep it in the scrollback. + if f, ok := stdin.(*os.File); ok { + if info, err := f.Stat(); err == nil && info.Mode()&os.ModeCharDevice != 0 { + return "", fmt.Errorf("%s: the passphrase would be read from a terminal and shown on screen: pipe it in, or use a file", cmd) + } + } + } else { f, err := os.Open(file) if err != nil { return "", err @@ -111,7 +118,9 @@ func authorKeygen(args []string, stdout, stderr io.Writer, stdin io.Reader) erro defer k.Clear() err = writeAtomic(*out, func(w io.Writer) error { if *plain { - _, err := w.Write(authorkey.Marshal(k)) + b := authorkey.Marshal(k) + defer clear(b) + _, err := w.Write(b) return err } return authorkey.Encrypt(w, k, pass) diff --git a/cmd/datekeys/author_test.go b/cmd/datekeys/author_test.go index bfa96d9..eba6132 100644 --- a/cmd/datekeys/author_test.go +++ b/cmd/datekeys/author_test.go @@ -114,8 +114,7 @@ func TestPublicNoteCLI(t *testing.T) { t.Error("a note of two lines was written") } out, _, err := cli(t, genesis, "inspect", "-in", dkc) - if err != nil || !strings.Contains(out, "Nota pública del creador (sin comprobar): Cartas del viaje a Lisboa") || - !strings.Contains(out, "Nadie puede comprobar antes de la fecha quién creó la cápsula ni si va firmada.") { + if err != nil || !strings.Contains(out, "┌ "+noteTitle+"\n│ Cartas del viaje a Lisboa\n└\n Nadie puede comprobar antes de la fecha quién creó la cápsula ni si va firmada.") { t.Errorf("inspect: %v\n%s", err, out) } js, _, err := cli(t, genesis, "inspect", "-in", dkc, "-json") diff --git a/cmd/datekeys/main.go b/cmd/datekeys/main.go index b3d1460..73e15af 100644 --- a/cmd/datekeys/main.go +++ b/cmd/datekeys/main.go @@ -262,6 +262,9 @@ func encrypt(args []string, stderr io.Writer, now func() time.Time) error { } opts.LargeArea = *largeArea opts.PublicNote = *note + if *note != "" { + fmt.Fprintln(stderr, "warning: the public note is in clear: anyone who has the .dkc reads it before the date, nobody can delete it from the copies that circulate, and with the date it can identify someone (spec §24.1).") + } sources, skipped, err := collect(ins, !*noMTime) if err != nil { return fmt.Errorf("encrypt: %w", err) @@ -374,7 +377,13 @@ func decrypt(args []string, stdout, stderr io.Writer, now func() time.Time) erro var pre [capsule.PreludeSize]byte n, _ := src.ReadAt(pre[:], 0) var opened *capsule.Opened - if p, perr := capsule.ParsePrelude(pre[:n]); perr == nil && p.Format == capsule.Format3 { + p, perr := capsule.ParsePrelude(pre[:n]) + if *expect != "" && (perr != nil || p.Format != capsule.Format3) { + // Only a capsule of format 3 has an author signature: fail before + // the release is requested and before anything is written. + return errors.New("decrypt: -expect-author: only a capsule of format 3 has an author signature, and this is not one") + } + if perr == nil && p.Format == capsule.Format3 { if err := checkNew(*out); err != nil { return err } @@ -405,6 +414,9 @@ func decrypt(args []string, stdout, stderr io.Writer, now func() time.Time) erro } present(stdout, opened, *out, outputWidth(stdout)) if *expect != "" && opened.Verdicts.Signature != capsule.VerdictSignedSaved { + if len(opened.Head.Files) == 0 { + return fmt.Errorf("decrypt: the capsule is not signed with the expected key %s: do not trust it as that author's", *expect) + } return fmt.Errorf("decrypt: the capsule is not signed with the expected key %s: its files were written to %s, but do not trust them as that author's", *expect, *out) } return nil @@ -487,6 +499,7 @@ func inspect(args []string, stdout io.Writer) error { } } else { v.WriteText(stdout) + showNote(stdout, result) } return inspectErr } diff --git a/cmd/datekeys/present.go b/cmd/datekeys/present.go index 65ca389..9f97cb9 100644 --- a/cmd/datekeys/present.go +++ b/cmd/datekeys/present.go @@ -7,6 +7,7 @@ import ( "strings" "g.activething.com/go/DateKeys/capsule" + "g.activething.com/go/DateKeys/extension" "g.activething.com/go/DateKeys/internal/pathrule" ) @@ -184,3 +185,29 @@ func risks(path string) []string { } return out } + +// showNote shows the public note of an inspected capsule as spec v0.11 §24.1 +// and §29.7 ask: as text of the creator that nobody has checked, with the +// prefix and the wrapping of any text of the creator, and, before the date, +// the warning that nobody can check who made the capsule or whether it is +// signed. A note that breaks the rules of text is not shown, and the person +// is told. It shows the note even when the capsule fails a check: that is +// when a forged one is most likely. +func showNote(w io.Writer, in *capsule.Inspection) { + if in == nil || in.Header == nil { + return + } + note, ok := in.Header.PublicNote() + if !ok { + for _, e := range in.Header.Noncritical { + if e.ID == extension.NoteID && e.Version == 1 { + fmt.Fprintln(w, " La cápsula lleva una nota pública que no cumple las reglas de texto: no se muestra.") + } + } + return + } + fmt.Fprintln(w, "┌ "+noteTitle) + writeCreator(w, note, outputWidth(w)) + fmt.Fprintln(w, "└") + fmt.Fprintln(w, " Nadie puede comprobar antes de la fecha quién creó la cápsula ni si va firmada.") +} diff --git a/extension/datekeys.go b/extension/datekeys.go index 60dea8c..48a5ad1 100644 --- a/extension/datekeys.go +++ b/extension/datekeys.go @@ -67,7 +67,12 @@ func Note(noncritical []Extension) (string, bool) { // public note in PUBLIC_HEADER and datekeys.capsule in a .dkk, both // noncritical. It validates their data, as spec §54 asks of a reader that // knows an extension, and registers each only where §72 does. -type Standard struct{} +type Standard struct { + // ValidateCapsule checks the data of datekeys.capsule, which package + // locator decodes: it cannot be imported here. locator.Standard sets it. + // When nil, only the presence of the data is checked. + ValidateCapsule func(Extension) error +} // Known reports whether (id, version) is one of the two. func (Standard) Known(id string, version uint64) bool { @@ -82,10 +87,9 @@ func (Standard) RegisteredIn(id string, version uint64, obj Object, arr Array) b return id == NoteID && obj == PublicHeader || id == CapsuleID && obj == AccessKey } -// ValidateData checks the data of a note. The data of datekeys.capsule has -// its own decoder, in package accesskey, which a caller uses to read it: -// this one checks only that it is present. -func (Standard) ValidateData(e Extension) error { +// ValidateData checks the data of a note, and the data of datekeys.capsule +// with ValidateCapsule, when it is set. +func (s Standard) ValidateData(e Extension) error { switch e.ID { case NoteID: if e.Data == nil { @@ -96,6 +100,9 @@ func (Standard) ValidateData(e Extension) error { if e.Data == nil { return fmt.Errorf("datekeys.capsule without data: %w", datekeys.ErrExtensionDataInvalid) } + if s.ValidateCapsule != nil { + return s.ValidateCapsule(e) + } } return nil } diff --git a/internal/inspectview/inspectview.go b/internal/inspectview/inspectview.go index 0cb19f6..d6479da 100644 --- a/internal/inspectview/inspectview.go +++ b/internal/inspectview/inspectview.go @@ -67,10 +67,5 @@ func (v View) WriteText(w io.Writer) { if v.Valid { fmt.Fprintf(w, " valid before unlock; opens at %s (round %d, %s, format %d)\n", v.UnlockAt, v.Round, v.AccessPolicy, v.Format) } - if v.PublicNote != "" { - fmt.Fprintf(w, " Nota pública del creador (sin comprobar): %s\n", v.PublicNote) - if v.Valid { - fmt.Fprintln(w, " Nadie puede comprobar antes de la fecha quién creó la cápsula ni si va firmada.") - } - } + } diff --git a/locator/locator.go b/locator/locator.go index 2aa570d..2e9881d 100644 --- a/locator/locator.go +++ b/locator/locator.go @@ -17,7 +17,7 @@ import ( "errors" "fmt" "io" - "net/url" + "net/netip" "strings" "filippo.io/age" @@ -59,6 +59,12 @@ type Info struct { // Extension returns the extension for the noncritical array of a .dkk. func (i *Info) Extension() (extension.Extension, error) { + if d, err := datekey.Parse(i.DateKey.Compact()); err != nil || d != i.DateKey { + return extension.Extension{}, errors.New("locator: Info.DateKey is not a canonical DateKey") + } + if i.Sealed != nil && (len(i.Sealed) < 1 || len(i.Sealed) > maxSealed) { + return extension.Extension{}, fmt.Errorf("locator: a sealed locator of %d bytes, not 1 to %d", len(i.Sealed), maxSealed) + } if i.Note != "" { if err := extension.CheckNote(i.Note); err != nil { return extension.Extension{}, err @@ -183,28 +189,130 @@ func CheckURI(uri string) error { return errors.New("locator: an address with a character outside printable ASCII") } } - u, err := url.Parse(uri) + scheme, host, err := splitAuthority(uri) if err != nil { - return fmt.Errorf("locator: an address that is not a URI: %w", err) - } - if u.User != nil || strings.Contains(u.Host, "@") { - return errors.New("locator: an address with userinfo") + return err } - switch u.Scheme { + switch scheme { case "https": - if u.Hostname() == "" { - return errors.New("locator: an https address without a host") - } + return checkHost(host) case "ipfs": - if !isCIDv1(u.Host) { + if !isCIDv1(host) { return errors.New("locator: an ipfs address without a CID v1") } - default: - return fmt.Errorf("locator: the scheme %q: only https and ipfs", u.Scheme) + return nil + } + return fmt.Errorf("locator: the scheme %q: only https and ipfs", scheme) +} + +// splitAuthority returns the scheme and the raw host of an address, without +// decoding anything: a percent sign in the authority, userinfo and a +// malformed port are refused, so that the host a reader shows is the host an +// HTTP client would use (spec v0.11, §44.1). +func splitAuthority(uri string) (scheme, host string, err error) { + scheme, rest, ok := strings.Cut(uri, "://") + if !ok || scheme == "" { + return "", "", errors.New("locator: an address without a scheme and ://") + } + authority := rest + if i := strings.IndexAny(rest, "/?#"); i >= 0 { + authority = rest[:i] + } + if strings.ContainsAny(authority, "%@\\") { + return "", "", errors.New("locator: an address with a percent sign, userinfo or a backslash in its authority") + } + host = authority + if scheme == "https" { + if strings.HasPrefix(authority, "[") { + end := strings.Index(authority, "]") + if end < 0 { + return "", "", errors.New("locator: an address with an unclosed IPv6 literal") + } + host = authority[:end+1] + if tail := authority[end+1:]; tail != "" { + if err := checkPort(tail); err != nil { + return "", "", err + } + } + } else if h, port, found := strings.Cut(authority, ":"); found { + host = h + if err := checkPort(":" + port); err != nil { + return "", "", err + } + } + } + return scheme, host, nil +} + +func checkPort(s string) error { + if len(s) < 2 || s[0] != ':' || len(s) > 6 { + return errors.New("locator: an address with a malformed port") + } + n := 0 + for _, c := range s[1:] { + if c < '0' || c > '9' { + return errors.New("locator: an address with a malformed port") + } + n = n*10 + int(c-'0') + } + if n < 1 || n > 65535 { + return errors.New("locator: an address with a port outside 1 to 65535") } return nil } +// checkHost accepts a name of letters, digits, hyphens and dots, or an IP +// literal that is not loopback, private, link-local or unspecified: the spec +// forbids following a redirect to those, and an address that starts there +// would defeat the same rule (§44.1). A name whose last label is numeric, or +// is a hexadecimal number, is refused: some clients read it as an IPv4 +// address in a form that netip does not. +func checkHost(host string) error { + if host == "" { + return errors.New("locator: an https address without a host") + } + if strings.HasPrefix(host, "[") { + a, err := netip.ParseAddr(strings.Trim(host, "[]")) + if err != nil || !publicIP(a) { + return errors.New("locator: an https address with an IPv6 literal that is not public") + } + return nil + } + labels := strings.Split(host, ".") + for _, l := range labels { + if l == "" || len(l) > 63 || l[0] == '-' || l[len(l)-1] == '-' { + return errors.New("locator: an https address with a malformed host") + } + for i := 0; i < len(l); i++ { + c := l[i] + if !(c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' || c >= '0' && c <= '9' || c == '-') { + return errors.New("locator: an https address whose host is not letters, digits and hyphens: write its punycode form") + } + } + } + last := labels[len(labels)-1] + if allDigits(last) || strings.HasPrefix(strings.ToLower(last), "0x") { + a, err := netip.ParseAddr(host) + if err != nil || !a.Is4() || !publicIP(a) { + return errors.New("locator: an https address with a numeric host that is not a public IPv4 address") + } + } + return nil +} + +func allDigits(s string) bool { + for i := 0; i < len(s); i++ { + if s[i] < '0' || s[i] > '9' { + return false + } + } + return s != "" +} + +func publicIP(a netip.Addr) bool { + return !(a.IsLoopback() || a.IsPrivate() || a.IsLinkLocalUnicast() || a.IsLinkLocalMulticast() || a.IsMulticast() || a.IsUnspecified()) +} + // isCIDv1 reports whether s looks like a CID v1 in base32, which starts with // 'b': it checks the alphabet and the length, not the multihash. func isCIDv1(s string) bool { @@ -222,14 +330,11 @@ func isCIDv1(s string) bool { // Host returns what a reader shows before it downloads: the host of an https // address, or the CID of an ipfs one (spec §44.1). func (a Address) Host() string { - u, err := url.Parse(a.URI) - if err != nil { + if CheckURI(a.URI) != nil { return "" } - if u.Scheme == "ipfs" { - return u.Host - } - return u.Hostname() + _, host, _ := splitAuthority(a.URI) + return strings.Trim(host, "[]") } // Locator is the plaintext of the sealed locator (spec §44.1). @@ -258,6 +363,14 @@ func (l *Locator) validate() error { if n := len(l.EnvelopeHeader); n < 1 || n > MaxHeaderLen { return fmt.Errorf("locator: an envelope header of %d bytes, not 1 to %d", n, MaxHeaderLen) } + if l.RestSize > codec.MaxSafeUint { + return errors.New("locator: a rest larger than 2^53 - 1 bytes") + } + for _, a := range l.Addresses { + if a.Offset > codec.MaxSafeUint { + return errors.New("locator: an offset larger than 2^53 - 1") + } + } return nil } @@ -346,6 +459,7 @@ func (l *Locator) Marshal() ([]byte, error) { if pad < 0 { return base, nil } + defer clear(base) // it holds I_SOBRE var p codec.Encoder l.encode(&p, pad) out, err := p.Out() @@ -384,7 +498,7 @@ func Unmarshal(b []byte) (*Locator, error) { case 3: err = copyBstr(d, l.RestDigest[:]) case 4: - l.RestSize, err = d.Uint(1<<63 - 1) + l.RestSize, err = d.Uint(codec.MaxSafeUint) case 5: err = copyBstr(d, l.CapsuleDigest[:]) case 6: @@ -417,6 +531,7 @@ func Unmarshal(b []byte) (*Locator, error) { } // The length is the one Marshal gives: nothing else is canonical. want, err := l.Marshal() + defer clear(want) if err != nil || !bytes.Equal(want, b) { return nil, fmt.Errorf("locator: the plaintext is not %d or the least multiple of %d that holds it: %w", Block, Block, datekeys.ErrNonCanonicalCBOR) } @@ -453,7 +568,7 @@ func decodeAddresses(d *codec.Decoder, l *Locator) error { case 0: a.URI, err = d.Text(MaxURILen) case 1: - if a.Offset, err = d.Uint(1<<63 - 1); err == nil && a.Offset == 0 { + if a.Offset, err = d.Uint(codec.MaxSafeUint); err == nil && a.Offset == 0 { err = fmt.Errorf("an offset of 0 is written by leaving it out: %w", datekeys.ErrNonCanonicalCBOR) } default: diff --git a/locator/open.go b/locator/open.go new file mode 100644 index 0000000..a39207b --- /dev/null +++ b/locator/open.go @@ -0,0 +1,34 @@ +package locator + +import ( + "errors" + + "g.activething.com/go/DateKeys/extension" + "g.activething.com/go/DateKeys/profile" + "g.activething.com/go/DateKeys/provider" +) + +// OpenLocator opens the sealed locator of the extension with the release of +// the round of its own DateKey, in the profile that DateKey names: a locator +// for another round or another chain does not open, and is unusable (spec +// v0.11, §44.1). It fails when the extension has no locator. +func (i *Info) OpenLocator(reg profile.Registry, release provider.Release) (*Locator, error) { + if i.Sealed == nil { + return nil, errors.New("locator: the extension has no locator") + } + p, ok := reg.Lookup(i.DateKey.ProfileID) + if !ok { + return nil, errors.New("locator: the profile of the DateKey is not pinned") + } + return Open(p, i.DateKey.Round, release, i.Sealed) +} + +// Standard returns the registry of the extensions of spec v0.11 with the +// data of datekeys.capsule validated by ParseInfo, as spec §54 asks of a +// reader that knows an extension. +func Standard() extension.Standard { + return extension.Standard{ValidateCapsule: func(e extension.Extension) error { + _, err := ParseInfo(e) + return err + }} +}