Implements a TOFU approach to tls certs #41

Merged
sloum merged 8 commits from tofu into develop 2019-10-02 19:49:36 +00:00
7 changed files with 230 additions and 45 deletions

View File

@ -61,7 +61,7 @@ func (b *Bookmarks) ToggleFocused() {
}
func (b Bookmarks) IniDump() string {
if len(b.Titles) < 0 {
if len(b.Titles) < 1 {
Review

I just happened to see this. The logic was flawed, the length of a slice will never be less than 0. What was meant was == 0 or < 1 (I went with the later, but if there is a strong opinion I can update to the former). This, as well as a few other things in this pr, ended up in the pr accidentally by me seeing something and fixing it rather than brnching and fixing... but then not wanting to git reset and redo changes.

I just happened to see this. The logic was flawed, the length of a slice will never be less than 0. What was meant was == 0 or < 1 (I went with the later, but if there is a strong opinion I can update to the former). This, as well as a few other things in this pr, ended up in the pr accidentally by me seeing something and fixing it rather than brnching and fixing... but then not wanting to git reset and redo changes.
Review

< 1 seems to fit fine in this context. This might be something you feel differently about in the morning.

< 1 seems to fit fine in this context. This might be something you feel differently about in the morning.
return ""
}
out := "[BOOKMARKS]\n"

View File

@ -16,7 +16,6 @@ import (
"tildegit.org/sloum/bombadillo/gopher"
"tildegit.org/sloum/bombadillo/http"
"tildegit.org/sloum/bombadillo/telnet"
// "tildegit.org/sloum/mailcap"
)
//------------------------------------------------\\
@ -33,6 +32,7 @@ type client struct {
BookMarks Bookmarks
TopBar Headbar
FootBar Footbar
Certs gemini.TofuDigest
}
@ -154,7 +154,7 @@ func (c *client) TakeControlInput() {
c.ClearMessage()
c.Scroll(-1)
case 'q', 'Q':
// quite bombadillo
// quit bombadillo
cui.Exit()
case 'g':
// scroll to top
@ -279,6 +279,8 @@ func (c *client) simpleCommand(action string) {
case "B", "BOOKMARKS":
c.BookMarks.ToggleOpen()
c.Draw()
case "R", "REFRESH":
// TODO build refresh code
case "SEARCH":
c.search("", "", "?")
case "HELP", "?":
@ -299,8 +301,27 @@ func (c *client) doCommand(action string, values []string) {
switch action {
case "CHECK", "C":
c.displayConfigValue(values[0])
case "PURGE", "P":
err := c.Certs.Purge(values[0])
if err != nil {
c.SetMessage(err.Error(), true)
c.DrawMessage()
return
}
if values[0] == "*" {
c.SetMessage("All certificates have been purged", false)
c.DrawMessage()
} else {
c.SetMessage(fmt.Sprintf("The certificate for %q has been purged", strings.ToLower(values[0])), false)
c.DrawMessage()
}
err = saveConfig()
if err != nil {
c.SetMessage("Error saving purge to file", true)
c.DrawMessage()
}
case "SEARCH":
c.search(strings.Join(values, " "), "", "")
c.search(values[0], "", "")
case "WRITE", "W":
if values[0] == "." {
values[0] = c.PageState.History[c.PageState.Position].Location.Full
@ -360,7 +381,8 @@ func (c *client) doCommandAs(action string, values []string) {
if c.BookMarks.IsOpen {
c.Draw()
}
case "SEARCH":
c.search(strings.Join(values, " "), "", "")
case "WRITE", "W":
u, err := MakeUrl(values[0])
if err != nil {
@ -472,7 +494,7 @@ func (c *client) saveFile(u Url, name string) {
case "gopher":
file, err = gopher.Retrieve(u.Host, u.Port, u.Resource)
case "gemini":
file, err = gemini.Fetch(u.Host, u.Port, u.Resource)
file, err = gemini.Fetch(u.Host, u.Port, u.Resource, &c.Certs)
default:
c.SetMessage(fmt.Sprintf("Saving files over %s is not supported", u.Scheme), true)
c.DrawMessage()
@ -551,7 +573,7 @@ func (c *client) doLinkCommand(action, target string) {
num -= 1
links := c.PageState.History[c.PageState.Position].Links
if num >= len(links) || num < 0 {
if num >= len(links) || num < 1 {
c.SetMessage(fmt.Sprintf("Invalid link id: %s", target), true)
c.DrawMessage()
return
@ -832,12 +854,13 @@ func (c *client) Visit(url string) {
c.Draw()
}
case "gemini":
capsule, err := gemini.Visit(u.Host, u.Port, u.Resource)
capsule, err := gemini.Visit(u.Host, u.Port, u.Resource, &c.Certs)
if err != nil {
c.SetMessage(err.Error(), true)
c.DrawMessage()
return
}
go saveConfig()
switch capsule.Status {
case 1:
c.search("", u.Full, capsule.Content)
@ -955,7 +978,7 @@ func (c *client) Visit(url string) {
//--------------------------------------------------\\
func MakeClient(name string) *client {
c := client{0, 0, defaultOptions, "", false, MakePages(), MakeBookmarks(), MakeHeadbar(name), MakeFootbar()}
c := client{0, 0, defaultOptions, "", false, MakePages(), MakeBookmarks(), MakeHeadbar(name), MakeFootbar(), gemini.MakeTofuDigest()}
return &c
}

View File

@ -68,9 +68,11 @@ func (s *scanner) scanText() Token {
capInput := strings.ToUpper(buf.String())
switch capInput {
case "DELETE", "ADD", "WRITE", "SET", "RECALL", "R", "SEARCH",
"W", "A", "D", "S", "Q", "QUIT", "B", "BOOKMARKS", "H",
"HOME", "?", "HELP", "C", "CHECK":
case "D", "DELETE", "A", "ADD","W", "WRITE",
"S", "SET", "R", "REFRESH", "SEARCH",
"Q", "QUIT", "B", "BOOKMARKS", "H",
"HOME", "?", "HELP", "C", "CHECK",
"P", "PURGE":
return Token{Action, capInput}
}

View File

@ -24,8 +24,8 @@ type Config struct {
Bookmarks struct {
Titles, Links []string
}
Colors []KeyValue
Review

I went a different way with colors allowing for inversion rather than full coloring. People can still light and dark mode this way, and it just uses their terminal's natural color pallet for foreground and background (plus inversion of that).

I went a different way with colors allowing for inversion rather than full coloring. People can still light and dark mode this way, and it just uses their terminal's natural color pallet for foreground and background (plus inversion of that).
Review

This is a good approach, I like the way these themes look.

This is a good approach, I like the way these themes look.
Settings []KeyValue
Certs []KeyValue
}
type KeyValue struct {
@ -90,8 +90,8 @@ func (p *Parser) Parse() (Config, error) {
case "BOOKMARKS":
c.Bookmarks.Titles = append(c.Bookmarks.Titles, keyval.Value)
c.Bookmarks.Links = append(c.Bookmarks.Links, keyval.Key)
case "COLORS":
c.Colors = append(c.Colors, keyval)
case "CERTS":
c.Certs = append(c.Certs, keyval)
case "SETTINGS":
c.Settings = append(c.Settings, keyval)
}

View File

@ -1,28 +1,138 @@
package gemini
import (
"bytes"
"crypto/sha1"
"crypto/tls"
"fmt"
"io/ioutil"
"strconv"
"strings"
// "tildegit.org/sloum/mailcap"
"time"
)
type Capsule struct {
MimeMaj string
MimeMin string
Status int
Content string
Links []string
Links []string
}
type TofuDigest struct {
certs map[string]string
}
//------------------------------------------------\\
// + + + R E C E I V E R S + + + \\
//--------------------------------------------------\\
func (t *TofuDigest) Purge(host string) error {
host = strings.ToLower(host)
if host == "*" {
t.certs = make(map[string]string)
return nil
} else if _, ok := t.certs[strings.ToLower(host)]; ok {
delete(t.certs, host)
return nil
}
return fmt.Errorf("Invalid host %q", host)
}
func (t *TofuDigest) Add(host, hash string) {
Review

On the topic of 'exists', I noticed it is the term used on wikipedia so maybe I'm just not familiar with it. Maybe this function can be commented to say "checks if the hostname has a certificate that exists in the certificate store."

But it seems very easy to read now that I understand what it is doing. The rest of these receivers have similar styles of names, all in the context of the certificate store. So I'm not sure anymore lol

On the topic of 'exists', I noticed it is the term used on wikipedia so maybe I'm just not familiar with it. Maybe this function can be commented to say "checks if the hostname has a certificate that exists in the certificate store." But it seems very easy to read now that I understand what it is doing. The rest of these receivers have similar styles of names, all in the context of the certificate store. So I'm not sure anymore lol
t.certs[strings.ToLower(host)] = hash
}
func (t *TofuDigest) Exists(host string) bool {
if _, ok := t.certs[strings.ToLower(host)]; ok {
return true
}
return false
}
func (t *TofuDigest) Find(host string) (string, error) {
if hash, ok := t.certs[strings.ToLower(host)]; ok {
return hash, nil
}
Outdated
Review

@asdf The match method has been redone to handle the loop for matching instead of jsut comparing certs. It will search through the PeerCertificates slice and find something or not and respond appropriately.

In the event that a certificate has expired, t.newCert is called to see if the host is offering another certificate that is valid (matches hsotname and is within active/expiry window).

@asdf The match method has been redone to handle the loop for matching instead of jsut comparing certs. It will search through the PeerCertificates slice and find something or not and respond appropriately. In the event that a certificate has expired, `t.newCert` is called to see if the host is offering another certificate that is valid (matches hsotname and is within active/expiry window).
Outdated
Review

This looks nice and I like that it can handle expiration.

This looks nice and I like that it can handle expiration.
return "", fmt.Errorf("Invalid hostname, no key saved")
}
func (t *TofuDigest) Match(host string, cState *tls.ConnectionState) error {
host = strings.ToLower(host)
now := time.Now()
for _, cert := range cState.PeerCertificates {
if t.certs[host] != hashCert(cert.Raw) {
continue
}
if now.Before(cert.NotBefore) {
return fmt.Errorf("Certificate is not valid yet")
}
if now.After(cert.NotAfter) {
return fmt.Errorf("EXP")
}
if err := cert.VerifyHostname(host); err != nil {
return fmt.Errorf("Certificate error: %s", err)
}
return nil
}
Review

@asdf This is a new method that will look at the available PeerCertificates and try to find a valid one to save as a certificate. If it cannot find a valid certificate it will respond with an error rather than nil. If it finds one, it adds it.

@asdf This is a new method that will look at the available PeerCertificates and try to find a valid one to save as a certificate. If it cannot find a valid certificate it will respond with an error rather than nil. If it finds one, it adds it.
return fmt.Errorf("No matching certificate was found for host %q", host)
}
func (t *TofuDigest) newCert(host string, cState *tls.ConnectionState) error {
host = strings.ToLower(host)
now := time.Now()
for _, cert := range cState.PeerCertificates {
if now.Before(cert.NotBefore) {
continue
}
if now.After(cert.NotAfter) {
continue
}
if err := cert.VerifyHostname(host); err != nil {
continue
}
t.Add(host, hashCert(cert.Raw))
return nil
}
return fmt.Errorf("No valid certificates were offered by host %q", host)
Review

If anything, maybe this error could indicate why the certs were not valid

If anything, maybe this error could indicate why the certs were not valid
}
func (t *TofuDigest) IniDump() string {
Outdated
Review

Old comments. I will update and try to comment out the mess that follows as well.

Old comments. I will update and try to comment out the mess that follows as well.
if len(t.certs) < 1 {
return ""
}
var out strings.Builder
out.WriteString("[CERTS]\n")
for k, v := range t.certs {
out.WriteString(k)
out.WriteString("=")
Review

This for loop I am really not in love with the internals of... It feels like a mess. It does seem to work, but there is likely a better way. If you/anyone thinks one out, I would LOVE for it to be a good starting point, but not the ending point.

This for loop I am really not in love with the internals of... It feels like a mess. It does seem to work, but there is likely a better way. If you/anyone thinks one out, I would LOVE for it to be a good starting point, but not the ending point.
Review

I see what you mean, but am not too sure how it could be improved.

In the situation where a host cert is recorded because you've visited the site previously, but then that host gets a new cert - maybe the old one expired - will it get to line 124 and return false and prevent the site from loading?

I see what you mean, but am not too sure how it could be improved. In the situation where a host cert is recorded because you've visited the site previously, but then that host gets a new cert - maybe the old one expired - will it get to line 124 and return false and prevent the site from loading?
Review

Just to comment further on the internals - it could just be that it is a bit hard to follow because it's doing a fair few different things each loop. The process requires a lot of checking to work properly. Switch statements can make things like this easier to read.

Also I find the term 'exists' like in hostCertExists a bit confusing. It means that the host has a certificate already recorded in the configuration file, right?

Just to comment further on the internals - it could just be that it is a bit hard to follow because it's doing a fair few different things each loop. The process requires a lot of checking to work properly. Switch statements can make things like this easier to read. Also I find the term 'exists' like in `hostCertExists` a bit confusing. It means that the host has a certificate already recorded in the configuration file, right?
Review

I agree a switch could improve this. Doing separate loops for separate conditionals could take away some of the nested conditionals inside the loop as well. It owuld mean duplicating the loop, but looping through certs should be pretty cheap as I have been assured that there should never really be more than one for gemini servers, two or three at the absolute most (that opinion comes from solderpunk).

Would something like hostIsKnown or previouslyVisited be better? I definitely want to give the sense that a boolean should be expected for the value and want to tie it tot he fact that a certificate is already held by Bombadillo for that host.

I agree a switch could improve this. Doing separate loops for separate conditionals could take away some of the nested conditionals inside the loop as well. It owuld mean duplicating the loop, but looping through certs should be pretty cheap as I have been assured that there should never really be more than one for gemini servers, two or three at the absolute most (that opinion comes from solderpunk). Would something like `hostIsKnown` or `previouslyVisited` be better? I definitely want to give the sense that a boolean should be expected for the value and want to tie it tot he fact that a certificate is already held by Bombadillo for that host.
Review

As to the changing host cert question. Yes, at present the site would be prevented from loading. I am unclear on what the behavior should be at that point. It doesn't make sense to just grab a new one, as we are explicitly trying to prevent someone getting to a site for which there is a bad certificate. and if we just grab a new one when certs dont match or are expired then we diminish the security as a man in the middle could just provide a bad cert with a matching hostname... we'd see no match and then just grab a new one?

Maybe a special case for expiration is in order? So that if the cert expired we know to try to grab a new one? The only problem I see there is if the server stops offering the expired cert, we have no way to know from the hash that it expired and it will always just look like the site doesnt have a valid/matching cert. I'll have to read more about TOFU and see how these cases should be handled. At present, it takes the most cautious method possible.

I do want to add a new command :clear [hostname] or maybe :clearcert [hostname] that would allow a user to clear out a certificate. This seems like something a system should do. It can allow the user to shoot themselves in the foot, but at least gives them the ability to control their system to their liking.

As to the changing host cert question. Yes, at present the site would be prevented from loading. I am unclear on what the behavior should be at that point. It doesn't make sense to just grab a new one, as we are explicitly trying to prevent someone getting to a site for which there is a bad certificate. and if we just grab a new one when certs dont match or are expired then we diminish the security as a man in the middle could just provide a bad cert with a matching hostname... we'd see no match and then just grab a new one? Maybe a special case for expiration is in order? So that if the cert expired we know to try to grab a new one? The only problem I see there is if the server stops offering the expired cert, we have no way to know from the hash that it expired and it will always just look like the site doesnt have a valid/matching cert. I'll have to read more about TOFU and see how these cases should be handled. At present, it takes the most cautious method possible. I do want to add a new command `:clear [hostname]` or maybe `:clearcert [hostname]` that would allow a user to clear out a certificate. This seems like something a system should do. It can allow the user to shoot themselves in the foot, but at least gives them the ability to control their system to their liking.
out.WriteString(v)
out.WriteString("\n")
}
return out.String()
}
//------------------------------------------------\\
// + + + F U N C T I O N S + + + \\
//--------------------------------------------------\\
func Retrieve(host, port, resource string) (string, error) {
func Retrieve(host, port, resource string, td *TofuDigest) (string, error) {
if host == "" || port == "" {
return "", fmt.Errorf("Incomplete request url")
}
@ -41,6 +151,38 @@ func Retrieve(host, port, resource string) (string, error) {
defer conn.Close()
Outdated
Review

This section is now commented and feels much cleaner to me. Let me know what you think. This feels like it can work. It allows for getting a new certificate in the event of an expired one.

The only issue left is if a host no longer offers the expired certificate and just changed their certificate. I believe that is where the :clear [host] command can come into play. If someone keeps getting invalid or no matching certificate, they can clear and try again if they want. Which is semi-inline with ssh (which notifies you of a difference and asks if you'd like to proceed). Both due to the way messaging is structured for bombadillo and my preferred approach to security, I think the way implemented here works well. It blocks things that have behaved weirdly and leaves it to a user that knows what they are doing to move forward and make that choice actively instead of just hitting ok every time a certificate warning comes up.

This section is now commented and feels _much_ cleaner to me. Let me know what you think. This feels like it can work. It allows for getting a new certificate in the event of an expired one. The only issue left is if a host no longer offers the expired certificate and just changed their certificate. I believe that is where the `:clear [host]` command can come into play. If someone keeps getting invalid or no matching certificate, they can clear and try again if they want. Which is semi-inline with ssh (which notifies you of a difference and asks if you'd like to proceed). Both due to the way messaging is structured for bombadillo and my preferred approach to security, I think the way implemented here works well. It blocks things that have behaved weirdly and leaves it to a user that knows what they are doing to move forward and make that choice actively instead of just hitting `ok` every time a certificate warning comes up.
Outdated
Review

Yeah I really like this change and the way this flows now.

I haven't too much experience with certificate handling, but this approach seems the most reasonable for this model. It should be supported by user-friendly information, but not something they might just disregard. I dislike the onus being put completely on the user to understand why something doesn't work.

Yeah I really like this change and the way this flows now. I haven't too much experience with certificate handling, but this approach seems the most reasonable for this model. It should be supported by user-friendly information, but not something they might just disregard. I dislike the onus being put completely on the user to understand why something doesn't work.
connState := conn.ConnectionState()
// Begin TOFU screening...
// If no certificates are offered, bail out
if len(connState.PeerCertificates) < 1 {
return "", fmt.Errorf("Insecure, no certificates offered by server")
}
if td.Exists(host) {
// See if we have a matching cert
err := td.Match(host, &connState)
if err != nil && err.Error() != "EXP" {
// If there is no match and it isnt because of an expiration
// just return the error
return "", err
} else if err != nil {
// The cert expired, see if they are offering one that is valid...
err := td.newCert(host, &connState)
if err != nil {
// If there are no valid certs to offer, let the client know
return "", err
}
}
} else {
err = td.newCert(host, &connState)
if err != nil {
// If there are no valid certs to offer, let the client know
return "", err
}
}
send := "gemini://" + addr + "/" + resource + "\r\n"
_, err = conn.Write([]byte(send))
@ -56,8 +198,8 @@ func Retrieve(host, port, resource string) (string, error) {
return string(result), nil
}
func Fetch(host, port, resource string) ([]byte, error) {
rawResp, err := Retrieve(host, port, resource)
func Fetch(host, port, resource string, td *TofuDigest) ([]byte, error) {
rawResp, err := Retrieve(host, port, resource, td)
if err != nil {
return make([]byte, 0), err
}
@ -103,9 +245,9 @@ func Fetch(host, port, resource string) ([]byte, error) {
}
func Visit(host, port, resource string) (Capsule, error) {
func Visit(host, port, resource string, td *TofuDigest) (Capsule, error) {
capsule := MakeCapsule()
rawResp, err := Retrieve(host, port, resource)
rawResp, err := Retrieve(host, port, resource, td)
if err != nil {
return capsule, err
}
@ -226,8 +368,20 @@ func handleRelativeUrl(u, root, current string) string {
return fmt.Sprintf("%s%s", current, u)
}
func hashCert(cert []byte) string {
hash := sha1.Sum(cert)
Outdated
Review

I changed the delimiter here to : to be more in line with what people are used to seeing in ssh. You may need to clear out the certs you have from your ~/.bombadillo.ini file to be able to view those sites now that the hashing has changed.

I changed the delimiter here to `:` to be more in line with what people are used to seeing in ssh. You may need to clear out the certs you have from your `~/.bombadillo.ini` file to be able to view those sites now that the hashing has changed.
hex := make([][]byte, len(hash))
for i, data := range hash {
hex[i] = []byte(fmt.Sprintf("%02X", data))
}
return fmt.Sprintf("%s", string(bytes.Join(hex, []byte(":"))))
}
func MakeCapsule() Capsule {
return Capsule{"", "", 0, "", make([]string, 0, 5)}
}
func MakeTofuDigest() TofuDigest {
return TofuDigest{make(map[string]string)}
}

11
main.go
View File

@ -24,6 +24,7 @@ import (
"os"
"strings"
_ "tildegit.org/sloum/bombadillo/gemini"
"tildegit.org/sloum/bombadillo/config"
"tildegit.org/sloum/bombadillo/cui"
"tildegit.org/sloum/mailcap"
@ -39,8 +40,8 @@ var mc *mailcap.Mailcap
func saveConfig() error {
var opts strings.Builder
bkmrks := bombadillo.BookMarks.IniDump()
certs := bombadillo.Certs.IniDump()
opts.WriteString(bkmrks)
opts.WriteString("\n[SETTINGS]\n")
for k, v := range bombadillo.Options {
if k == "theme" && v != "normal" && v != "inverse" {
@ -53,6 +54,10 @@ func saveConfig() error {
opts.WriteRune('\n')
}
opts.WriteString(bkmrks)
opts.WriteString(certs)
return ioutil.WriteFile(bombadillo.Options["configlocation"] + "/.bombadillo.ini", []byte(opts.String()), 0644)
}
@ -122,6 +127,10 @@ func loadConfig() error {
bombadillo.BookMarks.Add([]string{v, settings.Bookmarks.Links[i]})
}
for _, v := range settings.Certs {
bombadillo.Certs.Add(v.Key, v.Value)
}
return nil
}

39
url.go
View File

@ -37,7 +37,7 @@ type Url struct {
// an error (or nil).
func MakeUrl(u string) (Url, error) {
var out Url
re := regexp.MustCompile(`^((?P<scheme>gopher|telnet|http|https|gemini):\/\/)?(?P<host>[\w\-\.\d]+)(?::(?P<port>\d+)?)?(?:/(?P<type>[01345679gIhisp])?)?(?P<resource>.*)?$`)
re := regexp.MustCompile(`^((?P<scheme>[a-zA-Z]+):\/\/)?(?P<host>[\w\-\.\d]+)(?::(?P<port>\d+)?)?(?:/(?P<type>[01345679gIhisp])?)?(?P<resource>.*)?$`)
Review

This should not have been in this PR, but I went too far with it and didnt want to turn back. This just makes it so we'll allow any text for the scheme. It will get caught as an unsupported scheme later. This makes it so we dont have to do case insensitivity for only one part of the regex (the scheme)... which would be really slow. Plus the or for the different schemes was less than great anyway.

This should not have been in this PR, but I went too far with it and didnt want to turn back. This just makes it so we'll allow any text for the scheme. It will get caught as an unsupported scheme later. This makes it so we dont have to do case insensitivity for only one part of the regex (the scheme)... which would be really slow. Plus the or for the different schemes was less than great anyway.
match := re.FindStringSubmatch(u)
if valid := re.MatchString(u); !valid {
@ -59,14 +59,16 @@ func MakeUrl(u string) (Url, error) {
}
}
if out.Scheme == "" {
out.Scheme = "gopher"
}
if out.Host == "" {
return out, fmt.Errorf("no host")
}
out.Scheme = strings.ToLower(out.Scheme)
Review

A bunch of things in the client want a lowercase scheme, so doing it here makes sense rather than all over the place later.

A bunch of things in the client want a lowercase scheme, so doing it here makes sense rather than all over the place later.
if out.Scheme == "" {
out.Scheme = "gopher"
}
if out.Scheme == "gopher" && out.Port == "" {
out.Port = "70"
} else if out.Scheme == "http" && out.Port == "" {
@ -75,21 +77,20 @@ func MakeUrl(u string) (Url, error) {
out.Port = "443"
} else if out.Scheme == "gemini" && out.Port == "" {
out.Port = "1965"
}
if out.Scheme == "gopher" && out.Mime == "" {
out.Mime = "1"
}
if out.Mime == "" && (out.Resource == "" || out.Resource == "/") && out.Scheme == "gopher" {
out.Mime = "1"
}
if out.Mime == "7" && strings.Contains(out.Resource, "\t") {
out.Mime = "1"
} else if out.Scheme == "telnet" && out.Port == "" {
out.Port = "23"
}
if out.Scheme == "gopher" {
if out.Mime == "" {
out.Mime = "1"
}
if out.Resource == "" || out.Resource == "/" {
out.Mime = "1"
}
Review

I just realized that the first if could be an or condition on the second. :-/ shoulda noticed that.

I just realized that the first if could be an or condition on the second. :-/ shoulda noticed that.
if out.Mime == "7" && strings.Contains(out.Resource, "\t") {
out.Mime = "1"
}
switch out.Mime {
case "1", "0", "h", "7":
out.DownloadOnly = false
@ -101,10 +102,6 @@ func MakeUrl(u string) (Url, error) {
out.Mime = ""
}
if out.Scheme == "http" || out.Scheme == "https" {
out.Mime = ""
}
out.Full = out.Scheme + "://" + out.Host + ":" + out.Port + "/" + out.Mime + out.Resource
return out, nil